Subject: [PATCH] mac80211: avoid calling ieee80211_work_work unconditionally

On suspend, there might be usb wireless drivers which wrongly trigger
the warning in ieee80211_work_work. If an usb driver doesn't have a
suspend hook, the usb stack will disconnect the device. On disconnect,
a mac80211 driver calls ieee80211_unregister_hw, which calls dev_close,
which calls ieee80211_stop, and in the end calls ieee80211_work_purge->
ieee80211_work_work.

The problem is that this call to ieee80211_work_purge comes after
mac80211 is suspended, triggering the warning even when we don't have
work queued in work_list (the expected case when already suspended),
because it always calls ieee80211_work_work.

So, just call ieee80211_work_work in ieee80211_work_purge if we really
have to abort work. This addresses the warning reported at
https://bugzilla.kernel.org/show_bug.cgi?id=24402

Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
---
net/mac80211/work.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/work.c b/net/mac80211/work.c
index de43753..36305e0 100644
--- a/net/mac80211/work.c
+++ b/net/mac80211/work.c
@@ -1074,11 +1074,13 @@ void ieee80211_work_purge(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_local *local = sdata->local;
struct ieee80211_work *wk;
+ bool cleanup = false;

mutex_lock(&local->mtx);
list_for_each_entry(wk, &local->work_list, list) {
if (wk->sdata != sdata)
continue;
+ cleanup = true;
wk->type = IEEE80211_WORK_ABORT;
wk->started = true;
wk->timeout = jiffies;
@@ -1086,7 +1088,8 @@ void ieee80211_work_purge(struct ieee80211_sub_if_data *sdata)
mutex_unlock(&local->mtx);

/* run cleanups etc. */
- ieee80211_work_work(&local->work_work);
+ if (cleanup)
+ ieee80211_work_work(&local->work_work);

mutex_lock(&local->mtx);
list_for_each_entry(wk, &local->work_list, list) {
--
1.7.3.3



2010-12-13 17:30:13

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] mac80211: avoid calling ieee80211_work_work unconditionally

On Mon, Dec 13, 2010 at 5:43 AM, Herton Ronaldo Krzesinski
<[email protected]> wrote:
> On suspend, there might be usb wireless drivers which wrongly trigger
> the warning in ieee80211_work_work. If an usb driver doesn't have a
> suspend hook, the usb stack will disconnect the device. On disconnect,
> a mac80211 driver calls ieee80211_unregister_hw, which calls dev_close,
> which calls ieee80211_stop, and in the end calls ieee80211_work_purge->
> ieee80211_work_work.
>
> The problem is that this call to ieee80211_work_purge comes after
> mac80211 is suspended, triggering the warning even when we don't have
> work queued in work_list (the expected case when already suspended),
> because it always calls ieee80211_work_work.
>
> So, just call ieee80211_work_work in ieee80211_work_purge if we really
> have to abort work. This addresses the warning reported at
> https://bugzilla.kernel.org/show_bug.cgi?id=24402
>
> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>

Cc: [email protected] ?

Luis

2010-12-13 17:34:15

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: avoid calling ieee80211_work_work unconditionally

On Mon, 2010-12-13 at 09:29 -0800, Luis R. Rodriguez wrote:
> On Mon, Dec 13, 2010 at 5:43 AM, Herton Ronaldo Krzesinski
> <[email protected]> wrote:
> > On suspend, there might be usb wireless drivers which wrongly trigger
> > the warning in ieee80211_work_work. If an usb driver doesn't have a
> > suspend hook, the usb stack will disconnect the device. On disconnect,
> > a mac80211 driver calls ieee80211_unregister_hw, which calls dev_close,
> > which calls ieee80211_stop, and in the end calls ieee80211_work_purge->
> > ieee80211_work_work.
> >
> > The problem is that this call to ieee80211_work_purge comes after
> > mac80211 is suspended, triggering the warning even when we don't have
> > work queued in work_list (the expected case when already suspended),
> > because it always calls ieee80211_work_work.
> >
> > So, just call ieee80211_work_work in ieee80211_work_purge if we really
> > have to abort work. This addresses the warning reported at
> > https://bugzilla.kernel.org/show_bug.cgi?id=24402
> >
> > Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
>
> Cc: [email protected] ?

It's just a warning, do we really want to bother?

johannes


2010-12-13 17:39:47

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] mac80211: avoid calling ieee80211_work_work unconditionally

On Mon, Dec 13, 2010 at 9:34 AM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2010-12-13 at 09:29 -0800, Luis R. Rodriguez wrote:
>> On Mon, Dec 13, 2010 at 5:43 AM, Herton Ronaldo Krzesinski
>> <[email protected]> wrote:
>> > On suspend, there might be usb wireless drivers which wrongly trigger
>> > the warning in ieee80211_work_work. If an usb driver doesn't have a
>> > suspend hook, the usb stack will disconnect the device. On disconnect,
>> > a mac80211 driver calls ieee80211_unregister_hw, which calls dev_close,
>> > which calls ieee80211_stop, and in the end calls ieee80211_work_purge->
>> > ieee80211_work_work.
>> >
>> > The problem is that this call to ieee80211_work_purge comes after
>> > mac80211 is suspended, triggering the warning even when we don't have
>> > work queued in work_list (the expected case when already suspended),
>> > because it always calls ieee80211_work_work.
>> >
>> > So, just call ieee80211_work_work in ieee80211_work_purge if we really
>> > have to abort work. This addresses the warning reported at
>> > https://bugzilla.kernel.org/show_bug.cgi?id=24402
>> >
>> > Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
>>
>> Cc: [email protected] ?
>
> It's just a warning, do we really want to bother?

Why not, it pollutes logs.

Luis