[PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[Saravanan Dhanabal]

After last transmission, the device goes to sleep earlier
than the configured dynamic power save timeout. If timeout
is set to 400ms, device enters into sleep mode at around
330ms since from last TX.

This patch re-schedules dynamic power save timer
after PSM is disabled, which ensures timeout
period starts only after wakeup and the timeout
variation is around 2-3ms.

Signed-off-by: Saravanan Dhanabal <ext-saravanan.dhanabal@nokia.com>
Reviewed-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
---
 net/mac80211/mlme.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 86c6ad1..f7f3f61 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -537,6 +537,8 @@ void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
 
 	ieee80211_wake_queues_by_reason(&local->hw,
 					IEEE80211_QUEUE_STOP_REASON_PS);
+	mod_timer(&local->dynamic_ps_timer, jiffies +
+		  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 }
 
 void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
-- 
1.7.0

Re: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[Johannes Berg]

On Tue, 2010-03-16 at 11:31 -0400, Saravanan Dhanabal wrote:
> After last transmission, the device goes to sleep earlier
> than the configured dynamic power save timeout. If timeout
> is set to 400ms, device enters into sleep mode at around
> 330ms since from last TX.

So that means the scheduling to the workqueue takes you 70ms?

johannes

RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[ext-saravanan.dhanabal]

In worst case it takes up to that much,  though the average is less but still significant. 

regards
saravanan 
________________________________________
From: ext Johannes Berg [johannes@sipsolutions.net]
Sent: Tuesday, March 16, 2010 11:42 PM
To: Dhanabal Saravanan (EXT-Sasken/Tampere)
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

On Tue, 2010-03-16 at 11:31 -0400, Saravanan Dhanabal wrote:
> After last transmission, the device goes to sleep earlier
> than the configured dynamic power save timeout. If timeout
> is set to 400ms, device enters into sleep mode at around
> 330ms since from last TX.

So that means the scheduling to the workqueue takes you 70ms?

johannes

RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[Johannes Berg]

On Wed, 2010-03-17 at 12:09 +0100, ext-saravanan.dhanabal@nokia.com
wrote:
> In worst case it takes up to that much,  though the average is less but still significant. 

> On Tue, 2010-03-16 at 11:31 -0400, Saravanan Dhanabal wrote:
> > After last transmission, the device goes to sleep earlier
> > than the configured dynamic power save timeout. If timeout
> > is set to 400ms, device enters into sleep mode at around
> > 330ms since from last TX.

I still don't think I understand this patch. Why would you be interested
in the exact timing rather than the time since the transmission? I
mean ... why does it go to sleep _earlier_?

johannes

Re: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[John W. Linville]

On Wed, Mar 17, 2010 at 09:01:08AM -0700, Johannes Berg wrote:
> On Wed, 2010-03-17 at 12:09 +0100, ext-saravanan.dhanabal@nokia.com
> wrote:
> > In worst case it takes up to that much,  though the average is less but still significant. 
> 
> > On Tue, 2010-03-16 at 11:31 -0400, Saravanan Dhanabal wrote:
> > > After last transmission, the device goes to sleep earlier
> > > than the configured dynamic power save timeout. If timeout
> > > is set to 400ms, device enters into sleep mode at around
> > > 330ms since from last TX.
> 
> I still don't think I understand this patch. Why would you be interested
> in the exact timing rather than the time since the transmission? I
> mean ... why does it go to sleep _earlier_?

Any further comments on this one?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[ext-saravanan.dhanabal]

Sorry for the delay, we were checking this in driver also. 

The issue was due to a delay in workqueue scheduling,  caused by msleep() inside driver. Because of this, 
last transmission happens long after the dynamic power save timer is scheduled. So, the difference 
between last transmission time and the time it goes to sleep is less than the defined PSM timeout period.

I think that this patch might be considered as a workaround for such delays caused by driver as long as it not creating
any other issues.

saravanan.

________________________________________
From: ext John W. Linville [linville@tuxdriver.com]
Sent: Monday, March 29, 2010 8:55 PM
To: Johannes Berg
Cc: Dhanabal Saravanan (EXT-Sasken/Tampere); linux-wireless@vger.kernel.org
Subject: Re: [PATCH] mac80211: Reschedule dynamic power save timer after        wakeup

On Wed, Mar 17, 2010 at 09:01:08AM -0700, Johannes Berg wrote:
> On Wed, 2010-03-17 at 12:09 +0100, ext-saravanan.dhanabal@nokia.com
> wrote:
> > In worst case it takes up to that much,  though the average is less but still significant.
>
> > On Tue, 2010-03-16 at 11:31 -0400, Saravanan Dhanabal wrote:
> > > After last transmission, the device goes to sleep earlier
> > > than the configured dynamic power save timeout. If timeout
> > > is set to 400ms, device enters into sleep mode at around
> > > 330ms since from last TX.
>
> I still don't think I understand this patch. Why would you be interested
> in the exact timing rather than the time since the transmission? I
> mean ... why does it go to sleep _earlier_?

Any further comments on this one?

John
--
John W. Linville                Someday the world will need a hero, and you
linville@tuxdriver.com                  might be all we have.  Be ready.

RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[Johannes Berg]

On Mon, 2010-03-29 at 23:15 +0200, ext-saravanan.dhanabal@nokia.com
wrote:
> Sorry for the delay, we were checking this in driver also. 
> 
> The issue was due to a delay in workqueue scheduling,  caused by
> msleep() inside driver. Because of this, 
> last transmission happens long after the dynamic power save timer is
> scheduled. So, the difference 
> between last transmission time and the time it goes to sleep is less
> than the defined PSM timeout period.
> 
> I think that this patch might be considered as a workaround for such
> delays caused by driver as long as it not creating
> any other issues.

At the very least that needs to be very clearly documented in the patch,
but I'm not convinced we should be adding it. Why does the driver need
the delay, and could it maybe do it after TX? And if the driver sleeps
during the TX path how can it possibly get good performance??

johannes

RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[ext-saravanan.dhanabal]

I agree that this is totally a driver issue. When sending the patch
we did not notice that. We tried some workaround inside driver, but 
still causes around 10-15ms variation, probably due to ELP wakeup delay.

I guess this patch could be ignored at the moment. Thanks.

saravanan d.
________________________________________
From: ext Johannes Berg [johannes@sipsolutions.net]
Sent: Tuesday, March 30, 2010 8:33 AM
To: Dhanabal Saravanan (EXT-Sasken/Tampere)
Cc: linville@tuxdriver.com; linux-wireless@vger.kernel.org
Subject: RE: [PATCH] mac80211: Reschedule dynamic power save timer after wakeup

On Mon, 2010-03-29 at 23:15 +0200, ext-saravanan.dhanabal@nokia.com
wrote:
> Sorry for the delay, we were checking this in driver also.
>
> The issue was due to a delay in workqueue scheduling,  caused by
> msleep() inside driver. Because of this,
> last transmission happens long after the dynamic power save timer is
> scheduled. So, the difference
> between last transmission time and the time it goes to sleep is less
> than the defined PSM timeout period.
>
> I think that this patch might be considered as a workaround for such
> delays caused by driver as long as it not creating
> any other issues.

At the very least that needs to be very clearly documented in the patch,
but I'm not convinced we should be adding it. Why does the driver need
the delay, and could it maybe do it after TX? And if the driver sleeps
during the TX path how can it possibly get good performance??

johannes

[PATCH] mac80211: Reschedule dynamic power save timer after wakeup

[Saravanan Dhanabal]

After last transmission, the device goes to sleep earlier
than the configured dynamic power save timeout. If timeout
is set to 400ms, device enters into sleep mode at around
330ms since from last TX.

This patch re-schedules dynamic power save timer
after PSM is disabled, which ensures timeout
period starts only after wakeup and the timeout
variation is around 2-3ms.

Signed-off-by: Saravanan Dhanabal <ext-saravanan.dhanabal@nokia.com>
Reviewed-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
---
 net/mac80211/mlme.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 86c6ad1..f7f3f61 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -537,6 +537,8 @@ void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
 
 	ieee80211_wake_queues_by_reason(&local->hw,
 					IEEE80211_QUEUE_STOP_REASON_PS);
+	mod_timer(&local->dynamic_ps_timer, jiffies +
+		  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 }
 
 void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
-- 
1.7.0