2015-05-31 10:22:25

by Krishna Chaitanya

[permalink] [raw]
Subject: [PATCH] mac80211: Fix power save state stuck in ACTIVE caused by race between suspend and Tx packet.

From: Chaitanya T K <[email protected]>

If we receive suspend after TX path has executed
dynamic ps disable work, the driver will be in
ACTIVE state during suspend and even after it
resumes.

As before suspend all data packets are flushed
it is safe to put the driver in to sleep for
optimal power during suspend or up on resume.

Signed-off-by: Chaitanya T K <[email protected]>
---
net/mac80211/pm.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index ac6ad62..c9d71c2 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -76,6 +76,21 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
if (sdata->vif.type != NL80211_IFTYPE_STATION)
continue;
ieee80211_mgd_quiesce(sdata);
+ /* This is to handle a race where suspend
+ * is invoked after dynamic ps work disables
+ * power save due to TX. This causes the driver
+ * to be stuck in ACTIVE during suspend and
+ * after resume unless there is another TX,
+ * after which the dynamic ps puts driver
+ * back to DOZE.
+ */
+ if (sdata->u.mgd.associated &&
+ sdata->u.mgd.powersave &&
+ !(local->hw.conf.flags & IEEE80211_CONF_PS)) {
+ local->hw.conf.flags |= IEEE80211_CONF_PS;
+ ieee80211_hw_config(local,
+ IEEE80211_CONF_CHANGE_PS);
+ }
}

err = drv_suspend(local, wowlan);


2015-05-31 10:24:02

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Fix power save state stuck in ACTIVE caused by race between suspend and Tx packet.

On Sun, May 31, 2015 at 3:52 PM, Chaitanya T K <[email protected]> wrote:
> From: Chaitanya T K <[email protected]>
>
> If we receive suspend after TX path has executed
> dynamic ps disable work, the driver will be in
> ACTIVE state during suspend and even after it
> resumes.
>
> As before suspend all data packets are flushed
> it is safe to put the driver in to sleep for
> optimal power during suspend or up on resume.
>
> Signed-off-by: Chaitanya T K <[email protected]>
> ---
> net/mac80211/pm.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
> index ac6ad62..c9d71c2 100644
> --- a/net/mac80211/pm.c
> +++ b/net/mac80211/pm.c
> @@ -76,6 +76,21 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
> if (sdata->vif.type != NL80211_IFTYPE_STATION)
> continue;
> ieee80211_mgd_quiesce(sdata);
> + /* This is to handle a race where suspend
> + * is invoked after dynamic ps work disables
> + * power save due to TX. This causes the driver
> + * to be stuck in ACTIVE during suspend and
> + * after resume unless there is another TX,
> + * after which the dynamic ps puts driver
> + * back to DOZE.
> + */
> + if (sdata->u.mgd.associated &&
> + sdata->u.mgd.powersave &&
> + !(local->hw.conf.flags & IEEE80211_CONF_PS)) {
> + local->hw.conf.flags |= IEEE80211_CONF_PS;
> + ieee80211_hw_config(local,
> + IEEE80211_CONF_CHANGE_PS);
> + }
> }
>
> err = drv_suspend(local, wowlan);


Johannes,

I was final able to fix my "from" problem in git send-email.
Sorry for previous spamming. We can continue the discussion
in this thread.

Regards,
Chaitanya T K>


--
Thanks,
Regards,
Chaitanya T K.

2015-06-01 14:45:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Fix power save state stuck in ACTIVE caused by race between suspend and Tx packet.

On Sun, 2015-05-31 at 15:52 +0530, Chaitanya T K wrote:
> From: Chaitanya T K <[email protected]>
>
> If we receive suspend after TX path has executed
> dynamic ps disable work, the driver will be in
> ACTIVE state during suspend and even after it
> resumes.
>
> As before suspend all data packets are flushed
> it is safe to put the driver in to sleep for
> optimal power during suspend or up on resume.

Now that I've actually looked into the code, the patch makes some sense.

However, please rewrite the commit log and the comment in the code.

johannes