Re: [PATCH] mac80211: fix sdata locking around __ieee80211_request_smps

[Jakub Kicinski <kubakici@wp.pl>]

On Mon,  3 Jun 2013 13:53:20 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> My cfg80211/mac80211 locking unification broke the sdata
> locking in ieee80211_set_power_mgmt, it needs to acquire
> the lock for __ieee80211_request_smps(). Add the locking.
>
> Reported-by: Jakub Kicinski <kubakici@wp.pl>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/mac80211/cfg.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 9034da1..3062210 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -2378,7 +2378,9 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
>  	local->dynamic_ps_forced_timeout = timeout;
>  
>  	/* no change, but if automatic follow powersave */
> +	sdata_lock(sdata);
>  	__ieee80211_request_smps(sdata, sdata->u.mgd.req_smps);
> +	sdata_unlock(sdata);
>  
>  	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
>  		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);

I can confirm that this patch fixes the reported warning.

I had to double check that sdata_lock takes the wdev mutex
though, because lockdep assert on cfg.c:2321 looks like this:

lockdep_assert_held(&sdata->wdev.mtx)

instead of:

sdata_assert_lock(&sdata)

just saying... :-)

 -- kuba