svn commit: r301198 - head/sys/dev/xen/netfront

[John Baldwin]

On Tuesday, May 09, 2017 08:36:13 PM Colin Percival wrote:
> On 05/09/17 03:09, Roger Pau Monn� wrote:
> > On Wed, May 03, 2017 at 05:13:40AM +0000, Colin Percival wrote:
> >> On 06/02/16 04:16, Roger Pau Monn� wrote:
> >>> Author: royger
> >>> Date: Thu Jun  2 11:16:35 2016
> >>> New Revision: 301198
> >>> URL: https://svnweb.freebsd.org/changeset/base/301198
> >>
> >> I think this commit is responsible for panics I'm seeing in EC2 on T2 family
> >> instances.  [...]
> >> but under high traffic volumes I think a separate thread can already be
> >> running in xn_rxeof, having dropped the RX lock while it passes a packet
> >> up the stack.  This would result in two different threads trying to process
> >> the same set of responses from the ring, with (unsurprisingly) bad results.
> > 
> > Hm, right, xn_rxeof drops the lock while pushing the packet up the stack.
> > There's a "XXX" comment on top of that, could you try to remove the lock
> > dripping and see what happens?
> > 
> > I'm not sure there's any reason to drop the lock here, I very much doubt
> > if_input is going to sleep.
> 
> Judging by
> $ grep -R -B 1 -A 1 if_input /usr/src/sys/dev
> I'm pretty sure that we do indeed need to drop the lock.  If it's possible
> to enter if_input while holding locks, there are a *lot* of network interface
> drivers which are dropping locks unnecessarily...

It depends on how the locks are used.  If a NIC driver uses a single lock for
both TX and RX, then it needs to drop the lock as on the TX side,
if_transmit/if_start will be invoked with various network stack locks held.
However, if a driver uses separate locks for RX and TX and doesn't acquire
the RX lock while holding a TX lock, then it should be safe to hold the lock
across if_input.

A lot of the older 100Mbps PCI NIC drivers only use a single lock and thus
need to drop the lock.  Many multiqueue NIC drivers use separate locks
however and probably don't need to drop them around if_input.

-- 
John Baldwin