Re: apparently missing locking in if_bnx.c

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Tue, Mar 06, 2012 at 10:33:27AM -0500, Thor Lancelot Simon wrote:
> > 
> > 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.

I agree that patches to get rid of spl locking in drivers should not 
be rejected. But if a driver is made SMP-safe just to "fix" a bug,
then it's not the right thing to do because the bug is still there.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--