Re: apparently missing locking in if_bnx.c

[Thor Lancelot Simon <tls%panix.com@localhost>]

On Tue, Mar 06, 2012 at 03:55:09PM +0100, Manuel Bouyer wrote:
> On Tue, Mar 06, 2012 at 09:43:42AM -0500, Thor Lancelot Simon wrote:
> > On Tue, Mar 06, 2012 at 12:51:29PM +0100, Manuel Bouyer wrote:
> > >
> > > No network driver is SMP-safe at this time, so they're all running
> > > under KERNEL_LOCK. splnet() is enough to protect the queues.
> > 
> > I think there are a few that are SMP-safe.  One of Matt's for a powerpc 
> > board,
> > and probably ixgbe.
> > 
> 
> It's possible some of then have already been made SMP-safe (I think xennet
> is, or is close to be). But all the driver's entry points from upper
> level (e.g. if_ethersubr.c, netinet, etc ...) should still be called with
> KERNEL_LOCK held (partly because most network drivers are not SMP-safe
> yet; parly because the upper levels are not SMP-safe either).
> If the driver's interrupt is not registered SMP-safe it will be called
> with KERNEL_LOCK too, and so the whole driver can safely use spl locking.

But I believe there are, in fact, one or two drivers in the tree whose 
interrupts
are registered SMP-safe.  I don't think it's a good idea to skip over 
opportunities
to phase out spl "locking" in drivers just because, for a little while longer at
least, it's stiff safe.

> Now it's possible some code path enters a network driver without KERNEL_LOCK;
> but this would be a bug that could cause concurency issues, and I don't think
> the right place to fix it is in the driver itself at this time (or all drivers
> would have to be fixed at once).

I'd be in favor of both.  Replace spl "locking" in drivers incrementally, and 
also
figure out what isn't holding KERNEL_LOCK in the case we're talking about here.

I am wondering whether a stacked software driver is what's calling the bnx
start routine in BBN's case.  The locking in some of those is kind of dodgy.

Thor