Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Dmitry Torokhov]

Hi Linus,

It looks like

commit 7d930bc33653d5592dc386a76a38f39c2e962344
Author: Johannes Berg <johannes@sipsolutions.net>
Date:   Tue Oct 20 15:08:53 2009 +0900

    cfg80211: sme: deauthenticate on assoc failure

    When the in-kernel SME gets an association failure from
    the AP we don't deauthenticate, and thus get into a very
    confused state which will lead to warnings later on. Fix
    this by actually deauthenticating when the AP indicates
    an association failure.

    (Brought to you by the hacking session at Kernel Summit 2009 in Tokyo,
    Japan. -- JWL)

    Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

is causing oops on resume:

Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588844] BUG: unable to handle kernel NULL pointer dereference at (null)
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588852] IP: [<ffffffffa0b6d4d3>] cfg80211_conn_work+0x87/0x128 [cfg80211]
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588869] PGD 110bb4067 PUD 11a605067 PMD 0
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588875] Oops: 0000 [#1] PREEMPT SMP
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588880] last sysfs file: /sys/power/state
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588884] CPU 0
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588886] Modules linked in: fuse rfcomm sco bnep l2cap crc16 sunrpc autofs4 coretemp hwmon vmnet vmblock vsock vmci vmmon acpi_cpufreq uinput arc4 ecb snd_hda_codec_idt iwl3945 joydev iwlcore snd_hda_intel firewire_ohci snd_hda_codec snd_hwdep snd_pcm ppdev mac80211 tg3 firewire_core parport_pc psmouse snd_timer yenta_socket dell_laptop parport cfg80211 dcdbas nvidia(P) wmi video ac btusb bluetooth crc_itu_t rsrc_nonstatic serio_raw snd libphy battery pcspkr output iTCO_wdt soundcore snd_page_alloc i2c_i801 i2c_core iTCO_vendor_support ohci_hcd [last unloaded: microcode]
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588957] Pid: 9, comm: events/0 Tainted: P           2.6.32-rc5 #78 Latitude D630
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588960] RIP: 0010:[<ffffffffa0b6d4d3>]  [<ffffffffa0b6d4d3>] cfg80211_conn_work+0x87/0x128 [cfg80211]
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588970] RSP: 0000:ffff88011f8afd10  EFLAGS: 00010246
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588973] RAX: 0000000000000000 RBX: ffff8801183d4810 RCX: 0000000000000000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588975] RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffff8801183d4810
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588978] RBP: ffff88011f8afd80 R08: ffff8801183d4870 R09: 0000000000000000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588981] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880118de0000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588983] R13: ffff88011f8afd40 R14: ffff880118de00f8 R15: ffff880118de0018
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588986] FS:  0000000000000000(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588989] CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588992] CR2: 0000000000000000 CR3: 000000011afe5000 CR4: 00000000000006f0
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588995] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.588997] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589001] Process events/0 (pid: 9, threadinfo ffff88011f8ae000, task ffff88011f8b8000)
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589003] Stack:
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589005]  0000000000000000 ffffffff8105b93e ffff880100000000 0000000000000046
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] <0> ffff88011f8afd50 ffff880118de0170 ffff88011f8afdf0 0000000000000246
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] <0> ffff88011f8afd60 ffff8800282169c0 ffff88011f8b8000 ffff880118de0260
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] Call Trace:
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105b93e>] ? run_workqueue+0x12f/0x26d
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105b990>] run_workqueue+0x181/0x26d
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105b93e>] ? run_workqueue+0x12f/0x26d
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffffa0b6d44c>] ? cfg80211_conn_work+0x0/0x128 [cfg80211]
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105bb5c>] worker_thread+0xe0/0xf3
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105f951>] ? autoremove_wake_function+0x0/0x34
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105ba7c>] ? worker_thread+0x0/0xf3
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105f5ed>] kthread+0x7a/0x82
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8100cc9a>] child_rip+0xa/0x20
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8100c600>] ? restore_args+0x0/0x30
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8105f573>] ? kthread+0x0/0x82
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  [<ffffffff8100cc90>] ? child_rip+0x0/0x20
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] Code: eb 7f 48 89 df e8 ba e7 ff ff 48 8b 43 20 f6 40 48 01 74 5d 83 bb fc 00 00 00 01 75 54 48 8b 83 00 01 00 00 48 89 df 48 8b 40 08 <8b> 10 41 89 55 00 66 8b 40 04 66 41 89 45 04 e8 c6 eb ff ff 85
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] RIP  [<ffffffffa0b6d4d3>] cfg80211_conn_work+0x87/0x128 [cfg80211]
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006]  RSP <ffff88011f8afd10>
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589006] CR2: 0000000000000000
Nov  2 20:24:01 dtor-d630 kernel: [ 4696.589257] ---[ end trace 7169bd444788bb37 ]---

Reverting this particular commit allows me to suspend/resume again.

Thanks!

-- 
Dmitry

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[David Miller]

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Mon, 2 Nov 2009 21:31:56 -0800

> Hi Linus,
> 
> It looks like
> 
> commit 7d930bc33653d5592dc386a76a38f39c2e962344
> Author: Johannes Berg <johannes@sipsolutions.net>
> Date:   Tue Oct 20 15:08:53 2009 +0900
 ...
> is causing oops on resume:

There is a fix for this in my tree and I'll push it to Linus
tonight.

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Dmitry Torokhov]

On Mon, Nov 02, 2009 at 10:49:57PM -0800, David Miller wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Mon, 2 Nov 2009 21:31:56 -0800
> 
> > Hi Linus,
> > 
> > It looks like
> > 
> > commit 7d930bc33653d5592dc386a76a38f39c2e962344
> > Author: Johannes Berg <johannes@sipsolutions.net>
> > Date:   Tue Oct 20 15:08:53 2009 +0900
>  ...
> > is causing oops on resume:
> 
> There is a fix for this in my tree and I'll push it to Linus
> tonight.

Ah, even better ;) Thanks David.

-- 
Dmitry

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Dmitry,

> > > It looks like
> > > 
> > > commit 7d930bc33653d5592dc386a76a38f39c2e962344
> > > Author: Johannes Berg <johannes@sipsolutions.net>
> > > Date:   Tue Oct 20 15:08:53 2009 +0900
> >  ...
> > > is causing oops on resume:
> > 
> > There is a fix for this in my tree and I'll push it to Linus
> > tonight.
> 
> Ah, even better ;) Thanks David.

