* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Luis R. Rodriguez @ 2009-11-03 17:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ingo Molnar, Marcel Holtmann, Dmitry Torokhov, David Miller,
johannes, linville, linux-kernel, linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030930130.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Dmitry Torokhov @ 2009-11-03 17:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Luis R. Rodriguez, Ingo Molnar, Marcel Holtmann, David Miller,
johannes, linville, linux-kernel, linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030930130.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 17:37 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Ingo Molnar, Marcel Holtmann, Dmitry Torokhov, David Miller,
johannes, linville, linux-kernel, linux-wireless
In-Reply-To: <43e72e890911030924n26550ee4j619a41ec016281ea@mail.gmail.com>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Luis R. Rodriguez @ 2009-11-03 17:24 UTC (permalink / raw)
To: Ingo Molnar
Cc: Marcel Holtmann, Linus Torvalds, Dmitry Torokhov, David Miller,
johannes, linville, linux-kernel, linux-wireless
In-Reply-To: <20091103162955.GA4836@elte.hu>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Ingo Molnar @ 2009-11-03 17:04 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Linus Torvalds, Dmitry Torokhov, David Miller, johannes, linville,
linux-kernel, linux-wireless
In-Reply-To: <1257266984.3420.111.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 16:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <1257266647.3420.105.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Marcel Holtmann @ 2009-11-03 16:49 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Dmitry Torokhov, David Miller, johannes, linville,
linux-kernel, linux-wireless
In-Reply-To: <20091103162955.GA4836@elte.hu>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Marcel Holtmann @ 2009-11-03 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030815180.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 16:37 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030815180.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Ingo Molnar @ 2009-11-03 16:29 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Linus Torvalds, Dmitry Torokhov, David Miller, johannes, linville,
linux-kernel, linux-wireless
In-Reply-To: <1257264485.3420.87.camel@localhost.localdomain>
* 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
^ permalink raw reply
* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Oliver Neukum @ 2009-11-03 16:29 UTC (permalink / raw)
To: Alan Stern
Cc: Dan Williams, Matthew Dharm, Frank Schaefer, linux-wireless,
linux-usb
In-Reply-To: <Pine.LNX.4.44L0.0911031001470.10778-100000@netrider.rowland.org>
Am Dienstag, 3. November 2009 16:16:12 schrieb Alan Stern:
> This sounds as if you are trying very hard to get the _worst_ of both
> worlds! You would force users to upgrade their kernels before they can
> use new devices (because the kernel is where the mode-switch code will
> be), and you also would force them to upgrade their userspace settings
> (so that the new mode-switch code will be called).
Yes.
> I'm with Matt Dharm and Josua Dietze on this. Jobs that can be handled
> in userspace _should_ be handled there.
There is the inconvenient problem of hibernation. And, if more systems
start cutting power to USB in STR, the problem of STR. I am inclined
to declare that hibernation is a problem of those pesky misdesigns.
But dare we say that also about STR?
Regards
Oliver
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 16:23 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <1257264485.3420.87.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 16:07 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030738260.31845@localhost.localdomain>
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.
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Marcel Holtmann @ 2009-11-03 16:08 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030738260.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Zdenek Kabelac @ 2009-11-03 15:54 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Linus Torvalds, Dmitry Torokhov, David Miller, johannes, linville,
linux-kernel, linux-wireless
In-Reply-To: <1257262588.3420.79.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 15:43 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <1257262588.3420.79.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 15:38 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Johannes Berg, Dmitry Torokhov, David Miller, linville,
linux-kernel, linux-wireless
In-Reply-To: <1257262187.3420.73.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Marcel Holtmann @ 2009-11-03 15:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <alpine.LFD.2.01.0911030719430.31845@localhost.localdomain>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 15:31 UTC (permalink / raw)
To: Johannes Berg
Cc: Marcel Holtmann, Dmitry Torokhov, David Miller, linville,
linux-kernel, linux-wireless
In-Reply-To: <1257234299.28469.25.camel@johannes.local>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Marcel Holtmann @ 2009-11-03 15:29 UTC (permalink / raw)
To: Johannes Berg
Cc: Dmitry Torokhov, David Miller, torvalds, linville, linux-kernel,
linux-wireless
In-Reply-To: <1257238623.28469.47.camel@johannes.local>
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
^ permalink raw reply
* Re: Please consider reverting 7d930bc33653d5592dc386a76a38f39c2e962344
From: Linus Torvalds @ 2009-11-03 15:26 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Dmitry Torokhov, David Miller, johannes, linville, linux-kernel,
linux-wireless
In-Reply-To: <1257232587.3420.55.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Alan Stern @ 2009-11-03 15:16 UTC (permalink / raw)
To: Dan Williams; +Cc: Matthew Dharm, Frank Schaefer, linux-wireless, linux-usb
In-Reply-To: <1257198124.1027.63.camel@localhost.localdomain>
On Mon, 2 Nov 2009, Dan Williams wrote:
> In userspace, usb_modeswitch is the place to put all this logic. Then
> you tie it together with udev rules using some bubble-gum and duct-tape,
> and maybe it works. Of course, there's massive duplication of data
> between usb_modeswitch and the kernel drivers, because the there's
> simply no communication between the two.
The only reason for this "massive" duplication is that nobody has
bothered to remove from the kernel the parts that usb_modeswitch can
handle. Adding yet more switching code into the kernel, thereby
_increasing_ the amount of duplication, seems like a bad idea.
> The kernel drivers know which devices are supported and how to drive
> them.
No. The kernel drivers (usb-storage is where most if not all of the
kernel-side mode-switching code has ended up) only know about the
particular devices _they_ support. They don't know about the other
devices supported by usb_modeswitch.
> But because the eject code isn't in kernelspace, all that device
> selection logic has to be duplicated in userspace with usb_modeswitch.
> Pretty dumb.
What are you talking about? Of course usb-storage contains
mode-switching code for the devices it switches. And for the devices
it doesn't switch, it does not contain device-selection logic.
> Maybe we can get the kernel drivers to expose the information we need
> (maybe even an 'eject' attribute in sysfs or something) and then we just
> have to write udev rules instead of having a whole bunch of libusb junk
> in userspace? Would that preserve the policy-always-in-userspace
> requirement yet keep the code to drive the hardware in kernel space
> where it really belongs?
This sounds as if you are trying very hard to get the _worst_ of both
worlds! You would force users to upgrade their kernels before they can
use new devices (because the kernel is where the mode-switch code will
be), and you also would force them to upgrade their userspace settings
(so that the new mode-switch code will be called).
I'm with Matt Dharm and Josua Dietze on this. Jobs that can be handled
in userspace _should_ be handled there.
Alan Stern
^ permalink raw reply
* Re: 2.6.32-rc5-git3: Reported regressions from 2.6.31
From: Michael Buesch @ 2009-11-03 14:10 UTC (permalink / raw)
To: Christian Casteyde; +Cc: John W. Linville, linux-wireless, Johannes Berg
In-Reply-To: <200911030043.26510.casteyde.christian@free.fr>
On Tuesday 03 November 2009 00:43:26 Christian Casteyde wrote:
> Nothing to mention: it seems to work, at least on my hardware.
> I got the Allocated bounce buffer log, and managed to boot without any error,
> associate and access the web/ssh another computer (and didn't get any
> kmemcheck error of course).
Ok, cool. Thanks a lot for testing. I'll resubmit this patch for inclusion later.
--
Greetings, Michael.
^ permalink raw reply
* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Christian Lamparter @ 2009-11-03 12:58 UTC (permalink / raw)
To: Oliver Neukum
Cc: Matthew Dharm, Dan Williams, Frank Schaefer, linux-wireless,
linux-usb
In-Reply-To: <200911031157.23165.oliver@neukum.org>
On Tuesday 03 November 2009 11:57:22 Oliver Neukum wrote:
> Am Montag, 2. November 2009 22:11:23 schrieb Matthew Dharm:
> > It's worth noting that for some of these which are handled in-kernel, it
> > had to be done that way because their storage-emulation was so poor that
> > the normal 'eject' WOULD NOT work properly.
> >
> > Any device which can be handled in userspace SHOULD be handled there.
>
> Do these devices survive a reset and stay in the correct mode?
> Has that been tested?
ar9170usb does an usb reset right before it uploads the WLAN firmware.
The device stays in the correct mode, else it wouldn't be possible
to continue because usb-storage mode has a different PID.
Regards,
Chr
^ permalink raw reply
* Re: [PATCH] ar9170usb: add mode-switching for AVM Fritz!WLAN USB N devices in cdrom mode
From: Oliver Neukum @ 2009-11-03 10:57 UTC (permalink / raw)
To: Matthew Dharm; +Cc: Dan Williams, Frank Schaefer, linux-wireless, linux-usb
In-Reply-To: <20091102211123.GI24436@one-eyed-alien.net>
Am Montag, 2. November 2009 22:11:23 schrieb Matthew Dharm:
> It's worth noting that for some of these which are handled in-kernel, it
> had to be done that way because their storage-emulation was so poor that
> the normal 'eject' WOULD NOT work properly.
>
> Any device which can be handled in userspace SHOULD be handled there.
Do these devices survive a reset and stay in the correct mode?
Has that been tested?
Regards
Oliver
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox