[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: if_iwn.c patch to support the 5100... (analysis + patch)
On Fri, Sep 18, 2009 at 11:27:01AM -0400, Christos Zoulas wrote:
> On Sep 18, 9:14am, sverre%viewmark.com@localhost (Sverre Froyen) wrote:
> -- Subject: Re: if_iwn.c patch to support the 5100... (analysis + patch)
> | On Fri September 18 2009 04:05:19 Brett Lymn wrote:
> | > On Thu, Sep 17, 2009 at 09:39:20AM -0600, Sverre Froyen wrote:
> | > > It looks like these errors may be because iwn_init is called a second
> | > > time before the first call has completed. It is called via the
> | > > pointer and also through iwn_ioctl. The call via the ioctl path only
> | > > happens if the IFF_UP is set. I notice that, e.g., net/if_ethersubr.c
> | > > sets this flag before the call to if_init. While this seems
> | > > counterintuitive to me it is appears to be the way it's always worked.
> | > > The simplest fix is to remove the call to iwn_init from iwn_ioctl:
> | >
> | > Doesn't that have the unfortunate side effect of you then being unable
> | > to reset the interface by doing an ifconfig down/up?
> | You are correct. Calling it a patch was an exaggeration. I quick test
> | that ifconfig up/down indeed leaves the interface in a non-working state.
> | (Still better than a panic at boot, though.)
> | In order to serialize access (or really prevent a second simultaneous
> | is the following the correct approach?
> cvs rdiff -u -r1.151 -r1.152 src/sys/netinet6/in6.c
> cvs rdiff -u -r1.82 -r1.83 src/sys/netinet6/in6_ifattach.c
> cvs rdiff -u -r1.11 -r1.12 src/sys/netinet6/in6_ifattach.h
> Can you try backing out those three changes instead?
As I mentioned to Christos the other day, I strongly suspect that
backing out those changes of mine will fix iwn(4). It will fix some
problems that Matthias Drochner has with an(4) and wi(4), too. It
may also fix a problem that I am having with ath(4). It is just a
coincidence that those are all wireless drivers, BTW: an, wi, and
iwn are affected because they sleep in if_init, which gives the IPv6
workqueue (introduced with my changes) a chance to invoke if_init,
again, before the first invocation is complete. I am not sure if/why
ath(4) is affected.
In the short term, I will back out the IPv6 workqueue and rewrite agr(4)
and gre(4) locking to use an icky recursive lock. I will probably
re-use the recursive lock that I use to synchronize pmf(9) access to
device_t's. I will probably not get around to that until next week.
In the long run, we need to document the kernel's current expectations
about the ifnet entry points:
No more than one thread may be in the same if_ioctl routine
at once, however, a call stack if_ioctl -> ... -> if_ioctl
is possible, if only through the IPv6 stack.
if_init is not re-entrant.
We also need to think about how to synchronize access to ifnet's in the
David Young OJC Technologies
dyoung%ojctech.com@localhost Urbana, IL * (217) 278-3933
Main Index |
Thread Index |