and can we please stop jumping the gun here and going past the subsystem
maintainers. I think this happens a little bit too much lately.

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

On Tue, 2009-11-03 at 16:16 +0900, Marcel Holtmann wrote:

> and can we please stop jumping the gun here and going past the subsystem
> maintainers. I think this happens a little bit too much lately.

I'll rant a bit too -- I've been very annoyed by this many times. Note
this isn't really against you (Dmitry) in particular, just another
case ... but it does tick me off that many times when somebody manages
to blame a failure on a specific commit the first thing they do is ask
somebody way "above" (in terms of patch flow into mainline) the person
writing the patch (like Linus here) to revert it.

It'd help communication and be so much more friendly if the subject was
"found problem with commit ..." instead of "please consider
reverting ..." (which was comparatively friendly already!). You can even
leave the body almost identical, but I think it's presumptuous to
effectively say "hey I know the solution for the problem already". I'll
venture a guess and say that wasn't even the intent, but it certainly
comes across like that if you write an email with this subject, and
start the body with "Hi Linus," not even addressing the patch author,
just adding them to CC out of courtesy.

Should I think this is accepted practice?

</rant>

Before you get the wrong impression, yes, certainly this particular
commit was bad, and it shouldn't have happened. I tested it, but clearly
not every aspect of it. That's clearly my fault, I apologise for that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Dmitry Torokhov]

On Tue, Nov 03, 2009 at 08:44:59AM +0100, Johannes Berg wrote:
> On Tue, 2009-11-03 at 16:16 +0900, Marcel Holtmann wrote:
> 
> > and can we please stop jumping the gun here and going past the subsystem
> > maintainers. I think this happens a little bit too much lately.
> 
> I'll rant a bit too -- I've been very annoyed by this many times. Note
> this isn't really against you (Dmitry) in particular, just another
> case ... but it does tick me off that many times when somebody manages
> to blame a failure on a specific commit the first thing they do is ask
> somebody way "above" (in terms of patch flow into mainline) the person
> writing the patch (like Linus here) to revert it.
> 

