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

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

[Johannes Berg]

On Mon, 2015-05-18 at 14:30 +0530, chaitanya.mgit@gmail.com wrote:
> From: Chaitanya T K <chaitanya.tata@imgtec.com>
> 
> 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.

Please stop resending this patch a few times a day without indicating
any changes.

I'm going to ignore it for a few days now until this settles down.

johannes

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

[Krishna Chaitanya]

On Mon, May 18, 2015 at 2:35 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Mon, 2015-05-18 at 14:30 +0530, chaitanya.mgit@gmail.com wrote:
> > From: Chaitanya T K <chaitanya.tata@imgtec.com>
> >
> > 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.
>
> Please stop resending this patch a few times a day without indicating
> any changes.
>
> I'm going to ignore it for a few days now until this settles down.

My sendmail shows status as bounced so i was trying with different
options. I did not realize that it has already made it to the list.

Apologies for the trouble Johannes,

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

[Johannes Berg]

On Mon, 2015-05-18 at 14:49 +0530, Krishna Chaitanya wrote:

> My sendmail shows status as bounced so i was trying with different
> options. I did not realize that it has already made it to the list.

It looks like it didn't actually make it to the list *today*, but I
believe there were versions that did. I got 4 of them today though ...

You can always check
https://patchwork.kernel.org/project/linux-wireless/list/

johannes

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

[Krishna Chaitanya]

On Mon, May 18, 2015 at 2:54 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2015-05-18 at 14:49 +0530, Krishna Chaitanya wrote:
>
>> My sendmail shows status as bounced so i was trying with different
>> options. I did not realize that it has already made it to the list.
>
> It looks like it didn't actually make it to the list *today*, but I
> believe there were versions that did.
Yes, my git send-email still shows it as bounced due to some wrong
from address.

> I got 4 of them today though ...
Sorry.

> You can always check
> https://patchwork.kernel.org/project/linux-wireless/list/

Thanks, will use that.

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

[Johannes Berg]

It looks like your patch never actually made it to the list - perhaps
test that before you add me to Cc :)

> 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.

I'm not sure I get it - we always transmit a frame after resume.

johannes

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

[Krishna Chaitanya]

On Tue, May 19, 2015 at 1:01 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> It looks like your patch never actually made it to the list - perhaps
> test that before you add me to Cc :)
Sure, i realized that after a few trails and then removed you from CC :-).

>> 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.
>
> I'm not sure I get it - we always transmit a frame after resume.
Let me explain the sequence of events:

tx_frame:
      dynamic_ps_disable_work.
                 get the chip out of power save.
suspend came:
     dynamic ps timer is cancelled.
    tx queues are flushed.
chip stays out of power save unless there is one more
tx frame which kicks the dynamic ps timer again.



-- 
Thanks,
Regards,
Chaitanya T K.

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

[Johannes Berg]

On Tue, 2015-05-19 at 13:53 +0530, Krishna Chaitanya wrote:

> > I'm not sure I get it - we always transmit a frame after resume.
> Let me explain the sequence of events:
> 
> tx_frame:
>       dynamic_ps_disable_work.
>                  get the chip out of power save.
> suspend came:
>      dynamic ps timer is cancelled.
>     tx queues are flushed.
> chip stays out of power save unless there is one more
> tx frame which kicks the dynamic ps timer again.

So initially, for suspend, this shouldn't matter.

And after suspend we always transmit a frame. What gives?

johannes

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

[Krishna Chaitanya]

On Wed, May 20, 2015 at 7:11 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2015-05-19 at 13:53 +0530, Krishna Chaitanya wrote:
>
>> > I'm not sure I get it - we always transmit a frame after resume.
>> Let me explain the sequence of events:
>>
>> tx_frame:
>>       dynamic_ps_disable_work.
>>                  get the chip out of power save.
>> suspend came:
>>      dynamic ps timer is cancelled.
>>     tx queues are flushed.
>> chip stays out of power save unless there is one more
>> tx frame which kicks the dynamic ps timer again.
>
> So initially, for suspend, this shouldn't matter.
Some chipsets are using power_save state from mac80211
to stay in DOZE mode, so in that case we end up
drawing more power. We can force the chipset fw to enter
DOZE independent of the mac80211 power save state but
when resuming the chip FW should again configure as per
mac80211 power save state.

> And after suspend we always transmit a frame. What gives?

Yes, but we have seen that in idle case this delay is more like
30secs-1min depending on the keepalive packet.

Till that time the chip is active and drawing more power.

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

[Chaitanya T K]

From: Chaitanya T K <chaitanya.mgit@gmail.com>

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 <chaitanya.mgit@gmail.com>
---
 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);

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

[Krishna Chaitanya]

On Sun, May 31, 2015 at 3:52 PM, Chaitanya T K <chaitanya.mgit@gmail.com> wrote:
> From: Chaitanya T K <chaitanya.mgit@gmail.com>
>
> 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 <chaitanya.mgit@gmail.com>
> ---
>  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.

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

[Johannes Berg]

On Sun, 2015-05-31 at 15:52 +0530, Chaitanya T K wrote:
> From: Chaitanya T K <chaitanya.mgit@gmail.com>
> 
> 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