* [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
@ 2011-06-18 19:32 Arik Nemtsov
2011-06-19 8:11 ` Johannes Berg
2011-06-21 4:55 ` Vivek Natarajan
0 siblings, 2 replies; 10+ messages in thread
From: Arik Nemtsov @ 2011-06-18 19:32 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Arik Nemtsov
Use the tx_frames_pending() driver callback to determine if Tx frames are
pending for its internal queues. If so postpone the dynamic PS timeout
to avoid interrupting Tx traffic.
The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
for all drivers supporting dynamic PS.
This patch helps improve performance in noisy environments.
Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
net/mac80211/mlme.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index a41f234..3a1699b 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -758,19 +758,23 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
if (local->hw.conf.flags & IEEE80211_CONF_PS)
return;
+ /* don't enter PS if dynamic PS is enabled and TX frames are pending */
+ if (local->hw.conf.dynamic_ps_timeout > 0 &&
+ !local->disable_dynamic_ps && drv_tx_frames_pending(local)) {
+ mod_timer(&local->dynamic_ps_timer, jiffies +
+ msecs_to_jiffies(
+ local->hw.conf.dynamic_ps_timeout));
+ return;
+ }
+
if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
(!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED))) {
netif_tx_stop_all_queues(sdata->dev);
- if (drv_tx_frames_pending(local))
- mod_timer(&local->dynamic_ps_timer, jiffies +
- msecs_to_jiffies(
- local->hw.conf.dynamic_ps_timeout));
- else {
- ieee80211_send_nullfunc(local, sdata, 1);
- /* Flush to get the tx status of nullfunc frame */
- drv_flush(local, false);
- }
+ ieee80211_send_nullfunc(local, sdata, 1);
+
+ /* Flush to get the tx status of nullfunc frame */
+ drv_flush(local, false);
}
if (!((local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) &&
--
1.7.4.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-18 19:32 [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending Arik Nemtsov
@ 2011-06-19 8:11 ` Johannes Berg
2011-06-20 13:35 ` John W. Linville
2011-06-21 4:55 ` Vivek Natarajan
1 sibling, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2011-06-19 8:11 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless
On Sat, 2011-06-18 at 22:32 +0300, Arik Nemtsov wrote:
> Use the tx_frames_pending() driver callback to determine if Tx frames are
> pending for its internal queues. If so postpone the dynamic PS timeout
> to avoid interrupting Tx traffic.
>
> The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
> behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
> for all drivers supporting dynamic PS.
>
> This patch helps improve performance in noisy environments.
FWIW, I don't really consider myself maintaining the PS
implementation ;-) I don't understand it and think the code is way too
complex.
I'm still not convinced I like the drv_tx_frames_pending() thing, but
then again I don't really care much anyway about PS stuff.
johannes
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---
> net/mac80211/mlme.c | 22 +++++++++++++---------
> 1 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index a41f234..3a1699b 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -758,19 +758,23 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
> if (local->hw.conf.flags & IEEE80211_CONF_PS)
> return;
>
> + /* don't enter PS if dynamic PS is enabled and TX frames are pending */
> + if (local->hw.conf.dynamic_ps_timeout > 0 &&
> + !local->disable_dynamic_ps && drv_tx_frames_pending(local)) {
> + mod_timer(&local->dynamic_ps_timer, jiffies +
> + msecs_to_jiffies(
> + local->hw.conf.dynamic_ps_timeout));
> + return;
> + }
> +
> if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
> (!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED))) {
> netif_tx_stop_all_queues(sdata->dev);
>
> - if (drv_tx_frames_pending(local))
> - mod_timer(&local->dynamic_ps_timer, jiffies +
> - msecs_to_jiffies(
> - local->hw.conf.dynamic_ps_timeout));
> - else {
> - ieee80211_send_nullfunc(local, sdata, 1);
> - /* Flush to get the tx status of nullfunc frame */
> - drv_flush(local, false);
> - }
> + ieee80211_send_nullfunc(local, sdata, 1);
> +
> + /* Flush to get the tx status of nullfunc frame */
> + drv_flush(local, false);
> }
>
> if (!((local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) &&
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-19 8:11 ` Johannes Berg
@ 2011-06-20 13:35 ` John W. Linville
2011-06-20 13:59 ` Johannes Berg
0 siblings, 1 reply; 10+ messages in thread
From: John W. Linville @ 2011-06-20 13:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: Arik Nemtsov, linux-wireless, Kalle Valo
On Sun, Jun 19, 2011 at 10:11:16AM +0200, Johannes Berg wrote:
> On Sat, 2011-06-18 at 22:32 +0300, Arik Nemtsov wrote:
> > Use the tx_frames_pending() driver callback to determine if Tx frames are
> > pending for its internal queues. If so postpone the dynamic PS timeout
> > to avoid interrupting Tx traffic.
> >
> > The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
> > behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
> > for all drivers supporting dynamic PS.
> >
> > This patch helps improve performance in noisy environments.
>
> FWIW, I don't really consider myself maintaining the PS
> implementation ;-) I don't understand it and think the code is way too
> complex.
Perhaps Kalle can comment? Or...?
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-20 13:35 ` John W. Linville
@ 2011-06-20 13:59 ` Johannes Berg
2011-06-20 14:42 ` Kalle Valo
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2011-06-20 13:59 UTC (permalink / raw)
To: John W. Linville; +Cc: Arik Nemtsov, linux-wireless, Kalle Valo
On Mon, 2011-06-20 at 09:35 -0400, John W. Linville wrote:
> On Sun, Jun 19, 2011 at 10:11:16AM +0200, Johannes Berg wrote:
> > On Sat, 2011-06-18 at 22:32 +0300, Arik Nemtsov wrote:
> > > Use the tx_frames_pending() driver callback to determine if Tx frames are
> > > pending for its internal queues. If so postpone the dynamic PS timeout
> > > to avoid interrupting Tx traffic.
> > >
> > > The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
> > > behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
> > > for all drivers supporting dynamic PS.
> > >
> > > This patch helps improve performance in noisy environments.
> >
> > FWIW, I don't really consider myself maintaining the PS
> > implementation ;-) I don't understand it and think the code is way too
> > complex.
>
> Perhaps Kalle can comment? Or...?
Yeah he's a good candidate :)
johannes
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-20 13:59 ` Johannes Berg
@ 2011-06-20 14:42 ` Kalle Valo
2011-06-20 17:28 ` Sam Leffler
0 siblings, 1 reply; 10+ messages in thread
From: Kalle Valo @ 2011-06-20 14:42 UTC (permalink / raw)
To: Johannes Berg; +Cc: John W. Linville, Arik Nemtsov, linux-wireless
Johannes Berg <johannes@sipsolutions.net> writes:
>> > FWIW, I don't really consider myself maintaining the PS
>> > implementation ;-) I don't understand it and think the code is way too
>> > complex.
>>
>> Perhaps Kalle can comment? Or...?
>
> Yeah he's a good candidate :)
Hehe :)
But I agree with Johannes, the powersave in mac80211 is pretty
complex. I lost track of it a long time ago so it's pretty difficult
for me to comment about the patch.
--
Kalle Valo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-20 14:42 ` Kalle Valo
@ 2011-06-20 17:28 ` Sam Leffler
0 siblings, 0 replies; 10+ messages in thread
From: Sam Leffler @ 2011-06-20 17:28 UTC (permalink / raw)
To: Kalle Valo; +Cc: Johannes Berg, John W. Linville, Arik Nemtsov, linux-wireless
On Mon, Jun 20, 2011 at 7:42 AM, Kalle Valo <kvalo@adurom.com> wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
>
>>> > FWIW, I don't really consider myself maintaining the PS
>>> > implementation ;-) I don't understand it and think the code is way too
>>> > complex.
>>>
>>> Perhaps Kalle can comment? Or...?
>>
>> Yeah he's a good candidate :)
>
> Hehe :)
>
> But I agree with Johannes, the powersave in mac80211 is pretty
> complex. I lost track of it a long time ago so it's pretty difficult
> for me to comment about the patch.
I believe I've hit this situation (and have changes of my own to do
something similar but different); will review.
-Sam
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-18 19:32 [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending Arik Nemtsov
2011-06-19 8:11 ` Johannes Berg
@ 2011-06-21 4:55 ` Vivek Natarajan
2011-06-21 5:43 ` Arik Nemtsov
1 sibling, 1 reply; 10+ messages in thread
From: Vivek Natarajan @ 2011-06-21 4:55 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless, Johannes Berg
On Sun, Jun 19, 2011 at 1:02 AM, Arik Nemtsov <arik@wizery.com> wrote:
> Use the tx_frames_pending() driver callback to determine if Tx frames are
> pending for its internal queues. If so postpone the dynamic PS timeout
> to avoid interrupting Tx traffic.
>
> The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
> behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
> for all drivers supporting dynamic PS.
>
> This patch helps improve performance in noisy environments.
>
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
> ---
> net/mac80211/mlme.c | 22 +++++++++++++---------
> 1 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index a41f234..3a1699b 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -758,19 +758,23 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
> if (local->hw.conf.flags & IEEE80211_CONF_PS)
> return;
>
> + /* don't enter PS if dynamic PS is enabled and TX frames are pending */
> + if (local->hw.conf.dynamic_ps_timeout > 0 &&
> + !local->disable_dynamic_ps && drv_tx_frames_pending(local)) {
netif_tx_stop_all_queues has to be called before checking
frames_pending(), so that we can make sure no data is transmitted in
the time between checking frames_pending and transmitting null data
frames. If it is not done, this might cause some power save states
mismatch between the AP and the station in some corner cases.
Vivek.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-21 4:55 ` Vivek Natarajan
@ 2011-06-21 5:43 ` Arik Nemtsov
2011-06-21 6:33 ` Vivek Natarajan
0 siblings, 1 reply; 10+ messages in thread
From: Arik Nemtsov @ 2011-06-21 5:43 UTC (permalink / raw)
To: Vivek Natarajan; +Cc: linux-wireless, Johannes Berg, Sam Leffler
On Tue, Jun 21, 2011 at 07:55, Vivek Natarajan <vivek.natraj@gmail.com> wrote:
> On Sun, Jun 19, 2011 at 1:02 AM, Arik Nemtsov <arik@wizery.com> wrote:
>> Use the tx_frames_pending() driver callback to determine if Tx frames are
>> pending for its internal queues. If so postpone the dynamic PS timeout
>> to avoid interrupting Tx traffic.
>>
>> The commit e8306f989483e4b97a8b37dd268de6c8c6f35e75 enabled this
>> behavior for drivers with IEEE80211_HW_PS_NULLFUNC_STACK. We enable this
>> for all drivers supporting dynamic PS.
>>
>> This patch helps improve performance in noisy environments.
>>
>> Signed-off-by: Arik Nemtsov <arik@wizery.com>
>> ---
>> net/mac80211/mlme.c | 22 +++++++++++++---------
>> 1 files changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index a41f234..3a1699b 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -758,19 +758,23 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
>> if (local->hw.conf.flags & IEEE80211_CONF_PS)
>> return;
>>
>> + /* don't enter PS if dynamic PS is enabled and TX frames are pending */
>> + if (local->hw.conf.dynamic_ps_timeout > 0 &&
>> + !local->disable_dynamic_ps && drv_tx_frames_pending(local)) {
>
> netif_tx_stop_all_queues has to be called before checking
> frames_pending(), so that we can make sure no data is transmitted in
> the time between checking frames_pending and transmitting null data
> frames. If it is not done, this might cause some power save states
> mismatch between the AP and the station in some corner cases.
>
> Vivek.
>
That's a good point. But I guess its only relevant to drivers using
IEEE80211_HW_PS_NULLFUNC_STACK.
Still seems racy to me - even if queues are stopped it doesn't mean
mac80211 is not in the middle of ieee80211_tx(). Is there a way to
make sure?
Arik
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-21 5:43 ` Arik Nemtsov
@ 2011-06-21 6:33 ` Vivek Natarajan
2011-06-22 8:58 ` Arik Nemtsov
0 siblings, 1 reply; 10+ messages in thread
From: Vivek Natarajan @ 2011-06-21 6:33 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless, Johannes Berg, Sam Leffler
On Tue, Jun 21, 2011 at 11:13 AM, Arik Nemtsov <arik@wizery.com> wrote:
> On Tue, Jun 21, 2011 at 07:55, Vivek Natarajan <vivek.natraj@gmail.com> wrote:
>>>
>>> + /* don't enter PS if dynamic PS is enabled and TX frames are pending */
>>> + if (local->hw.conf.dynamic_ps_timeout > 0 &&
>>> + !local->disable_dynamic_ps && drv_tx_frames_pending(local)) {
>>
>> netif_tx_stop_all_queues has to be called before checking
>> frames_pending(), so that we can make sure no data is transmitted in
>> the time between checking frames_pending and transmitting null data
>> frames. If it is not done, this might cause some power save states
>> mismatch between the AP and the station in some corner cases.
>
> That's a good point. But I guess its only relevant to drivers using
> IEEE80211_HW_PS_NULLFUNC_STACK.
> Still seems racy to me - even if queues are stopped it doesn't mean
> mac80211 is not in the middle of ieee80211_tx(). Is there a way to
> make sure?
Well, I can't find any way to stop this race. :)
Atleast the other race could be fixed with stop_queues.
How about the below patch? I am not sure about the disable_dynamic_ps
and its BT/WLAN antenna sharing. Please add that if you see it as
appropriate.
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index faca503..67ecad1 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -778,15 +778,14 @@ void ieee80211_dynamic_ps_enable_work(struct
work_struct *work)
}
spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
- if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
- (!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED))) {
+ if (!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
netif_tx_stop_all_queues(sdata->dev);
if (drv_tx_frames_pending(local))
mod_timer(&local->dynamic_ps_timer, jiffies +
msecs_to_jiffies(
local->hw.conf.dynamic_ps_timeout));
- else {
+ else if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
ieee80211_send_nullfunc(local, sdata, 1);
/* Flush to get the tx status of nullfunc frame */
drv_flush(local, false);
Vivek.
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending
2011-06-21 6:33 ` Vivek Natarajan
@ 2011-06-22 8:58 ` Arik Nemtsov
0 siblings, 0 replies; 10+ messages in thread
From: Arik Nemtsov @ 2011-06-22 8:58 UTC (permalink / raw)
To: Vivek Natarajan; +Cc: linux-wireless, Johannes Berg, Sam Leffler
On Tue, Jun 21, 2011 at 09:33, Vivek Natarajan <vivek.natraj@gmail.com> wrote:
>
> Well, I can't find any way to stop this race. :)
> Atleast the other race could be fixed with stop_queues.
Sure I'll add a stop_queues for for the IEEE80211_HW_PS_NULLFUNC_STACK
case. If you can't hit the other race I guess it doesn't matter. It's
a corner case anyway.
> How about the below patch? I am not sure about the disable_dynamic_ps
> and its BT/WLAN antenna sharing. Please add that if you see it as
> appropriate.
I think the disable_dynamic_ps and peers are needed. Otherwise we risk
getting inconsistent PS notifications from mac80211 when dyn_ps is
disabled.
>
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index faca503..67ecad1 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -778,15 +778,14 @@ void ieee80211_dynamic_ps_enable_work(struct
> work_struct *work)
> }
> spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
>
> - if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
> - (!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED))) {
> + if (!(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
> netif_tx_stop_all_queues(sdata->dev);
>
> if (drv_tx_frames_pending(local))
> mod_timer(&local->dynamic_ps_timer, jiffies +
> msecs_to_jiffies(
> local->hw.conf.dynamic_ps_timeout));
> - else {
> + else if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
> ieee80211_send_nullfunc(local, sdata, 1);
> /* Flush to get the tx status of nullfunc frame */
> drv_flush(local, false);
>
Well drivers without IEEE80211_HW_PS_NULLFUNC_STACK need to leave the
function after mod_timer(). Otherwise we end up changing the PS state
anyway.
But yea, basically its very similar to my patch.
I'll submit a v2 with the stop_queues pending Sam's review.
Arik
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-06-22 8:59 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-18 19:32 [RFC] mac80211: dynamic PS - don't enter PS when TX frames are pending Arik Nemtsov
2011-06-19 8:11 ` Johannes Berg
2011-06-20 13:35 ` John W. Linville
2011-06-20 13:59 ` Johannes Berg
2011-06-20 14:42 ` Kalle Valo
2011-06-20 17:28 ` Sam Leffler
2011-06-21 4:55 ` Vivek Natarajan
2011-06-21 5:43 ` Arik Nemtsov
2011-06-21 6:33 ` Vivek Natarajan
2011-06-22 8:58 ` Arik Nemtsov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.