I do not understand what the fuss is about. We are pretty far in release
process (rc6 is about to be cut I'd expect) and we have an issue that
for all practical purposes kills the box on resume. Yes, I want action
to be swift in this case and (unless author or maintainer - who were
CCed on the email - have otehr solutiuon) the offending commit to be
reverted. If it was rc1 or rc2 or 3 I'd feel differently.

> It'd help communication and be so much more friendly if the subject was
> "found problem with commit ..." instead of "please consider
> reverting ..." (which was comparatively friendly already!). You can even
> leave the body almost identical, but I think it's presumptuous to
> effectively say "hey I know the solution for the problem already".

Not at all. I do know the solution since reverting this commit makes by
laptop operable again. It may not be the best solution in the long run
but a solution nonetheless.

> I'll
> venture a guess and say that wasn't even the intent, but it certainly
> comes across like that if you write an email with this subject, and
> start the body with "Hi Linus," not even addressing the patch author,
> just adding them to CC out of courtesy.
> 
> Should I think this is accepted practice?
> 

Again, I do not see the problem here. We have a severe regression so
yes, I am addressing Linus and asking him to consider reverting bad
commit. I also CCing the author and the wireless maintainer so they
can object if they have alternative solution. If the problem was in a
single driver I might consider doing it differenty but I think this
commit affects many wireless drivers.

-- 
Dmitry

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Tue, 2009-11-03 at 00:22 -0800, Dmitry Torokhov wrote:

> I do not understand what the fuss is about. We are pretty far in release
> process (rc6 is about to be cut I'd expect) and we have an issue that
> for all practical purposes kills the box on resume. Yes, I want action
> to be swift in this case and (unless author or maintainer - who were
> CCed on the email - have otehr solutiuon) the offending commit to be
> reverted. If it was rc1 or rc2 or 3 I'd feel differently.

Oh, I don't disagree that swift action is good.

I just think that it's a matter of courtesy that should be independent
from the release cycle to ask the author/maintainer by default, not as a
second thought ("unless [...] have other solution"). You can always CC
Linus and ask him to revert if you don't get a response.

What's wrong with that? It doesn't actually delay the action, but it
makes the discussion much more friendly and cooperative instead of
giving the author and maintainer the feeling that their opinion only
matters as a second thought.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Dmitry Torokhov]

On Tue, Nov 03, 2009 at 09:31:01AM +0100, Johannes Berg wrote:
> On Tue, 2009-11-03 at 00:22 -0800, Dmitry Torokhov wrote:
> 
> > I do not understand what the fuss is about. We are pretty far in release
> > process (rc6 is about to be cut I'd expect) and we have an issue that
> > for all practical purposes kills the box on resume. Yes, I want action
> > to be swift in this case and (unless author or maintainer - who were
> > CCed on the email - have otehr solutiuon) the offending commit to be
> > reverted. If it was rc1 or rc2 or 3 I'd feel differently.
> 
> Oh, I don't disagree that swift action is good.
> 
> I just think that it's a matter of courtesy that should be independent
> from the release cycle to ask the author/maintainer by default, not as a
> second thought ("unless [...] have other solution"). You can always CC
> Linus and ask him to revert if you don't get a response.
> 
> What's wrong with that? It doesn't actually delay the action, but it
> makes the discussion much more friendly and cooperative instead of
> giving the author and maintainer the feeling that their opinion only
> matters as a second thought.
> 

I think you are reading too much into who was addressed directly and who
was "only" CCed... OK, next time (which I hope won't happen :) ) I'll
just address everyone directly. Will that work?

-- 
Dmitry

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Tue, 2009-11-03 at 00:47 -0800, Dmitry Torokhov wrote:

> > I just think that it's a matter of courtesy that should be independent
> > from the release cycle to ask the author/maintainer by default, not as a
> > second thought ("unless [...] have other solution"). You can always CC
> > Linus and ask him to revert if you don't get a response.
> > 
> > What's wrong with that? It doesn't actually delay the action, but it
> > makes the discussion much more friendly and cooperative instead of
> > giving the author and maintainer the feeling that their opinion only
> > matters as a second thought.
> > 
> 
> I think you are reading too much into who was addressed directly and who
> was "only" CCed... 

Maybe. But it seems to be happening pretty often recently that people
first ask for a revert and then for a fix, ignoring any thought that
might have gone into a particular commit...

> OK, next time (which I hope won't happen :) ) 

So do I! :)

> I'll just address everyone directly. Will that work?

Much better, at least for me. Hey, I try to respond quickly.

(incidentally, I can't imagine an upstream revert actually helping at
all ... that just creates a merge mess)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Tue, 3 Nov 2009, Marcel Holtmann wrote:
> 
> and can we please stop jumping the gun here and going past the subsystem
> maintainers. I think this happens a little bit too much lately.

NO!

Quite frankly, I'm very unhappy indeed with the maintainers when it comes 
to this bug:

 - it was introduced after -rc5

 - it's been bisected by multiple people 

 - I've seen one of the encounters with a person who bisected it, and the 
   author of the buggy commit just wanted "more information" after having 
   been told that small commit causes lockups.

In other words - the LAST thing we should do is to pat the subsystem 
maintainers on the back and say "good job".

The fact is, when somebody reports a major bug that is fixed by a revert, 
then I shoudl probably revert _more_ eagerly rather than less!

And subsystem maintainers should jump on it, not wait several days.

		Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Johannes,

> > > I just think that it's a matter of courtesy that should be independent
> > > from the release cycle to ask the author/maintainer by default, not as a
> > > second thought ("unless [...] have other solution"). You can always CC
> > > Linus and ask him to revert if you don't get a response.
> > > 
> > > What's wrong with that? It doesn't actually delay the action, but it
> > > makes the discussion much more friendly and cooperative instead of
> > > giving the author and maintainer the feeling that their opinion only
> > > matters as a second thought.
> > > 
> > 
> > I think you are reading too much into who was addressed directly and who
> > was "only" CCed... 
> 
> Maybe. But it seems to be happening pretty often recently that people
> first ask for a revert and then for a fix, ignoring any thought that
> might have gone into a particular commit...

I have to agree here. It happens why too often lately. And this needs to
stop. Otherwise why bother with subsystem maintainers? Just send
everything to Linus directly and have him to review every line of code.

Dmitry, this is not against you, but the proper way would have been to
just mail linux-wireless about it and you would have gotten the same
response to it than you got by including Linus and LKML. This blind CC
to LKML is not helpful. It starts confusion and just increases the load
on that mailing list. There is a reason why the MAINTAINERS file now
contains the mailing list contacts, please use them and not try to jump
over two subsystem maintainers to get something fixed. Neither Linus nor
Dave are the right people to comment on your bug.

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Tue, 3 Nov 2009, Johannes Berg wrote:
> 
> I'll rant a bit too -- I've been very annoyed by this many times. Note
> this isn't really against you (Dmitry) in particular, just another
> case ... but it does tick me off that many times when somebody manages
> to blame a failure on a specific commit the first thing they do is ask
> somebody way "above" (in terms of patch flow into mainline) the person
> writing the patch (like Linus here) to revert it.

Johannes, you're simply WRONG.

At this point (ie _way_ after -rc1), "just revert it" really is the right 
thing to do. The commit was shit. It caused more problems than it fixed. I 
should have reverted it immediately when that was clear. I didn't, and 
because I didn't, other people then had to waste time bisecting it.

So instead of complaining about other people, I would suggest you look 
yourself in the mirror. Stop thinking you "own" code. If you wrote a buggy 
commit, and somebody else went through the work to bisect it, you should 
 (a) expect it to be reverted
 (b) thank the person for finding the bug YOU introduced.
 (c) be ashamed of YOURSELF
instead of whining about it as if we should thank you!

		Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Linus,

> > and can we please stop jumping the gun here and going past the subsystem
> > maintainers. I think this happens a little bit too much lately.
> 
> NO!
> 
> Quite frankly, I'm very unhappy indeed with the maintainers when it comes 
> to this bug:
> 
>  - it was introduced after -rc5
> 
>  - it's been bisected by multiple people 
> 
>  - I've seen one of the encounters with a person who bisected it, and the 
>    author of the buggy commit just wanted "more information" after having 
>    been told that small commit causes lockups.
> 
> In other words - the LAST thing we should do is to pat the subsystem 
> maintainers on the back and say "good job".
> 
> The fact is, when somebody reports a major bug that is fixed by a revert, 
> then I shoudl probably revert _more_ eagerly rather than less!
> 
> And subsystem maintainers should jump on it, not wait several days.

no questions that it needs fixed, I agree with you. However just blindly
reverting something, because it fixes it for one or two people, might
have side effects that causes more problems than the revert would
actually fix. In this case, let at least give John or Johannes a chance
to comment on it.

I do love the fact that it gets bisected down to one particular commit.
That is great and thanks to the people who did that, but let the
subsystem maintainers know and then have them either provide a fix or
revert it by them. Sometimes it might take more than one day. And lets
be honest here, Johannes is one of the most responsive persons when it
comes to wireless bugs.

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Wed, 4 Nov 2009, Marcel Holtmann wrote:
> 
> I have to agree here. It happens why too often lately. And this needs to
> stop. Otherwise why bother with subsystem maintainers? Just send
> everything to Linus directly and have him to review every line of code.

You're full of sh*t.

Bugs are bugs. They should be reverted, and the people who introduced them 
should be SHAMED if the thing was introduced after the merge window.

I don't need to review any line of code at all - a revert is a revert. 
There's not a lot of review that needs, just a very obvious "that bug 
causes more problems than it fixed".

And yes, I'm upset, because in this case I saw one of the _earlier_ bisect 
results too, and I did actually spend time debugging it and sending 
Johannes the information, because he basically ignored the bisect result. 

That makes me upset. The fact that somebody has bisected the problem means 
that you should damn well thank them, not complain. And look at the -rc 
number, look at the commit - and you should realize that "please revert" 
is OBVIOUSLY the right thing to say to something that introduces problems 
after -rc5.

The fact is, maintainership does _not_ mean ownership. It means that you 
should be _responsible_ for the code, and you get credit for it, but if 
problems happen you do NOT "own" it. Not at all.

If you don't understand that, you shouldn't be a maintainer.

And if it's not obvious - I'm really upset that people are complaining 
about "please revert" for this case. YOU were wrong. 

		Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Wed, 4 Nov 2009, Marcel Holtmann wrote:
> 
> no questions that it needs fixed, I agree with you. However just blindly
> reverting something, because it fixes it for one or two people, might
> have side effects that causes more problems than the revert would
> actually fix.

Stop whining. Really.

Everybody understands that it should be fixed.  That's not the question.

But it should be fixed _quickly_. In this case, I have a bisection report 
FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
that piece-of-shit commit then, because I spent the time to look at the 
oops and the commit, and could tell that it was crap.

Instead, I _did_ wait for the subsystem maintainer to get around to it. As 
a result of waiting, I've now wasted time for a lot of other people.

So stop your claptrap. You're wrong. I would suggest you now thank Dmitry, 
and ask his forgiveness for (a) wasting his time and (b) then berating him 
for it.

			Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Zdenek Kabelac]

2009/11/3 Marcel Holtmann <marcel@holtmann.org>:
> Hi Linus,
>
>> > and can we please stop jumping the gun here and going past the subsystem
>> > maintainers. I think this happens a little bit too much lately.
>>
>> NO!
>>
>> Quite frankly, I'm very unhappy indeed with the maintainers when it comes
>> to this bug:
>>
>>  - it was introduced after -rc5
>>
>>  - it's been bisected by multiple people
>>
>>  - I've seen one of the encounters with a person who bisected it, and the
>>    author of the buggy commit just wanted "more information" after having
>>    been told that small commit causes lockups.
>>
>> In other words - the LAST thing we should do is to pat the subsystem
>> maintainers on the back and say "good job".
>>
>> The fact is, when somebody reports a major bug that is fixed by a revert,
>> then I shoudl probably revert _more_ eagerly rather than less!
>>
>> And subsystem maintainers should jump on it, not wait several days.
>
> no questions that it needs fixed, I agree with you. However just blindly
> reverting something, because it fixes it for one or two people, might
> have side effects that causes more problems than the revert would
> actually fix. In this case, let at least give John or Johannes a chance
> to comment on it.
>
> I do love the fact that it gets bisected down to one particular commit.
> That is great and thanks to the people who did that, but let the
> subsystem maintainers know and then have them either provide a fix or
> revert it by them. Sometimes it might take more than one day. And lets
> be honest here, Johannes is one of the most responsive persons when it
> comes to wireless bugs.
>
> Regards

Well for me the issue has been fixed by http://lkml.org/lkml/2009/10/30/68

But it was not easy to decrypt bug after resume in my case....

However doing commit of memcpy where the src could be NULL in -rc5
looks really  suspicious.

Regards

Zdenek

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Tue, 3 Nov 2009, Linus Torvalds wrote:
> 
> But it should be fixed _quickly_. In this case, I have a bisection report 
> FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
> that piece-of-shit commit then, because I spent the time to look at the 
> oops and the commit, and could tell that it was crap.

Btw, "two days" may not sound like a lot, but it's over five weeks since 
the merge window closed - we should be close to release. And the 
particular buggy commit was merged just a few days ago (Oct 29) - and it 
got some bisect loving within days of being merged.

If this had all happened during the merge window, and if the patch that 
got bisected was some complex one, my view of what "quickly" means would 
have been very different.

But this late in the -rc game, we should encourage people to expect the 
kernel to be stable. That means that if it's not stable, we should jump on 
it. As soon as possible. Quite often that should mean "revert it now, so 
that users don't have to see it, and then let the developers see if they 
can fix it properly".

Let the _maintainers_ run the buggy kernels in order to debug them, but we 
want all the _users_ to have kernels that are useful. For the last couple 
of days, we've done it the wrong way around.

			Linus

PS. The fix is pushed out now. But it could have been reverted on Sunday.

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Linus,

> > no questions that it needs fixed, I agree with you. However just blindly
> > reverting something, because it fixes it for one or two people, might
> > have side effects that causes more problems than the revert would
> > actually fix.
> 
> Stop whining. Really.
> 
> Everybody understands that it should be fixed.  That's not the question.
> 
> But it should be fixed _quickly_. In this case, I have a bisection report 
> FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
> that piece-of-shit commit then, because I spent the time to look at the 
> oops and the commit, and could tell that it was crap.
> 
> Instead, I _did_ wait for the subsystem maintainer to get around to it. As 
> a result of waiting, I've now wasted time for a lot of other people.

I do have a patch in my inbox from Johannes from 4 days ago that fixes
this issue.

	http://marc.info/?l=linux-wireless&m=125697124819563&w=2

So what is the take away from this now? Do you wanna have Johannes step
over John and Dave and send such a patch directly to you?

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Wed, 4 Nov 2009, Marcel Holtmann wrote:
>
> I do have a patch in my inbox from Johannes from 4 days ago that fixes
> this issue.
> 
> 	http://marc.info/?l=linux-wireless&m=125697124819563&w=2
> 
> So what is the take away from this now? Do you wanna have Johannes step
> over John and Dave and send such a patch directly to you?

Hell yes. If it causes lockups for people, and the original commit is 
_known_ to be buggy, these kinds of things should be expedited.

How much users time and effort do we want to waste?

And there's a secondary issue too - how comfortable do we want people to 
be to test late-in-the-game -git trees? I should hope that they should be 
considered pretty stable. And ask yourself: would it have been better to 
have had this bug in my -git tree for just one day, or for five days?

Of course, the optimal situation would have been that such a buggy commit 
wouldn't have been ever merged in the first place - at least not after 
-rc5. But notice how I'm not really complaining about that part: I'm a 
firm believer in the "bugs happen" reality, and while we should try to be 
careful, things like this _will_ slip through. 

So I'm not unhappy about the bug happening in the first place. It would 
have been better had it not, but hey, mistakes happen. We should just 
"Deal with it". 

And yes, "dealing with it" very much means by-passing maintainers if 
necessary. It can mean sending patches directly to me, but it _also_ means 
asking me to just revert a commit that turns out to be buggy and was 
merged late.

And that's what I'm really arguing for here - I don't like how you and 
Johannes were arguing against "dealing with it". As it was, we clearly had 
users wasting their time on this.

		Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Ingo Molnar]

* Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Linus,
> 
> > > no questions that it needs fixed, I agree with you. However just blindly
> > > reverting something, because it fixes it for one or two people, might
> > > have side effects that causes more problems than the revert would
> > > actually fix.
> > 
> > Stop whining. Really.
> > 
> > Everybody understands that it should be fixed.  That's not the question.
> > 
> > But it should be fixed _quickly_. In this case, I have a bisection report 
> > FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
> > that piece-of-shit commit then, because I spent the time to look at the 
> > oops and the commit, and could tell that it was crap.
> > 
> > Instead, I _did_ wait for the subsystem maintainer to get around to it. As 
> > a result of waiting, I've now wasted time for a lot of other people.
> 
> I do have a patch in my inbox from Johannes from 4 days ago that fixes 
> this issue.
> 
> 	http://marc.info/?l=linux-wireless&m=125697124819563&w=2
> 
> So what is the take away from this now? Do you wanna have Johannes 
> step over John and Dave and send such a patch directly to you?

The problem as i see it is the kind of answer Johannes gave when the bug 
was bisected to by Jeff Chua two days ago:

  Subject: wpa2 hangs v2.6.32-rc5-402-gb6727b1. Revert
           7d930bc33653d5592dc386a76a38f39c2e962344 fixed it.

 [ <1257151742.3555.165.camel@johannes.local> ]
 ...
 |
 | On Sun, 2009-11-01 at 23:18 +0800, Jeff Chua wrote:
 | > wpa2 (wpa_supplicant) hangs v2.6.32-rc5-402-gb6727b1.
 |
 | Explain?
 |
 | > Reverting 7d930bc33653d5592dc386a76a38f39c2e962344 fixes it.
 |
 | Certainly not a good idea, will break when your AP denies association.
 |
 | johannes

Unhelpful, defensive, in denial.

Plus that you tried to berate Dmitry in this particular thread about the 
revert was pretty bad form too IMO.

_Anyone_ who went through the unnecessary, avoidable cost of having to 
do a bisection of a 3 days old commit merged at around -rc5 time is in 
his full rights to ask for a revert, straight from Linus if he thinks 
so. No ifs and when about it.

So IMO you are showing the wrong kind of attitude for a post-rc5 
regression, by a _wide_ margin. The right kind of attitude would be:

  "Oops, my bad - thanks. I've queued up a revert."

or:

  "Oops, my bad - thanks. Does the attached patch fix it?
   If not we'll revert it."

Furthermore, your 'hey, nothing happened, we fixed it after all' 
argument is just a forewarning that you learned nothing and such 
avoidable incidents could repeat in the future.

	Ingo

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Tue, 3 Nov 2009, Linus Torvalds wrote:
> 
> And yes, "dealing with it" very much means by-passing maintainers if 
> necessary. It can mean sending patches directly to me, but it _also_ means 
> asking me to just revert a commit that turns out to be buggy and was 
> merged late.

By the way, the "if necessary" part obviously means that I'll be very 
happy if it all happens through maintainers. I'm absolutely _not_ arguing 
for sending patches directly to me if there is a better way of doing 
things. 

But I do think that especially as a user who finds a problem, an email 
saying "please revert" is a great thing to send to me when you've 
identified a problem - especially late in the -rc series.

Of course, you should always Cc: all the people in the patch (author _and_ 
the sign-off-chain), to give them the opportunity to ask the reporter to 
try another patch or to ask me to pull a fix.

Especially as nobody reads email 24/7 and we're all on slightly different 
clocks (even within the same timezone we have different schedules: I start 
readin mail at 7:30AM, which is probably _not_ what most techies do), so 
the optimal situation is that by the time I see the revert request, I 
_also_ have another email in my mailbox saying "oh, apply this patch 
instead".

So the different schedules _can_ be an advantage, and it's why email is 
such a great communication medium for being "immediate, but asynchronous". 
We can have overlapping work, and be very efficient when things work well.

The pessimal solution, on the other hand, is to have a very rigid 
"channel" so that we end up waiting for several people in a long chain, 
all of which are on different schedules, and so each step takes a day or 
so to percolate.

Which is what I think happened now. 

			Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Linus,

> > I do have a patch in my inbox from Johannes from 4 days ago that fixes
> > this issue.
> > 
> > 	http://marc.info/?l=linux-wireless&m=125697124819563&w=2
> > 
> > So what is the take away from this now? Do you wanna have Johannes step
> > over John and Dave and send such a patch directly to you?
> 
> Hell yes. If it causes lockups for people, and the original commit is 
> _known_ to be buggy, these kinds of things should be expedited.
> 
> How much users time and effort do we want to waste?
> 
> And there's a secondary issue too - how comfortable do we want people to 
> be to test late-in-the-game -git trees? I should hope that they should be 
> considered pretty stable. And ask yourself: would it have been better to 
> have had this bug in my -git tree for just one day, or for five days?
> 
> Of course, the optimal situation would have been that such a buggy commit 
> wouldn't have been ever merged in the first place - at least not after 
> -rc5. But notice how I'm not really complaining about that part: I'm a 
> firm believer in the "bugs happen" reality, and while we should try to be 
> careful, things like this _will_ slip through. 
> 
> So I'm not unhappy about the bug happening in the first place. It would 
> have been better had it not, but hey, mistakes happen. We should just 
> "Deal with it". 
> 
> And yes, "dealing with it" very much means by-passing maintainers if 
> necessary. It can mean sending patches directly to me, but it _also_ means 
> asking me to just revert a commit that turns out to be buggy and was 
> merged late.
> 
> And that's what I'm really arguing for here - I don't like how you and 
> Johannes were arguing against "dealing with it". As it was, we clearly had 
> users wasting their time on this.

that is a clear statement and I am perfectly fine with this.

However to be fair here, this one didn't hit everybody. I for myself
haven't seen it at all and actually I have been running this "faulty"
kernel for a while now and suspending happily multiple times a day. As
much as you hate that the patch for this bug took longer than needed to
find its way into your tree, the immediate need and the wide problem was
not clear. So there will be always cases where at the time of writing
the patch, the sensible thing to do is following the normal merge path
and the patch will end up in your tree multiple days later. It is not
optimal, but it happens.

And for the record, I think that a quick bug report to linux-wireless
would have been as effective and a request for a revert. But that is my
pure personal opinion here.

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Marcel Holtmann]

