Re: [PATCH 2/6] mac80211: sync suspend and stop interface

[Johannes Berg <johannes@sipsolutions.net>]

On Mon, 2013-02-25 at 12:50 +0100, Stanislaw Gruszka wrote:
> Is possible that we close interface while we are suspended, that
> results in warning like below (and some others similar):
> 
> WARNING: at net/mac80211/driver-ops.h:12 ieee80211_do_stop+0x62e/0x670 [mac80211]()
> wlan0:  Failed check-sdata-in-driver check, flags: 0x4

> @@ -834,16 +835,18 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>  
>  		skb_queue_purge(&sdata->skb_queue);
>  
> -		/*
> -		 * Free all remaining keys, there shouldn't be any,
> -		 * except maybe group keys in AP more or WDS?
> -		 */
> -		ieee80211_free_keys(sdata);
> -
>  		drv_remove_interface_debugfs(local, sdata);
>  
> -		if (going_down)
> -			drv_remove_interface(local, sdata);
> +		if (!local->suspended) {
> +			/*
> +			 * There shouldn't be any kays, sice we disconnected
> +			 * before suspend.
> +			 */
> +			WARN_ON(!list_empty(&sdata->key_list));

This is wrong -- you're now leaking the keys after a warning; freeing
the keys had nothing to do with suspend originally. You shouldn't change
this, in fact you should just move the comment & code I think.

More generally, does this really make much sense? There are other things
here, e.g. ieee80211_configure_filter(), ieee80211_recalc_ps(),
ieee80211_hw_config() and probably more that can be executed in this
function -- I don't think protecting these two calls is really
sufficient?

I think it'd be smarter to delay the down until resumed, or so.

johannes