From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] mac80211: Don't restart sta-timer if not associated.
Date: Tue, 19 Mar 2013 13:46:37 -0700 [thread overview]
Message-ID: <5148CEAD.3010908@candelatech.com> (raw)
In-Reply-To: <1363724490.8336.18.camel@jlt4.sipsolutions.net>
On 03/19/2013 01:21 PM, Johannes Berg wrote:
> On Thu, 2013-03-14 at 17:24 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> I found another crash when deleting lots of virtual stations
>> in a congested environment. I think the problem is that
>> the ieee80211_mlme_notify_scan_completed could call
>> ieee80211_restart_sta_timer for a non-associated interface
>> that was about to be deleted.
>>
>> With the following patch I am unable to reproduce the
>> crash.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>> v2: Be more selective: Still need to do sdata->work even
>> if not associated so that we *can* associate.
>>
>> :100644 100644 81e0619... da805e2... M net/mac80211/mlme.c
>> net/mac80211/mlme.c | 7 +++++--
>> 1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index 81e0619..da805e2 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -2935,11 +2935,14 @@ static void ieee80211_restart_sta_timer(struct ieee80211_sub_if_data *sdata)
>>
>> /* let's probe the connection once */
>> flags = sdata->local->hw.flags;
>> - if (!(flags & IEEE80211_HW_CONNECTION_MONITOR))
>> + if ((!(flags & IEEE80211_HW_CONNECTION_MONITOR)) &&
>> + sdata->u.mgd.associated)
>
> You really didn't need those extra parentheses :-)
I like them :)
...never can remember if the ! operator takes
precedence over &&, but can remove them...
>> ieee80211_queue_work(&sdata->local->hw,
>> &sdata->u.mgd.monitor_work);
>> +
>> /* and do all the other regular work too */
>> - ieee80211_queue_work(&sdata->local->hw, &sdata->work);
>> + if (ieee80211_sdata_running(sdata))
>> + ieee80211_queue_work(&sdata->local->hw, &sdata->work);
>
> Overall, it seems that it would be safe to just check
> ieee80211_sdata_running() in the beginning of the function instead?
I'm not sure. I can look at the code closer tomorrow perhaps.
> Maybe you could also look at the ibss/mesh code and fix this issue for
> good?
Will poke around in that as well, but I have no way to test it currently.
Ben
>
> johannes
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
prev parent reply other threads:[~2013-03-19 20:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-15 0:24 [PATCH v2] mac80211: Don't restart sta-timer if not associated greearb
2013-03-19 20:21 ` Johannes Berg
2013-03-19 20:46 ` Ben Greear [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5148CEAD.3010908@candelatech.com \
--to=greearb@candelatech.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.