2009-04-16 11:21:08

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 0/4] mac80211 powersave work

This is pretty much unchanged from my previous v2 RFCs,
except that I added a default of 500ms for the dynamic
powersave timer.

The fourth patch is new, it adds beacon filtering to
mac80211 -- for the reasons explained in that patch.

For 2.6.31, we should make MAC80211_DEFAULT_PS default
off instead. John, how do you want to handle that?

johannes



2009-04-21 13:09:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

"John W. Linville" <[email protected]> writes:

>> Yes, in my opinion the power save should not be enabled by default for
>> the big masses, at least not yet. My understanding is that only few
>> people have tested it and I would like to see more testing first.
>>
>> But if you (Johannes&John) think it's ready for the prime time, I'm not
>> going to complain :)
>
> Well there is always a trade-off. But by the time 2.6.31 come around
> the code will have been in wireless-testing, net-next, and linux-next
> for a reasonably long time. Hopefully we will have good confidence
> in it by then. If not, then we can disable it by default later.

That sounds good. With those three trees there should be quite a lot of
testers.

--
Kalle Valo

2009-04-21 13:00:52

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Tue, Apr 21, 2009 at 08:24:28AM +0300, Kalle Valo wrote:
> Johannes Berg <[email protected]> writes:
>
> >> I'm not sure I understand. Your patch (which would go to 2.6.31)
> >> has it turned-on by default. Which way do you want it?
> >
> > I want it turned on by default, at least in wireless-testing; Kalle
> > thinks that we should turn it off for a .31 release.
> >
> > I would think that since an easy workaround is available ("iwconfig
> > wlan0 power off") and it doesn't affect most hardware yet anyway (only
> > those supporting powersave) we should turn it on by default anyway,
> > since otherwise we won't find any bugs -- but maybe that's just me.
>
> Yes, in my opinion the power save should not be enabled by default for
> the big masses, at least not yet. My understanding is that only few
> people have tested it and I would like to see more testing first.
>
> But if you (Johannes&John) think it's ready for the prime time, I'm not
> going to complain :)

Well there is always a trade-off. But by the time 2.6.31 come around
the code will have been in wireless-testing, net-next, and linux-next
for a reasonably long time. Hopefully we will have good confidence
in it by then. If not, then we can disable it by default later.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-04-20 19:45:50

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> This is pretty much unchanged from my previous v2 RFCs,
> except that I added a default of 500ms for the dynamic
> powersave timer.
>
> The fourth patch is new, it adds beacon filtering to
> mac80211 -- for the reasons explained in that patch.
>
> For 2.6.31, we should make MAC80211_DEFAULT_PS default
> off instead. John, how do you want to handle that?

I'm not sure I understand. Your patch (which would go to 2.6.31)
has it turned-on by default. Which way do you want it?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-04-21 05:24:32

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

Johannes Berg <[email protected]> writes:

>> I'm not sure I understand. Your patch (which would go to 2.6.31)
>> has it turned-on by default. Which way do you want it?
>
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
>
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (only
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.

Yes, in my opinion the power save should not be enabled by default for
the big masses, at least not yet. My understanding is that only few
people have tested it and I would like to see more testing first.

But if you (Johannes&John) think it's ready for the prime time, I'm not
going to complain :)

--
Kalle Valo

