2008-09-06 21:51:22

by Larry Finger

[permalink] [raw]
Subject: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

A coding error present since b43legacy was incorporated into the
kernel has prevented the driver from using the rate-setting mechanism
of mac80211. The driver has been forced to remain at a 1 Mb/s rate.

Signed-off-by: Larry Finger <[email protected]>
Cc: Stable <[email protected]> [2.6.26], [2.6.25]
---

John,

This is a bug, not a regression. I guess under the new rules that it
is 2.6.28 material.

Thanks,

Larry
---

Index: wireless-testing/drivers/net/wireless/b43legacy/xmit.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/xmit.c
+++ wireless-testing/drivers/net/wireless/b43legacy/xmit.c
@@ -624,7 +624,7 @@ void b43legacy_handle_hwtxstatus(struct
tmp = hw->count;
status.frame_count = (tmp >> 4);
status.rts_count = (tmp & 0x0F);
- tmp = hw->flags;
+ tmp = hw->flags << 1;
status.supp_reason = ((tmp & 0x1C) >> 2);
status.pm_indicated = !!(tmp & 0x80);
status.intermediate = !!(tmp & 0x40);


2008-09-08 21:00:34

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Mon, Sep 08, 2008 at 11:22:43AM -0500, Larry Finger wrote:
> Greg KH wrote:
>>
>> Bug fixes, not new features, it's pretty simple :)
>
> Just bug fixes, or does it have to be a regression?

As I understand it, the rule is more like "bug fixes that are committed
in the linux-2.6 tree". Since Linus has become more strict about
requiring "regressions only" after the merge window, that effectively
enforces the "regressions only" rule on the -stable trees as well.

John
--
John W. Linville
[email protected]

2008-09-10 05:25:26

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Mon, Sep 08, 2008 at 11:22:43AM -0500, Larry Finger wrote:
> Greg KH wrote:
>> Bug fixes, not new features, it's pretty simple :)
>
> Just bug fixes, or does it have to be a regression?

See the detailed rules in Documentation/stable_kernel_rules.txt

thanks,

greg k-h

2008-09-08 16:22:51

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

Greg KH wrote:
>
> Bug fixes, not new features, it's pretty simple :)

Just bug fixes, or does it have to be a regression?

Larry


2008-09-10 13:21:25

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Wed, Sep 10, 2008 at 12:40:44AM -0600, Otto Solares wrote:
> On Mon, Sep 08, 2008 at 04:54:59PM -0400, John W. Linville wrote:
> > On Mon, Sep 08, 2008 at 11:22:43AM -0500, Larry Finger wrote:
> > > Greg KH wrote:
> > >>
> > >> Bug fixes, not new features, it's pretty simple :)
> > >
> > > Just bug fixes, or does it have to be a regression?
> >
> > As I understand it, the rule is more like "bug fixes that are committed
> > in the linux-2.6 tree". Since Linus has become more strict about
> > requiring "regressions only" after the merge window, that effectively
> > enforces the "regressions only" rule on the -stable trees as well.
>
> In this case that rule is harming, is not idiotic to not accept bug
> fixes early or later?

I'm just the messenger...FWIW the argument is that even a "fix"
can introduce a new "bug" somewhere else, often quite unexpectedly.

John
--
John W. Linville
[email protected]

2008-09-08 07:52:53

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Sun, Sep 07, 2008 at 12:40:04AM +0200, Michael Buesch wrote:
> On Saturday 06 September 2008 23:51:22 Larry Finger wrote:
> > A coding error present since b43legacy was incorporated into the
> > kernel has prevented the driver from using the rate-setting mechanism
> > of mac80211. The driver has been forced to remain at a 1 Mb/s rate.
> >
> > Signed-off-by: Larry Finger <[email protected]>
> > Cc: Stable <[email protected]> [2.6.26], [2.6.25]
>
> Reviewed-by: Michael Buesch <[email protected]>
>
> > ---
> >
> > John,
> >
> > This is a bug, not a regression. I guess under the new rules that it
> > is 2.6.28 material.
>
> I wonder what the -stable rules are.

Bug fixes, not new features, it's pretty simple :)

> It seems really screwed to me to avoid applying the fix to 2.6.27,
> but still apply it to 2.6.25/26-stable.

I have to wait until it is in upstream before adding it to the -stable
tree, so this will not happen.

> I'd like to see this patch in .25, .26 and .27, however I'm not sure
> what the rules are.
>
> For reference:
> Three people agree on the correctness of the patch, it's a oneliner
> and it's tested to fix the bug. However it is not a regression. The
> bug is in there since day-0 of b43legacy. But this fix improves
> TX rates a lot.

If it goes into .27, please forward it on to [email protected] so we can
include it in older releases.

thanks,

greg k-h

2008-09-10 06:51:18

by Otto Solares

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Mon, Sep 08, 2008 at 04:54:59PM -0400, John W. Linville wrote:
> On Mon, Sep 08, 2008 at 11:22:43AM -0500, Larry Finger wrote:
> > Greg KH wrote:
> >>
> >> Bug fixes, not new features, it's pretty simple :)
> >
> > Just bug fixes, or does it have to be a regression?
>
> As I understand it, the rule is more like "bug fixes that are committed
> in the linux-2.6 tree". Since Linus has become more strict about
> requiring "regressions only" after the merge window, that effectively
> enforces the "regressions only" rule on the -stable trees as well.

In this case that rule is harming, is not idiotic to not accept bug
fixes early or later?

-otto

2008-09-06 22:41:01

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Saturday 06 September 2008 23:51:22 Larry Finger wrote:
> A coding error present since b43legacy was incorporated into the
> kernel has prevented the driver from using the rate-setting mechanism
> of mac80211. The driver has been forced to remain at a 1 Mb/s rate.
>
> Signed-off-by: Larry Finger <[email protected]>
> Cc: Stable <[email protected]> [2.6.26], [2.6.25]

Reviewed-by: Michael Buesch <[email protected]>

> ---
>
> John,
>
> This is a bug, not a regression. I guess under the new rules that it
> is 2.6.28 material.

I wonder what the -stable rules are.
It seems really screwed to me to avoid applying the fix to 2.6.27,
but still apply it to 2.6.25/26-stable.

I'd like to see this patch in .25, .26 and .27, however I'm not sure
what the rules are.

For reference:
Three people agree on the correctness of the patch, it's a oneliner
and it's tested to fix the bug. However it is not a regression. The
bug is in there since day-0 of b43legacy. But this fix improves
TX rates a lot.

> Index: wireless-testing/drivers/net/wireless/b43legacy/xmit.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/b43legacy/xmit.c
> +++ wireless-testing/drivers/net/wireless/b43legacy/xmit.c
> @@ -624,7 +624,7 @@ void b43legacy_handle_hwtxstatus(struct
> tmp = hw->count;
> status.frame_count = (tmp >> 4);
> status.rts_count = (tmp & 0x0F);
> - tmp = hw->flags;
> + tmp = hw->flags << 1;
> status.supp_reason = ((tmp & 0x1C) >> 2);
> status.pm_indicated = !!(tmp & 0x80);
> status.intermediate = !!(tmp & 0x40);

--
Greetings Michael.

2008-09-08 12:54:29

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH V2] b43legacy: Fix failure in rate-adjustment mechanism

On Monday 08 September 2008 07:31:55 Greg KH wrote:
> On Sun, Sep 07, 2008 at 12:40:04AM +0200, Michael Buesch wrote:
> > On Saturday 06 September 2008 23:51:22 Larry Finger wrote:
> > > A coding error present since b43legacy was incorporated into the
> > > kernel has prevented the driver from using the rate-setting mechanism
> > > of mac80211. The driver has been forced to remain at a 1 Mb/s rate.
> > >
> > > Signed-off-by: Larry Finger <[email protected]>
> > > Cc: Stable <[email protected]> [2.6.26], [2.6.25]
> >
> > Reviewed-by: Michael Buesch <[email protected]>
> >
> > > ---
> > >
> > > John,
> > >
> > > This is a bug, not a regression. I guess under the new rules that it
> > > is 2.6.28 material.
> >
> > I wonder what the -stable rules are.
>
> Bug fixes, not new features, it's pretty simple :)

Yeah, well. My concern was:
- We put it into .25-stable and .26-stable, because it's a bugfix.
That's perfectly fine.
- We do _not_ put it into .27 mainline, because it is just a simple
bug, but not a regression. Instead we wait for .27-stable.

Doesn't make a lot sense to me. :)

So I'd like to see it in .27, too, despite the strict regressions-only
rule. A general regressions-only rule simply doesn't always make sense, IMO.

--
Greetings Michael.