Hi Ingo,

> > > > no questions that it needs fixed, I agree with you. However just blindly
> > > > reverting something, because it fixes it for one or two people, might
> > > > have side effects that causes more problems than the revert would
> > > > actually fix.
> > > 
> > > Stop whining. Really.
> > > 
> > > Everybody understands that it should be fixed.  That's not the question.
> > > 
> > > But it should be fixed _quickly_. In this case, I have a bisection report 
> > > FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
> > > that piece-of-shit commit then, because I spent the time to look at the 
> > > oops and the commit, and could tell that it was crap.
> > > 
> > > Instead, I _did_ wait for the subsystem maintainer to get around to it. As 
> > > a result of waiting, I've now wasted time for a lot of other people.
> > 
> > I do have a patch in my inbox from Johannes from 4 days ago that fixes 
> > this issue.
> > 
> > 	http://marc.info/?l=linux-wireless&m=125697124819563&w=2
> > 
> > So what is the take away from this now? Do you wanna have Johannes 
> > step over John and Dave and send such a patch directly to you?
> 
> The problem as i see it is the kind of answer Johannes gave when the bug 
> was bisected to by Jeff Chua two days ago:
> 
>   Subject: wpa2 hangs v2.6.32-rc5-402-gb6727b1. Revert
>            7d930bc33653d5592dc386a76a38f39c2e962344 fixed it.
> 
>  [ <1257151742.3555.165.camel@johannes.local> ]
>  ...
>  |
>  | On Sun, 2009-11-01 at 23:18 +0800, Jeff Chua wrote:
>  | > wpa2 (wpa_supplicant) hangs v2.6.32-rc5-402-gb6727b1.
>  |
>  | Explain?
>  |
>  | > Reverting 7d930bc33653d5592dc386a76a38f39c2e962344 fixes it.
>  |
>  | Certainly not a good idea, will break when your AP denies association.
>  |
>  | johannes
> 
> Unhelpful, defensive, in denial.
> 
> Plus that you tried to berate Dmitry in this particular thread about the 
> revert was pretty bad form too IMO.
> 
> _Anyone_ who went through the unnecessary, avoidable cost of having to 
> do a bisection of a 3 days old commit merged at around -rc5 time is in 
> his full rights to ask for a revert, straight from Linus if he thinks 
> so. No ifs and when about it.
> 
> So IMO you are showing the wrong kind of attitude for a post-rc5 
> regression, by a _wide_ margin. The right kind of attitude would be:
> 
>   "Oops, my bad - thanks. I've queued up a revert."
> 
> or:
> 
>   "Oops, my bad - thanks. Does the attached patch fix it?
>    If not we'll revert it."
> 
> Furthermore, your 'hey, nothing happened, we fixed it after all' 
> argument is just a forewarning that you learned nothing and such 
> avoidable incidents could repeat in the future.

