On Thu, Feb 17, 2011 at 10:24 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
> On Thu, 2011-02-17 at 02:51 +0200, Eliad Peller wrote:
>
>> >> changed |= BSS_CHANGED_ASSOC;
>> >> + mutex_lock(&sdata->u.mgd.mtx);
>> >> ieee80211_bss_info_change_notify(sdata, changed);
>> >> + mutex_unlock(&sdata->u.mgd.mtx);
>> >> break;
>> >> case NL80211_IFTYPE_ADHOC:
>> >> changed |= BSS_CHANGED_IBSS;
>> >
>> > Oi, this is a complicated locking rule.
>> >
>> > Can you please stick a
>> >
>> > lockdep_assert_held(...)
>> >
>> > in the right place to make sure we catch it even on drivers that don't
>> > use the probe_req function?
>> >
>> well, i'm not sure what is the right place / when this mutex should be taken.
>> when doing something like:
>>
>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>> index 1c507c6..ab1843d 100644
>> --- a/net/mac80211/main.c
>> +++ b/net/mac80211/main.c
>> @@ -191,6 +191,7 @@ void ieee80211_bss_info_change_notify(struct
>> ieee80211_sub_if_data *sdata,
>> return;
>>
>> if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>> + lockdep_assert_held(&sdata->u.mgd.mtx);
>> /*
>> * While not associated, claim a BSSID of all-zeroes
>> * so that drivers don't do any weird things with the
>>
>> i got several warnings from ieee80211_open, ieee80211_recalc_idle, etc.
>> should this mutex be taken if the psm has been changed (for example)?
>> or should we check the mutex only if certain flags has been changed?
>
> Yikes!! So you're saying that the locking is conditional on a certain
> flag? That makes the driver API a lot more complex, and the locking
> rules get really icky. I really don't like that.
>
me neither :)
> I think the answer to that is that we must rewrite the entire API. My
> suggestion would be:
> * remove ieee80211_ap_probereq_get
> * add a new driver op "void set_ap_probe_req(..., skb)"
> * call this in the few appropriate places, but not from
> ieee80211_bss_info_change_notify
> * make sure the locking for it is correct
>
such solution will simplify the locking.
however, it does require some additional work, and i'm pretty loaded right now.
i'll add it to my TODO list.
Eliad.
--
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
