Re: [PATCH 2.6.31] mac80211: fix suspend

[Jan Scholz]

I applied the patch to v2.6.31-rc4 and suspend works just fine.

Thanks,
   Jan

Johannes Berg <johannes@xxxxxxxxxxxxxxxx> writes:

> Jan reported that his b43-based laptop hangs during suspend.
> The problem turned out to be mac80211 asking the driver to
> stop the hardware before removing interfaces, and interface
> removal caused b43 to touch the hardware (while down, which
> causes the hang).
>
> This patch fixes mac80211 to do reorder these operations to
> have them in the correct order -- first remove interfaces
> and then stop the hardware. Some more code is necessary to
> be able to do so in a race-free manner, in particular it is
> necessary to not process frames received during quiescing.
>
> Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13337.
>
> Reported-by: Jan Scholz <scholz@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> ---
>  net/mac80211/pm.c |   24 +++++++++++++++---------
>  net/mac80211/rx.c |   12 ++++++++++++
>  2 files changed, 27 insertions(+), 9 deletions(-)
>
> --- wireless-testing.orig/net/mac80211/pm.c	2009-07-28 17:58:11.000000000 +0200
> +++ wireless-testing/net/mac80211/pm.c	2009-07-28 18:06:25.000000000 +0200
> @@ -54,15 +54,6 @@ int __ieee80211_suspend(struct ieee80211
>  
>  	rcu_read_unlock();
>  
> -	/* flush again, in case driver queued work */
> -	flush_workqueue(local->hw.workqueue);
> -
> -	/* stop hardware - this must stop RX */
> -	if (local->open_count) {
> -		ieee80211_led_radio(local, false);
> -		drv_stop(local);
> -	}
> -
>  	/* remove STAs */
>  	spin_lock_bh(&local->sta_lock);
>  	list_for_each_entry(sta, &local->sta_list, list) {
> @@ -110,7 +101,22 @@ int __ieee80211_suspend(struct ieee80211
>  		drv_remove_interface(local, &conf);
>  	}
>  
> +	/* stop hardware - this must stop RX */
> +	if (local->open_count) {
> +		ieee80211_led_radio(local, false);
> +		drv_stop(local);
> +	}
> +
> +	/*
> +	 * flush again, in case driver queued work -- it
> +	 * shouldn't be doing (or cancel everything in the
> +	 * stop callback) that but better safe than sorry.
> +	 */
> +	flush_workqueue(local->hw.workqueue);
> +
>  	local->suspended = true;
> +	/* need suspended to be visible before quiescing is false */
> +	barrier();
>  	local->quiescing = false;
>  
>  	return 0;
> --- wireless-testing.orig/net/mac80211/rx.c	2009-07-28 17:58:52.000000000 +0200
> +++ wireless-testing/net/mac80211/rx.c	2009-07-28 18:04:24.000000000 +0200
> @@ -2441,6 +2441,18 @@ void __ieee80211_rx(struct ieee80211_hw 
>  		return;
>  	}
>  
> +	/*
> +	 * If we're suspending, it is possible although not too likely
> +	 * that we'd be receiving frames after having already partially
> +	 * quiesced the stack. We can't process such frames then since
> +	 * that might, for example, cause stations to be added or other
> +	 * driver callbacks be invoked.
> +	 */
> +	if (unlikely(local->quiescing || local->suspended)) {
> +		kfree_skb(skb);
> +		return;
> +	}
> +
>  	if (status->flag & RX_FLAG_HT) {
>  		/* rate_idx is MCS index */
>  		if (WARN_ON(status->rate_idx < 0 ||
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/