who said 'hey, nothing happened, we fixed it after all'. The fix for
this issue is 4 days old and was already on the way to Linus. And I
remember the first response was that this got fixed already and that the
patch is going to Linus.

Regards

Marcel

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Wed, 4 Nov 2009, Marcel Holtmann wrote:
> 
> However to be fair here, this one didn't hit everybody. I for myself
> haven't seen it at all and actually I have been running this "faulty"
> kernel for a while now and suspending happily multiple times a day. As
> much as you hate that the patch for this bug took longer than needed to
> find its way into your tree, the immediate need and the wide problem was
> not clear.

I do not agree.

A _single_ NULL pointer report should have made it clear. As mentioned, I 
was involved in one of the "please revert" threads, and I looked at the 
code, and it was obviously bogus. I could (and did) point to the exact 
memcpy() that caused the problem.

There really was no excuse. That 7d930bc33 commit had a NULL pointer 
dereference, and people hit it. End of story.

The fact that not _all_ people hit it explains how it got pulled into my 
tree in the first place, but that's it.

			Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Ingo Molnar]

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Ingo,
> 
> > > > > no questions that it needs fixed, I agree with you. However just blindly
> > > > > reverting something, because it fixes it for one or two people, might
> > > > > have side effects that causes more problems than the revert would
> > > > > actually fix.
> > > > 
> > > > Stop whining. Really.
> > > > 
> > > > Everybody understands that it should be fixed.  That's not the question.
> > > > 
> > > > But it should be fixed _quickly_. In this case, I have a bisection report 
> > > > FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting 
> > > > that piece-of-shit commit then, because I spent the time to look at the 
> > > > oops and the commit, and could tell that it was crap.
> > > > 
> > > > Instead, I _did_ wait for the subsystem maintainer to get around to it. As 
> > > > a result of waiting, I've now wasted time for a lot of other people.
> > > 
> > > I do have a patch in my inbox from Johannes from 4 days ago that fixes 
> > > this issue.
> > > 
> > > 	http://marc.info/?l=linux-wireless&m=125697124819563&w=2
> > > 
> > > So what is the take away from this now? Do you wanna have Johannes 
> > > step over John and Dave and send such a patch directly to you?
> > 
> > The problem as i see it is the kind of answer Johannes gave when the bug 
> > was bisected to by Jeff Chua two days ago:
> > 
> >   Subject: wpa2 hangs v2.6.32-rc5-402-gb6727b1. Revert
> >            7d930bc33653d5592dc386a76a38f39c2e962344 fixed it.
> > 
> >  [ <1257151742.3555.165.camel@johannes.local> ]
> >  ...
> >  |
> >  | On Sun, 2009-11-01 at 23:18 +0800, Jeff Chua wrote:
> >  | > wpa2 (wpa_supplicant) hangs v2.6.32-rc5-402-gb6727b1.
> >  |
> >  | Explain?
> >  |
> >  | > Reverting 7d930bc33653d5592dc386a76a38f39c2e962344 fixes it.
> >  |
> >  | Certainly not a good idea, will break when your AP denies association.
> >  |
> >  | johannes
> > 
> > Unhelpful, defensive, in denial.
> > 
> > Plus that you tried to berate Dmitry in this particular thread about the 
> > revert was pretty bad form too IMO.
> > 
> > _Anyone_ who went through the unnecessary, avoidable cost of having to 
> > do a bisection of a 3 days old commit merged at around -rc5 time is in 
> > his full rights to ask for a revert, straight from Linus if he thinks 
> > so. No ifs and when about it.
> > 
> > So IMO you are showing the wrong kind of attitude for a post-rc5 
> > regression, by a _wide_ margin. The right kind of attitude would be:
> > 
> >   "Oops, my bad - thanks. I've queued up a revert."
> > 
> > or:
> > 
> >   "Oops, my bad - thanks. Does the attached patch fix it?
> >    If not we'll revert it."
> > 
> > Furthermore, your 'hey, nothing happened, we fixed it after all' 
> > argument is just a forewarning that you learned nothing and such 
> > avoidable incidents could repeat in the future.
> 
> who said 'hey, nothing happened, we fixed it after all'. The fix for
> this issue is 4 days old and was already on the way to Linus. And I
> remember the first response was that this got fixed already and that the
> patch is going to Linus.

