On Monday 06 April 2009 12:25:29 Johannes Berg wrote: > Actually, I think the check is invalid, or rather, rfkill.registered > needs to be set before registering the rfkill struct. Would cause a wl->mutex deadlock. > See, right now, and I'm not entirely happy with this, during > rfkill_register I call the set_block callback. The reason I'm not really > happy with that is that I said calling driver -> rfkill -> driver is > bad, but on the other hand due to locking constraints across the > subsystems the driver cannot take the same locks around rfkill_register > as in the callbacks. Exactly. ;) > Additionally, the driver must be ready to service rfkill calls _before_ > rfkill_register() is called, everything else results in a race > condition. Compare with the shared interrupt handlers, for instance. The driver _is_ ready. It's just not ready to service recursive calls due to the deadlock. That's what we protect against. > The reason I call it during rfkill_register is that I need to sync the > software state to the driver's block state, and that needs to be done > somewhere. Doing that from a work struct would be possible, but wouldn't > solve the locking constraints nor would it actually help since > rfkill_register can potentially sleep, so the work could still be > executed before rfkill_register returns -- the driver still needs to be > able to handle rfkill callbacks before rfkill_register returns. No. The mutex mechanism will make sure there's no deadlock and the scheduler will schedule the threads in correct order, if you call set_block from the workqueue. So I think doing the initial set_block from the workqueue asynchronously _would_ solve the deadlock issue. > Therefore, the warning is spurious, but because of a driver bug > elsewhere -- I think the rfkill.registered = 1 assignment should be > moved up to before rfkill_register() (and be = 0 again if that fails > unexpectedly); I'll do that in my patch. As I said. The driver cannot service callbacks while it's registering. So the registered=1 must remain where it is. Simply remove the warning, please. -- Greetings, Michael. -- 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