Re: [PATCH] mac80211: Clean up work-queues on disassociation.

[Ben Greear <greearb@candelatech.com>]

On 02/19/2013 10:23 PM, Julian Calaby wrote:
> Hi Ben,
>
> On Wed, Feb 20, 2013 at 1:11 PM,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> The monitor_work and beacon_connection_loss_work items were
>> not being canceled on disassociation (and not on deletion
>> either).  This leads to work-items trying to run after memory
>> has been deleted.
>>
>> I could not find a cleaner way to do this because the
>> cancel_work_sync for these items must be done outside
>> of the ifmgd->mtx.
>>
>> In addition, re-order the quiesce code so that timers are
>> always stopped before work-items are flushed.  This was
>> not the problem I saw, but I think it may still be more
>> correct.
>>
>> This fixes the crashes we see in 3.7.9+.  The crash stack
>> trace itself isn't so helpful, but this warning gives
>> more useful info:
>
> [snip]
>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>
>> NOTE:  Please do not apply this until it is reviewed by Johannes,
>> at least.
>>
>>   net/mac80211/mlme.c |   68 +++++++++++++++++++++++++++++++++++++++++++++-----
>>   1 files changed, 61 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index 164ecf0..5a65144 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -2954,19 +2978,15 @@ void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata)
>>
>>          cancel_work_sync(&ifmgd->request_smps_work);
>>
>> +       sdata_err(sdata, "Cancel-work-sync monitor_work, sta_quiesce.\n");
>
> Debug code?

Err, yes..will remove.

>>   void ieee80211_sta_restart(struct ieee80211_sub_if_data *sdata)
>> @@ -3262,6 +3282,8 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
>>          struct ieee80211_mgd_auth_data *auth_data;
>>          u16 auth_alg;
>>          int err;
>> +       bool maybe_cancel_work = false;
>> +       bool hit_err_clear = false;
>
> You could replace these with a single variable.

I don't see how.  It seems that we should only cancel the work items
if we called set_disassoc AND if the subsequent attempts to (re)associate
failed.

Maybe I missed something?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com