Well, what formed my opinion was the first response to Jeff Chua's 
bisection result - see it above, i quoted it in its entirety.

Thanks,

	Ingo

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Luis R. Rodriguez]

On Tue, Nov 3, 2009 at 8:29 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Marcel Holtmann <marcel@holtmann.org> wrote:
>
>> Hi Linus,
>>
>> > > no questions that it needs fixed, I agree with you. However just blindly
>> > > reverting something, because it fixes it for one or two people, might
>> > > have side effects that causes more problems than the revert would
>> > > actually fix.
>> >
>> > Stop whining. Really.
>> >
>> > Everybody understands that it should be fixed.  That's not the question.
>> >
>> > But it should be fixed _quickly_. In this case, I have a bisection report
>> > FROM TWO DAYS AGO. And I'm still kicking myself for not just reverting
>> > that piece-of-shit commit then, because I spent the time to look at the
>> > oops and the commit, and could tell that it was crap.
>> >
>> > Instead, I _did_ wait for the subsystem maintainer to get around to it. As
>> > a result of waiting, I've now wasted time for a lot of other people.
>>
>> I do have a patch in my inbox from Johannes from 4 days ago that fixes
>> this issue.
>>
>>       http://marc.info/?l=linux-wireless&m=125697124819563&w=2
>>
>> So what is the take away from this now? Do you wanna have Johannes
>> step over John and Dave and send such a patch directly to you?
>
> The problem as i see it is the kind of answer Johannes gave when the bug
> was bisected to by Jeff Chua two days ago:
>
>  Subject: wpa2 hangs v2.6.32-rc5-402-gb6727b1. Revert
>           7d930bc33653d5592dc386a76a38f39c2e962344 fixed it.
>
>  [ <1257151742.3555.165.camel@johannes.local> ]
>  ...
>  |
>  | On Sun, 2009-11-01 at 23:18 +0800, Jeff Chua wrote:
>  | > wpa2 (wpa_supplicant) hangs v2.6.32-rc5-402-gb6727b1.
>  |
>  | Explain?
>  |
>  | > Reverting 7d930bc33653d5592dc386a76a38f39c2e962344 fixes it.
>  |
>  | Certainly not a good idea, will break when your AP denies association.

