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

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2B_eA9jxs3FatSLs8fseb%2BJSrqWrYkegDHyKA1q2QH=Q4_LbJQ>