[PATCH] mac80211: change workqueue back to non-freezeable

[PATCH] mac80211: change workqueue back to non-freezeable

[Bob Copeland]

[apologies for resend, forgot to cc wireless]

Commit 59959a6150c8af737898e83f727e824dbed7b0fa made the mac80211
workqueue freezeable to prevent us from doing any work after the 
driver went away.  This was fine before mac80211 had any suspend 
support.

However, now we want to flush this workqueue in suspend().  Because
the thread for a freezeable workqueue is stopped before the device
class suspend() is called, flush_workqueue() will hang in the
suspend-to-disk case.

Converting it back to a non-freezeable queue will keep suspend from
hanging.  Moreover, since we flush the workqueue under RTNL and
userspace is stopped, there won't be any new work in the workqueue
until after resume.  Thus we still don't have to worry about pinging
the AP without hardware.

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 net/mac80211/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index a100793..a109c06 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -859,7 +859,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	mdev->set_multicast_list = ieee80211_master_set_multicast_list;
 
 	local->hw.workqueue =
-		create_freezeable_workqueue(wiphy_name(local->hw.wiphy));
+		create_singlethread_workqueue(wiphy_name(local->hw.wiphy));
 	if (!local->hw.workqueue) {
 		result = -ENOMEM;
 		goto fail_workqueue;
-- 
1.6.0.6

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] mac80211: change workqueue back to non-freezeable

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Fri, 2009-01-23 at 23:12 -0500, Bob Copeland wrote:
> [apologies for resend, forgot to cc wireless]
> 
> Commit 59959a6150c8af737898e83f727e824dbed7b0fa made the mac80211
> workqueue freezeable to prevent us from doing any work after the 
> driver went away.  This was fine before mac80211 had any suspend 
> support.
> 
> However, now we want to flush this workqueue in suspend().  Because
> the thread for a freezeable workqueue is stopped before the device
> class suspend() is called, flush_workqueue() will hang in the
> suspend-to-disk case.
> 
> Converting it back to a non-freezeable queue will keep suspend from
> hanging.  Moreover, since we flush the workqueue under RTNL and
> userspace is stopped, there won't be any new work in the workqueue
> until after resume.  Thus we still don't have to worry about pinging
> the AP without hardware.

Good catch! Do we have to freeze the workqueue ourselves manually or
something, to avoid drivers adding work to it?

> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
>  net/mac80211/main.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index a100793..a109c06 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -859,7 +859,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>  	mdev->set_multicast_list = ieee80211_master_set_multicast_list;
>  
>  	local->hw.workqueue =
> -		create_freezeable_workqueue(wiphy_name(local->hw.wiphy));
> +		create_singlethread_workqueue(wiphy_name(local->hw.wiphy));
>  	if (!local->hw.workqueue) {
>  		result = -ENOMEM;
>  		goto fail_workqueue;
> -- 
> 1.6.0.6
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: change workqueue back to non-freezeable

[Bob Copeland]

> Good catch! Do we have to freeze the workqueue ourselves manually or
> something, to avoid drivers adding work to it?

Yeah I guess so - it would only matter if they add work in one
of the callbacks we use to remove keys, interfaces, etc.   This
might be a good case for the in_suspend flag.  Or, we could just 
flush the workqueue again at the end of suspend to make sure 
nothing was added.

The other thing I don't get is why STR works fine but STD hangs,
from my reading of stuff in kernel/power they should be the same
with respect to order of task-freeze and suspend.  

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] mac80211: change workqueue back to non-freezeable

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On Sat, 2009-01-24 at 09:46 -0500, Bob Copeland wrote:
> > Good catch! Do we have to freeze the workqueue ourselves manually or
> > something, to avoid drivers adding work to it?
> 
> Yeah I guess so - it would only matter if they add work in one
> of the callbacks we use to remove keys, interfaces, etc.   This
> might be a good case for the in_suspend flag.  Or, we could just 
> flush the workqueue again at the end of suspend to make sure 
> nothing was added.

True, just flushing it again is probably better. Best do it before the
->stop() call so the driver doesn't try to bang dead hardware... just in
case it's broken and assumes work structs will run before ->stop().

> The other thing I don't get is why STR works fine but STD hangs,
> from my reading of stuff in kernel/power they should be the same
> with respect to order of task-freeze and suspend.  

Indeed, that is a little odd. But I think this patch is correct, and the
question is why the incorrect code doesn't break in one case. Maybe not
something to worry about too much?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]