It certainly would have helped to have elaborated more on this and to
be fair let me elaborate a little more on the issue which the patch in
question tried to address.

After the merge window we have been getting WARN_ON()s on an extremely
rare situation which was very difficult to debug. The issue can be
seen even on the kerneloops.org track list [1]. This also applied to
people running older kernels but running our stable compat-wireless
for 2.6.32 [2]. Only a few of us have been able to trigger this
warning and report it but it was never possible to easily reproduce.
It usually took a day's worth of connectivity to reproduce.

Late in the merge window, right before the kernel summit I found a way
to finally reproduce and sent a one line patch to fix this [3] but as
noted by Johannes it was not the proper fix for a few reasons.

The unfortunate side of the issue is that once you hit the WARN_ON()
it is already too late and will simply not be able to associate to any
AP and hence Johannes' comment. The WARN_ON() was just a secondary
warning of shit already having hit the fan elsewhere. The problem was
reproducible whenever an Access Point denied association and you
typically would not run into this unless you mis-configure your
wireless settings or you happen to have some AP in some funny
situations (I believe in my case it was -E_TOO_MANY_PEOPLE_CONNECTED).
What this means is that the issue at hand was serious and it did need
to be fixed one way or another for 2.6.32.

At the kernel summit during the hacking session I poked Johannes and
we reviewed the issue at hand and tried to come up with a proper
solution. The solution were the two sets of patches, one of which
caused a regression as noted by a few people.
 Truth is an oops is worse than a WARN_ON() and also loosing the
ability to associate to your AP. Because of that perhaps this should
have been reverted but one way or another this issue needed to be
fixed for good for 2.6.32. So unfortunately reverting the patch would
re-introduce an issue for all users.

How this sort of issue is dealt with is subjective and it is up to
maintainers to deal with.

> Unhelpful, defensive, in denial.
>
> Plus that you tried to berate Dmitry in this particular thread about the
> revert was pretty bad form too IMO.
>
> _Anyone_ who went through the unnecessary, avoidable cost of having to
> do a bisection of a 3 days old commit merged at around -rc5 time is in
> his full rights to ask for a revert, straight from Linus if he thinks
> so. No ifs and when about it.
>
> So IMO you are showing the wrong kind of attitude for a post-rc5
> regression, by a _wide_ margin. The right kind of attitude would be:
>
>  "Oops, my bad - thanks. I've queued up a revert."
>
> or:
>
>  "Oops, my bad - thanks. Does the attached patch fix it?
>   If not we'll revert it."
>
> Furthermore, your 'hey, nothing happened, we fixed it after all'
> argument is just a forewarning that you learned nothing and such
> avoidable incidents could repeat in the future.

Having more information on the patch and better communication about
the issue it solved, and the issues that reverting it would have
caused would certainly have helped maintainers make a better call at a
regression caused by it but knowing Johannes he'd probably cook up a
followup fix ASAP and that is exactly what he did.

[1] http://kerneloops.org/guilty.php?guilty=__cfg80211_disconnected&version=2.6.32-rc&start=2097152&end=2129919&class=warn
[2] http://kerneloops.org/search.php?filter=2.6.31&search=__cfg80211_disconnected
[3] http://marc.info/?l=linux-wireless&m=125548148731042&w=2

  Luis

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Linus Torvalds]

On Tue, 3 Nov 2009, Luis R. Rodriguez wrote:
> 
> How this sort of issue is dealt with is subjective and it is up to
> maintainers to deal with.

Not when they then complain when others hit the same issue several days 
later.

> Having more information on the patch and better communication about
> the issue it solved, and the issues that reverting it would have
> caused would certainly have helped maintainers make a better call at a
> regression caused by it but knowing Johannes he'd probably cook up a
> followup fix ASAP and that is exactly what he did.

He may have cooked it up, but he didn't send it to me, and he didn't even 
bother to post it as a response to people who complained about the same 
commit.

The fact that people on the wireless mailing lists may have known about 
this just makes things _worse_, I think. It shows that we really _need_ to 
go around maintainers, when not going around them seems to result in days 
of delays and total waste of time for everybody.

Btw, the reason it's likely not getting a lot of reports is not because 
people aren't hitting it, but that the symptom when you _do_ hit it tends 
to be a dead machine. If you were running X, you have no idea what 
happened. This is why "I have one NULL pointer dereference report" should 
mean "The fix needs to go upstream _now_".