2009-04-20 21:15:50

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, Apr 20, 2009 at 09:53:08PM +0200, Johannes Berg wrote:
> On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
> > On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> > > This is pretty much unchanged from my previous v2 RFCs,
> > > except that I added a default of 500ms for the dynamic
> > > powersave timer.
> > >
> > > The fourth patch is new, it adds beacon filtering to
> > > mac80211 -- for the reasons explained in that patch.
> > >
> > > For 2.6.31, we should make MAC80211_DEFAULT_PS default
> > > off instead. John, how do you want to handle that?
> >
> > I'm not sure I understand. Your patch (which would go to 2.6.31)
> > has it turned-on by default. Which way do you want it?
>
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
>
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (only
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.

Ah, OK -- let's leave it on for now and if it is a problem we can
turn it off in 2.6.31 during/after 2.6.31-rc1.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-04-20 21:28:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, 2009-04-20 at 23:19 +0200, Davide Pesavento wrote:

> >> As I've already reported [1][2], ath9k is currently broken when power
> >> saving is enabled, at least on AR5418 hardware. So please don't turn
> >> it on by default yet.
> >
> > But if that's just a hardware/driver bug shouldn't the driver disable PS
> > for that combination? It seems wrong to disable PS in the stack by
> > default just because one driver has a problem with it, when that driver
> > could just unset the PS support bit.
> >
>
> You're right. It makes perfect sense.
> Unfortunately I don't know how to do it... :-(

Something like this.

johannes

--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/main.c 2009-04-20 23:27:15.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath/ath9k/main.c 2009-04-20 23:27:54.000000000 +0200
@@ -1537,8 +1537,6 @@ void ath_set_hw_capab(struct ath_softc *
IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_AMPDU_AGGREGATION |
- IEEE80211_HW_SUPPORTS_PS |
- IEEE80211_HW_PS_NULLFUNC_STACK |
IEEE80211_HW_SPECTRUM_MGMT;

if (AR_SREV_9160_10_OR_LATER(sc->sc_ah) || modparam_nohwcrypt)



2009-04-20 21:58:55

by Fabio Rossi

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Monday 20 April 2009, Johannes Berg wrote:

> But if that's just a hardware/driver bug shouldn't the driver disable PS
> for that combination? It seems wrong to disable PS in the stack by
> default just because one driver has a problem with it, when that driver
> could just unset the PS support bit.

I think that a printk notice message could be useful to state that the
powersave management is active (of course with DEBUG on, at least during the
first larger-user-base test period).

Fabio

2009-04-20 20:26:26

by Davide Pesavento

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, Apr 20, 2009 at 21:53, Johannes Berg <[email protected]=
> wrote:
> On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
>> On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
>> > This is pretty much unchanged from my previous v2 RFCs,
>> > except that I added a default of 500ms for the dynamic
>> > powersave timer.
>> >
>> > The fourth patch is new, it adds beacon filtering to
>> > mac80211 -- for the reasons explained in that patch.
>> >
>> > For 2.6.31, we should make MAC80211_DEFAULT_PS default
>> > off instead. John, how do you want to handle that?
>>
>> I'm not sure I understand. =C2=A0Your patch (which would go to 2.6.3=
1)
>> has it turned-on by default. =C2=A0Which way do you want it?
>
> I want it turned on by default, at least in wireless-testing; Kalle
> thinks that we should turn it off for a .31 release.
>
> I would think that since an easy workaround is available ("iwconfig
> wlan0 power off") and it doesn't affect most hardware yet anyway (onl=
y
> those supporting powersave) we should turn it on by default anyway,
> since otherwise we won't find any bugs -- but maybe that's just me.
>

As I've already reported [1][2], ath9k is currently broken when power
saving is enabled, at least on AR5418 hardware. So please don't turn
it on by default yet.

Thanks,
Davide

[1] http://marc.info/?l=3Dlinux-wireless&m=3D123816406913599&w=3D2
[2] http://marc.info/?l=3Dlinux-wireless&m=3D123999864507104&w=3D2

2009-04-20 20:31:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, 2009-04-20 at 22:26 +0200, Davide Pesavento wrote:

> As I've already reported [1][2], ath9k is currently broken when power
> saving is enabled, at least on AR5418 hardware. So please don't turn
> it on by default yet.

But if that's just a hardware/driver bug shouldn't the driver disable PS
for that combination? It seems wrong to disable PS in the stack by
default just because one driver has a problem with it, when that driver
could just unset the PS support bit.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-04-20 19:53:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, 2009-04-20 at 15:44 -0400, John W. Linville wrote:
> On Thu, Apr 16, 2009 at 01:17:23PM +0200, Johannes Berg wrote:
> > This is pretty much unchanged from my previous v2 RFCs,
> > except that I added a default of 500ms for the dynamic
> > powersave timer.
> >
> > The fourth patch is new, it adds beacon filtering to
> > mac80211 -- for the reasons explained in that patch.
> >
> > For 2.6.31, we should make MAC80211_DEFAULT_PS default
> > off instead. John, how do you want to handle that?
>
> I'm not sure I understand. Your patch (which would go to 2.6.31)
> has it turned-on by default. Which way do you want it?

I want it turned on by default, at least in wireless-testing; Kalle
thinks that we should turn it off for a .31 release.

I would think that since an easy workaround is available ("iwconfig
wlan0 power off") and it doesn't affect most hardware yet anyway (only
those supporting powersave) we should turn it on by default anyway,
since otherwise we won't find any bugs -- but maybe that's just me.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-04-20 21:20:03

by Davide Pesavento

[permalink] [raw]
Subject: Re: [PATCH 0/4] mac80211 powersave work

On Mon, Apr 20, 2009 at 22:30, Johannes Berg <[email protected]> wrote:
> On Mon, 2009-04-20 at 22:26 +0200, Davide Pesavento wrote:
>
>> As I've already reported [1][2], ath9k is currently broken when power
>> saving is enabled, at least on AR5418 hardware. So please don't turn
>> it on by default yet.
>
> But if that's just a hardware/driver bug shouldn't the driver disable PS
> for that combination? It seems wrong to disable PS in the stack by
> default just because one driver has a problem with it, when that driver
> could just unset the PS support bit.
>

You're right. It makes perfect sense.
Unfortunately I don't know how to do it... :-(

Regards,
Davide