On Tue, 2008-05-27 at 07:13 -0600, Matthew Wilcox wrote: > On Thu, May 22, 2008 at 04:21:03PM -0700, Daniel Walker wrote: > > Signed-off-by: Daniel Walker <dwalker@xxxxxxxxxx> > > Reviewing the driver, this really doesn't feel like a classical lock. > It feels very much ad-hoc, with most references to ->sem being > unbalanced. It should be a classical lock actually. There shouldn't be any case where semaphore behavior is used or needed. > The driver has a thread (airo_thread) and it seems to be trying to > synchronise against both the interrupt handler, and other threads > (calling eg ->get_stats). Right, though it's important to note that anything coming from WEXT will already be under the rtnl lock and should already be serialized, so we don't need to protect any of the WEXT handlers _from themselves_, just from the main thread and the interrupt handler. > I think it really wants to be using spin_lock_irqsave() instead of a > mutex. I'd want to study the driver in a lot more detail before making > that kind of change though. Yeah, that might be the right way to go. If you want to jump into the driver, feel quite free, but I don't think you need to go into too much detail with it. Your overall behavior analysis is right on the mark already. Dan -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
