These patches propose a way to adjust the dynamic ps timeout in the mac80211
based on the pm-qos network latency value. This allows user space to influence
the level of power saving performed by the mac80211.
These patches will the current 100ms timeout for the default network
latency value (2000s), so that for non-adjusted pm-qos the functionality will
remain as it is currently. Dynamic ps is disabled for any larger latency
values, and a timeout of 300s is used for latency values smaller than 50ms.
As wext still has an interface to adjust the dynamic ps timeout from userspace,
the wext configuration will override the above, to remain backwards
compatible.
Comments are appreciated.
Juuso Oikarinen (2):
mac80211: Determine dynamic PS timeout based on ps-qos network
latency
cfg80211: Remove default dynamic PS timeout value
include/net/mac80211.h | 5 ++++-
net/mac80211/cfg.c | 4 ++--
net/mac80211/main.c | 2 ++
net/mac80211/mlme.c | 12 ++++++++++++
net/wireless/core.c | 3 ++-
5 files changed, 22 insertions(+), 4 deletions(-)
Hi,
On Fri, 2010-04-16 at 07:17 +0200, ext Kalle Valo wrote:
> Juuso Oikarinen <[email protected]> writes:
>
> > Determine the dynamic PS timeout based on the configured ps-qos network
> > latency. For backwards wext compatibility, allow the dynamic PS timeout
> > configured by the cfg80211 to overrule the automatically determined value.
>
> Thanks. I took a quick look, but I haven't tested this yet myself.
>
> > + timeout = local->hw.conf.dynamic_ps_forced_timeout;
> > + if (timeout < 0) {
> > + if (latency <= 50000)
> > + timeout = 300;
> > + else if (latency <= 2000000000)
> > + timeout = 100;
> > + else
> > + timeout = 0;
> > + }
> > + local->hw.conf.dynamic_ps_timeout = timeout;
>
> What will be the timeout value when there are no pm_qos requirements
> from user space applications? Do you know what will be the default
> latency value in that case?
Yes, the default value is 2000000000 (2000 seconds, ugh.) Hence the
value in the if statement for 100ms.
> I'm mostly worried about timeout 0 case, for example I suspect ath9k
> is broken with that value.
>
> Also does this patch change the default value of dynamic ps timeout?
>
No, the current default does not change. As the default value for the
latency is 2000s, we will, by default, get a timeout of 100ms.
-Juuso
Determine the dynamic PS timeout based on the configured ps-qos network
latency. For backwards wext compatibility, allow the dynamic PS timeout
configured by the cfg80211 to overrule the automatically determined value.
Signed-off-by: Juuso Oikarinen <[email protected]>
---
include/net/mac80211.h | 5 ++++-
net/mac80211/cfg.c | 4 ++--
net/mac80211/main.c | 2 ++
net/mac80211/mlme.c | 12 ++++++++++++
4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index dcf3c5f..243e4ab 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -661,6 +661,9 @@ enum ieee80211_smps_mode {
* @dynamic_ps_timeout: The dynamic powersave timeout (in ms), see the
* powersave documentation below. This variable is valid only when
* the CONF_PS flag is set.
+ * @dynamic_ps_forced_timeout: The dynamic powersave timeout (in ms) configured
+ * by cfg80211 (essentially, wext) If set, this value overrules the value
+ * chosen by mac80211 based on ps qos network latency.
*
* @power_level: requested transmit power (in dBm)
*
@@ -680,7 +683,7 @@ enum ieee80211_smps_mode {
*/
struct ieee80211_conf {
u32 flags;
- int power_level, dynamic_ps_timeout;
+ int power_level, dynamic_ps_timeout, dynamic_ps_forced_timeout;
int max_sleep_period;
u16 listen_interval;
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 7dd7cda..9a1a91c 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1388,11 +1388,11 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
return -EOPNOTSUPP;
if (enabled == sdata->u.mgd.powersave &&
- timeout == conf->dynamic_ps_timeout)
+ timeout == conf->dynamic_ps_forced_timeout)
return 0;
sdata->u.mgd.powersave = enabled;
- conf->dynamic_ps_timeout = timeout;
+ conf->dynamic_ps_forced_timeout = timeout;
/* no change, but if automatic follow powersave */
mutex_lock(&sdata->u.mgd.mtx);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 4afe851..ebcca0e 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -569,6 +569,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
local->hw.conf.listen_interval = local->hw.max_listen_interval;
+ local->hw.conf.dynamic_ps_forced_timeout = -1;
+
result = sta_info_start(local);
if (result < 0)
goto fail_sta_info;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 35d8502..6c5c034 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -476,6 +476,7 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
{
struct ieee80211_sub_if_data *sdata, *found = NULL;
int count = 0;
+ int timeout;
if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS)) {
local->ps_sdata = NULL;
@@ -509,6 +510,17 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
beaconint_us = ieee80211_tu_to_usec(
found->vif.bss_conf.beacon_int);
+ timeout = local->hw.conf.dynamic_ps_forced_timeout;
+ if (timeout < 0) {
+ if (latency <= 50000)
+ timeout = 300;
+ else if (latency <= 2000000000)
+ timeout = 100;
+ else
+ timeout = 0;
+ }
+ local->hw.conf.dynamic_ps_timeout = timeout;
+
if (beaconint_us > latency) {
local->ps_sdata = NULL;
} else {
--
1.6.3.3
Now that the mac80211 is choosing dynamic ps timeouts based on the ps-qos
network latency configuration, configure a default value of -1 as the dynamic
ps timeout in cfg80211. This value allows the mac80211 to determine the value
to be used.
Signed-off-by: Juuso Oikarinen <[email protected]>
---
net/wireless/core.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 6ac70c1..37d0e0a 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -705,7 +705,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
wdev->ps = true;
else
wdev->ps = false;
- wdev->ps_timeout = 100;
+ /* allow mac80211 to determine the timeout */
+ wdev->ps_timeout = -1;
if (rdev->ops->set_power_mgmt)
if (rdev->ops->set_power_mgmt(wdev->wiphy, dev,
wdev->ps,
--
1.6.3.3
On Fri, 2010-04-16 at 08:10 +0200, ext Kalle Valo wrote:
> Maybe at some point we can talk with QoS framework about this.
>
> >> I'm mostly worried about timeout 0 case, for example I suspect ath9k
> >> is broken with that value.
> >>
> >> Also does this patch change the default value of dynamic ps timeout?
> >>
> >
> > No, the current default does not change. As the default value for the
> > latency is 2000s, we will, by default, get a timeout of 100ms.
>
> Excellent. Thank you for explaining these.
>
> From my point of view these patches look good and I have nothing to
> complain.
You're definitely getting soft. I will stretch my luck by submitting a
v3 with one more latency range, giving a dynamic PS timeout of 50 ms.
> (I hope Juuso didn't hit his head when falling off from his chair when
> reading this.)
>
*ouch*
-Juuso
Juuso Oikarinen <[email protected]> writes:
> Determine the dynamic PS timeout based on the configured ps-qos network
> latency. For backwards wext compatibility, allow the dynamic PS timeout
> configured by the cfg80211 to overrule the automatically determined value.
Thanks. I took a quick look, but I haven't tested this yet myself.
> + timeout = local->hw.conf.dynamic_ps_forced_timeout;
> + if (timeout < 0) {
> + if (latency <= 50000)
> + timeout = 300;
> + else if (latency <= 2000000000)
> + timeout = 100;
> + else
> + timeout = 0;
> + }
> + local->hw.conf.dynamic_ps_timeout = timeout;
What will be the timeout value when there are no pm_qos requirements
from user space applications? Do you know what will be the default
latency value in that case?
I'm mostly worried about timeout 0 case, for example I suspect ath9k
is broken with that value.
Also does this patch change the default value of dynamic ps timeout?
--
Kalle Valo
Juuso Oikarinen <[email protected]> writes:
> Hi,
Moi,
>> > + timeout = local->hw.conf.dynamic_ps_forced_timeout;
>> > + if (timeout < 0) {
>> > + if (latency <= 50000)
>> > + timeout = 300;
>> > + else if (latency <= 2000000000)
>> > + timeout = 100;
>> > + else
>> > + timeout = 0;
>> > + }
>> > + local->hw.conf.dynamic_ps_timeout = timeout;
>>
>> What will be the timeout value when there are no pm_qos requirements
>> from user space applications? Do you know what will be the default
>> latency value in that case?
>
> Yes, the default value is 2000000000 (2000 seconds, ugh.) Hence the
> value in the if statement for 100ms.
Ah, now I understand better.
And ugh indeed, having a default of 2000 seconds for latency is very
awkward. I would have preferred to have a magic value denoting (like
-1 or something) that user space doesn't care. But I guess we have to
live with this :/
Maybe at some point we can talk with QoS framework about this.
>> I'm mostly worried about timeout 0 case, for example I suspect ath9k
>> is broken with that value.
>>
>> Also does this patch change the default value of dynamic ps timeout?
>>
>
> No, the current default does not change. As the default value for the
> latency is 2000s, we will, by default, get a timeout of 100ms.
Excellent. Thank you for explaining these.
>From my point of view these patches look good and I have nothing to
complain.
(I hope Juuso didn't hit his head when falling off from his chair when
reading this.)
--
Kalle Valo