(And no, as far as I can tell, it needs no suspend/resume cycle at all. I 
don't know the code very well, but as far as I can tell it just needs a 
wireless deauthentication, which easily happens if you're running 
something like NetworkManager and your wireless network may be noisy or 
weak).

				Linus

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Dmitry Torokhov]

On Tue, Nov 03, 2009 at 09:37:00AM -0800, Linus Torvalds wrote:
> 
> (And no, as far as I can tell, it needs no suspend/resume cycle at all. I 
> don't know the code very well, but as far as I can tell it just needs a 
> wireless deauthentication, which easily happens if you're running 
> something like NetworkManager and your wireless network may be noisy or 
> weak).
> 

I think I also hit it once by disabling the wireless through network
manager (wanted to refresh DHCP server list in resolv.conf)... But that
operation is not as common as suspend/resume for me.

-- 
Dmitry

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Luis R. Rodriguez]

On Tue, Nov 3, 2009 at 9:37 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Tue, 3 Nov 2009, Luis R. Rodriguez wrote:
>>
>> How this sort of issue is dealt with is subjective and it is up to
>> maintainers to deal with.
>
> Not when they then complain when others hit the same issue several days
> later.
>
>> Having more information on the patch and better communication about
>> the issue it solved, and the issues that reverting it would have
>> caused would certainly have helped maintainers make a better call at a
>> regression caused by it but knowing Johannes he'd probably cook up a
>> followup fix ASAP and that is exactly what he did.
>
> He may have cooked it up, but he didn't send it to me, and he didn't even
> bother to post it as a response to people who complained about the same
> commit.
>
> The fact that people on the wireless mailing lists may have known about
> this just makes things _worse_, I think. It shows that we really _need_ to
> go around maintainers, when not going around them seems to result in days
> of delays and total waste of time for everybody.

Well I wouldn't quite say that. Me and Johannes know about it, I
cannot say everyone else who reads linux-wireless understood the
issue. I was trying to explain that the root cause of this whole issue
was non-obvious and even when I found a fix that worked for me it
turned out that wasn't the "proper" solution. So in reality the only
one who probably really understood this issue inside out and backwards
was Johannes.

  Luis

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Andrew Morton]

On Tue, 03 Nov 2009 08:44:59 +0100 Johannes Berg <johannes@sipsolutions.net> wrote:

> On Tue, 2009-11-03 at 16:16 +0900, Marcel Holtmann wrote:
> 
> > and can we please stop jumping the gun here and going past the subsystem
> > maintainers. I think this happens a little bit too much lately.
> 
> I'll rant a bit too -- I've been very annoyed by this many times.

Problem is, subsystem maintainers are very unreliable.

Right now I'm sitting on 17 patches which I think should be in 2.6.32,
which I need to plead with subsystem maintainers to take a look at. 
They've already seen the patches (usually at least twice) and they've
just blown them off.  This is typical.

And then there are the 100 to 200 non-critical patches which I'm always
sitting on, similarly ignored.  And I'm increasingly just ignoring
stuff nowadays because this situation is so bad.

And then there are all the bug reports which flow in one ear and out
the other.

Now, some subsystem maintainer are good, and some aren't.  I'm probably
the only person who could write the detailed list for each column. 
Unfortunately, people who have better lives than me are best off
assuming that a subsystem maintainer is unreliable.  If the problem is
severe enough, bypassing the maintainer is a sensible default.

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Tue, 3 Nov 2009 22:34:33 -0800

> Problem is, subsystem maintainers are very unreliable.

This is common, but not universal Andrew.

Or, if it is, what patches from you have I been sitting on? :-)

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Andrew Morton]

On Wed, 04 Nov 2009 00:41:57 -0800 (PST) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Tue, 3 Nov 2009 22:34:33 -0800
> 
> > Problem is, subsystem maintainers are very unreliable.
> 
> This is common, but not universal Andrew.

That's what I said too.

> Or, if it is, what patches from you have I been sitting on? :-)

zippo.  Although if you can help these:

isdn-hisax-fix-lock-imbalance.patch
hfc_usb-fix-read-buffer-overflow.patch
misdn-fix-reversed-if-in-st_own_ctrl.patch
isdn-eicon-use-offsetof.patch
isdn-eicon-return-on-error.patch
hisax-fix-test-in-waitforxfw.patch

move forward it'd be nice!

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Wed, 4 Nov 2009 07:23:15 -0800

> Although if you can help these:
> 
> isdn-hisax-fix-lock-imbalance.patch
> misdn-fix-reversed-if-in-st_own_ctrl.patch
> isdn-eicon-use-offsetof.patch
> hisax-fix-test-in-waitforxfw.patch
> hfc_usb-fix-read-buffer-overflow.patch

Applied to net-2.6

> isdn-eicon-return-on-error.patch

Also applied to net-2.6, holy crap someone run that driver through
some auto-formatting tool! :-)

Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344

[Pavel Machek]

On Wed 2009-11-04 00:29:47, Marcel Holtmann wrote:
> Hi Johannes,
> 
> > > > I just think that it's a matter of courtesy that should be independent
> > > > from the release cycle to ask the author/maintainer by default, not as a
> > > > second thought ("unless [...] have other solution"). You can always CC
> > > > Linus and ask him to revert if you don't get a response.
> > > > 
> > > > What's wrong with that? It doesn't actually delay the action, but it
> > > > makes the discussion much more friendly and cooperative instead of
> > > > giving the author and maintainer the feeling that their opinion only
> > > > matters as a second thought.
> > > > 
> > > 
> > > I think you are reading too much into who was addressed directly and who
> > > was "only" CCed... 
> > 
> > Maybe. But it seems to be happening pretty often recently that people
> > first ask for a revert and then for a fix, ignoring any thought that
> > might have gone into a particular commit...
> 
> I have to agree here. It happens why too often lately. And this needs to
> stop. Otherwise why bother with subsystem maintainers? Just send
> everything to Linus directly and have him to review every line of code.
> 
> Dmitry, this is not against you, but the proper way would have been to
> just mail linux-wireless about it and you would have gotten the same
> response to it than you got by including Linus and LKML. This blind CC
> to LKML is not helpful. It starts confusion and just increases the load

Yes, lkml cc *is* helpful, as he's probably not the only one hitting
that problem.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html