Re: b43 WARN_ON

[Michael Buesch <mb@bu3sch.de>]

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.