Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Dec 2016 11:05:40 -0800
From:      Navdeep Parhar <np@freebsd.org>
To:        Vincenzo Maffione <v.maffione@gmail.com>
Cc:        Luigi Rizzo <luigi@freebsd.org>, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>,  Giuseppe Lettieri <g.lettieri@iet.unipi.it>
Subject:   Re: cxgbe's native netmap support broken since r307394
Message-ID:  <CAPFoGT9VoRWF306gBYgSKd2wi1rx3_2-yLpnxO98GyM0V-0Xog@mail.gmail.com>
In-Reply-To: <CA%2B_eA9jxs3FatSLs8fseb%2BJSrqWrYkegDHyKA1q2QH=Q4_LbJQ@mail.gmail.com>
References:  <20161217222812.GA4979@ox> <CA%2B_eA9jxs3FatSLs8fseb%2BJSrqWrYkegDHyKA1q2QH=Q4_LbJQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
IFNET_RLOCK will work, thanks.

Navdeep

On Mon, Dec 19, 2016 at 3:21 AM, Vincenzo Maffione <v.maffione@gmail.com> wrote:
> Hi Navdeep,
>
>   Indeed, we have reviewed the code, and we think it is ok to
> implement nm_os_ifnet_lock() with IFNET_RLOCK(), instead of using
> IFNET_WLOCK().
> Since IFNET_RLOCK() results into sx_slock(), this should fix the issue.
>
> On FreeBSD, this locking is needed to protect a flag read by nm_iszombie().
> However, on Linux the same lock is also needed to protect the call to
> the nm_hw_register() callback, so we prefer to have an "unified"
> locking scheme, i.e. always calling nm_hw_register under the lock.
>
> Does this make sense to you? Would it be easy for you to make a quick
> test by replacing IFNET_WLOCK with IFNET_RLOCK?
>
> Thanks,
>   Vincenzo
>
> 2016-12-17 23:28 GMT+01:00 Navdeep Parhar <np@freebsd.org>:
>> Luigi, Vincenzo,
>>
>> The last major update to netmap (r307394 and followups) broke cxgbe's
>> native netmap support.  The problem is that netmap_hw_reg now holds an
>> rw_lock around the driver's netmap_on/off routines.  It has always been
>> safe for the driver to sleep during these operations but now it panics
>> instead.
>>
>> Why is IFNET_WLOCK needed here?  It seems like a regression to disallow
>> sleep on the control path.
>>
>> Regards,
>> Navdeep
>>
>> begin_synchronized_op with the following non-sleepable locks held:
>> exclusive rw ifnet_rw (ifnet_rw) r = 0 (0xffffffff8271d680) locked @
>> /root/ws/head/sys/dev/netmap/netmap_freebsd.c:95
>> stack backtrace:
>> #0 0xffffffff810837a5 at witness_debugger+0xe5
>> #1 0xffffffff81084d88 at witness_warn+0x3b8
>> #2 0xffffffff83ef2bcc at begin_synchronized_op+0x6c
>> #3 0xffffffff83f14beb at cxgbe_netmap_reg+0x5b
>> #4 0xffffffff809846f1 at netmap_hw_reg+0x81
>> #5 0xffffffff809806de at netmap_do_regif+0x19e
>> #6 0xffffffff8098121d at netmap_ioctl+0x7ad
>> #7 0xffffffff8098682f at freebsd_netmap_ioctl+0x5f
>
>
>
> --
> Vincenzo Maffione
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPFoGT9VoRWF306gBYgSKd2wi1rx3_2-yLpnxO98GyM0V-0Xog>