On Sat, 19 Jul 2008, Ivo van Doorn wrote:
> > @@ -150,7 +150,7 @@ static void update_rfkill_state(struct rfkill *rfkill)
> > * even if the radio is in RFKILL_STATE_HARD_BLOCKED state, so as to
> > * give the driver a hint that it should double-BLOCK the transmitter.
> > *
> > - * Caller must have aquired rfkill_mutex.
> > + * Caller must have acquired rfkill->mutex.
>
> Should rfkill_toggle_radio() not grab the rfkill->mutex itself?
> At the moment every caller to rfkill_toggle_radio() does:
>
> mutex_lock(&rfkill->mutex);
> rfkill_toggle_radio(rfkill, state, 0);
> mutex_unlock(&rfkill->mutex);
>
> without anything in between, so perhaps the safest way would be moving
> the locking requirement into the function.
sysfs attributes need to use mutex_lock_interruptible or
mutex_lock_killable, and I also need it with external locking in some later
patches that I haven't sent yet because I am reviewing them.
> > @@ -521,8 +527,11 @@ static void rfkill_remove_switch(struct rfkill *rfkill)
> > {
> > mutex_lock(&rfkill_mutex);
> > list_del_init(&rfkill->node);
> > - rfkill_toggle_radio(rfkill, RFKILL_STATE_SOFT_BLOCKED, 1);
> > mutex_unlock(&rfkill_mutex);
> > +
> > + mutex_lock(&rfkill->mutex);
> > + rfkill_toggle_radio(rfkill, RFKILL_STATE_SOFT_BLOCKED, 1);
> > + mutex_unlock(&rfkill->mutex);
>
> Not sure about this one, something tells me it should be something like:
>
> mutex_lock(&rfkill_mutex);
> list_del_init(&rfkill->node);
>
> mutex_lock(&rfkill->mutex);
> rfkill_toggle_radio(rfkill, RFKILL_STATE_SOFT_BLOCKED, 1);
> mutex_unlock(&rfkill->mutex);
>
> mutex_unlock(&rfkill_mutex);
We really shouldn't need the nesting, as once we have deleted something from
the list (which is always iterated and manipulated with rfkill_mutex
locked), nothing that would need the rfkill_mutex will access that rfkill
struct anymore.
Nesting wouldn't hurt anything, though. If you really feel better with
that nesting in place, I can nest them.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
--
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
