Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Kalle Valo]

(dropping stable, adding linux-wireless)

"Alf Marius" <post@alfmarius.net> writes:

> Hi,
> I recently installed Arch Linux on an old laptop (Fujitsu-Siemens AMILO Xi 2550) and noticed that:
>
> - when booting Linux from the Arch ISO (kernel version 6.10.10) WIFI is working fine
> - after installing Arch Linux from the ISO and booting (kernel version 6.11.5) WIFI was not working properly
>
> By "not working properly" I mean: 
> downloading small files or installing a few small packages was working
> ok, but when downloading larger files or installing larger packages
> with lots of dependencies, the connection would gradually slow down
> and eventually die.
>
> I reported this on the Arch Linux forum (https://bbs.archlinux.org/viewtopic.php?pid=2206757)
> and some helpful memeber suggested that this might be the commit that broke things:
> https://github.com/torvalds/linux/commit/02b682d54598f61cbb7dbb14d98ec1801112b878
>
> An Arch Linux packet manager (gromit) helped me debug this issue by building a couple of kernels that I tested.
>
> - https://pkgbuild.com/\~gromit/linux-bisection-kernels/linux-mainline-6.12rc5-1-x86_64.pkg.tar.zst
> - https://pkgbuild.com/\~gromit/linux-bisection-kernels/linux-mainline-6.12rc5-1.1-x86_64.pkg.tar.zst
>
> The first one didn't work, but the second (in which he reverted the commit linked above) did fix my problem.
> So, I guess this commit should be investigated by those in the know.
> Thats why I also added Andrii and Kalle to CC as they are listed in the commit message.
>
> My network controller: Intel corporation PRO/Wireless 4965 AG or AGN [Kedron] Network Connection (rev 61)
> Kernel driver in use: iwl4965
>
> This is my first kernel bug report, hope I did everything right :)

Perfect report, thanks.

> I'm ofc willing to help provide more info and debug locally here to help solve this issue.

Andrii, any ideas? Unless we can fix this quickly I think we need to
revert commit 02b682d5459.

#regzbot introduced: 02b682d54598f61cbb7dbb14d98ec1801112b878 ^

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Andrey Batyiev]

Hello everyone,

I've only tested it on my 3945. I have no equipment to test it on 4965, sorry.

Regards,
   Andrii

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Alf Marius]

Good evening folks :)

[Andrey Batyiev]
> Hello everyone,
> I've only tested it on my 3945. I have no equipment to test it on 4965, sorry.

Yes the commit message says "Tested on iwl3945 only."
..which I did find a bit strange. Is it normal to deploy code to the mainline
that is untested? Why was this also applied to 4965?

I'm just asking questions here, as I have no direct knownledge of C or
kernel driver programming. I've 20 yrs of web-dev experience though
and know from experience that shipping untested code is a bad idea.

Anyway, not trying to point fingers here! Just curious to find those who
wrote the actual code, maybe get some info on why this was added
and if it is really important. If not, maybe a revert is in order

Regarding testing, I obviously have a laptop with the 4965 card and
I'm more than willing to test stuff out if needed.

-Alf
-- 
"The generation of random numbers is too important to be left to chance."

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Kalle Valo]

"Alf Marius" <post@alfmarius.net> writes:

> Good evening folks :)
>
> [Andrey Batyiev]
>> Hello everyone,
>> I've only tested it on my 3945. I have no equipment to test it on 4965, sorry.
>
> Yes the commit message says "Tested on iwl3945 only."
> ..which I did find a bit strange. Is it normal to deploy code to the mainline
> that is untested? Why was this also applied to 4965?
>
> I'm just asking questions here, as I have no direct knownledge of C or
> kernel driver programming. I've 20 yrs of web-dev experience though
> and know from experience that shipping untested code is a bad idea.
>
> Anyway, not trying to point fingers here! Just curious to find those who
> wrote the actual code, maybe get some info on why this was added
> and if it is really important. If not, maybe a revert is in order
>
> Regarding testing, I obviously have a laptop with the 4965 card and
> I'm more than willing to test stuff out if needed.

I sent a revert to fix this:

https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?id=11597043d74809daf5d14256b96d6781749b3f82

If all goes well this should be in v6.13-rc1.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Linux regression tracking (Thorsten Leemhuis)]

On 14.11.24 08:32, Kalle Valo wrote:
> "Alf Marius" <post@alfmarius.net> writes:
>> [Andrey Batyiev]
>>> Hello everyone,
>>> I've only tested it on my 3945. I have no equipment to test it on 4965, sorry.
>>
>> Yes the commit message says "Tested on iwl3945 only."
>> ..which I did find a bit strange. Is it normal to deploy code to the mainline
>> that is untested? Why was this also applied to 4965?
>>
>> I'm just asking questions here, as I have no direct knownledge of C or
>> kernel driver programming. I've 20 yrs of web-dev experience though
>> and know from experience that shipping untested code is a bad idea.
>>
>> Anyway, not trying to point fingers here! Just curious to find those who
>> wrote the actual code, maybe get some info on why this was added
>> and if it is really important. If not, maybe a revert is in order
>>
>> Regarding testing, I obviously have a laptop with the 4965 card and
>> I'm more than willing to test stuff out if needed.
> 
> I sent a revert to fix this:
> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?id=11597043d74809daf5d14256b96d6781749b3f82

Many thx for taking care of this!

> If all goes well this should be in v6.13-rc1.

Sigh. FWIW, I think that should have gone straight to mainline, as the
situation afaics is quite similar to this one where Linus clarified that
he wants such fixes even at this stage of the cycle:

https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/

Given that this is a hardware that likely is not much in use any more I
guess it's not making much noise about besides this mail.

Alf: that patch lacks a stable tag, so there is no guarantee that the
fix will be backported to 6.12.y and earlier; but it likely will due to
the Fixes tag. If it wasn't backported within 2 weeks after 6.13-rc1 is
out, please speak up.

Ciao, Thorsten

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Johannes Berg]

On Thu, 2024-11-14 at 10:24 +0100, Linux regression tracking (Thorsten
Leemhuis) wrote:
> 
> Sigh.

Please. You really should consider adjusting your attitude on all this. 

Do you _actually_ think that everyone is just out to make everyone's
life difficult? Because you certainly consistently make it sound like
it. It takes time to get something into the kernel, it's not like we can
just commit it to Linus's tree.

johannes

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Johannes Berg]

On Thu, 2024-11-14 at 10:52 +0100, Johannes Berg wrote:
> On Thu, 2024-11-14 at 10:24 +0100, Linux regression tracking (Thorsten
> Leemhuis) wrote:
> > 
> > Sigh.
> 
> Please. You really should consider adjusting your attitude on all this. 

Also, btw, you're always the most vocal about this, and pretty much the
only feedback at all.

As a result (at least to me), you're creating a reverse incentive. Since
I'm going to get shouted at all the time _anyway_, why bother putting in
any extra effort some of the time (realistically can't do it all or even
most of the time). Just do the simplest thing possible and throw it into
the tree that'll make it upstream somewhere without any risks (like
here, what if wireless won't get pulled quickly, Jakub had been on
vacation recently and things got delayed, and if you miss then it's a
lot of work, etc.)

johannes

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Thorsten Leemhuis]

[adding Linus to the list of recipients in case he want to clarify
things or pick up the revert in question directly; also CCing Greg due
to the stable aspect JFYI]

On 14.11.24 10:52, Johannes Berg wrote:
> On Thu, 2024-11-14 at 10:24 +0100, Linux regression tracking (Thorsten
> Leemhuis) wrote:
>>
>> Sigh.
> 
> Please. You really should consider adjusting your attitude on all this. 

My approach/attitude is based on actions from Linus and/or what I expect
he wants me to do, so let's bring him in to give him a chance to state
if I went to far here.

Linus, Johannes' reaction was due to this mail of mine:

https://lore.kernel.org/all/5f64abc6-017f-4283-bf08-dba1aea28e9d@leemhuis.info/

To quote a bit more than Johannes did:
"""
>>> I sent a revert to fix this:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?id=11597043d74809daf5d14256b96d6781749b3f82
>> 
>> Many thx for taking care of this!
>> 
>>> If all goes well this should be in v6.13-rc1.
>> 
>> Sigh. FWIW, I think that should have gone straight to mainline, as the
>> situation afaics is quite similar to this one where Linus clarified that
>> he wants such fixes even at this stage of the cycle:
>> 
>> https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/
>> 
>> Given that this is a hardware that likely is not much in use any more I
>> guess it's not making much noise about besides this mail.
>> 
>> Alf: that patch lacks a stable tag, so there is no guarantee that the
>> fix will be backported to 6.12.y and earlier; but it likely will due to
>> the Fixes tag. If it wasn't backported within 2 weeks after 6.13-rc1 is
>> out, please speak up.
>> 
>> Ciao, Thorsten
"""

BTW, the fix in question can also be found here:
https://lore.kernel.org/all/20241112142419.1023743-1-kvalo@kernel.org/

> Do you _actually_ think that everyone is just out to make everyone's
> life difficult? Because you certainly consistently make it sound like
> it. It takes time to get something into the kernel, it's not like we can
> just commit it to Linus's tree.

Well, Linus is known to have no problem at all with picking up fixes
straight from lists if there are good reasons (like the unlucky timing
we have here), as long as that does not become the norm. That approach
was actually used in the situation I pointed to, as it really is quite
similar.

And it might be the right thing to do in this situation as well, as then
the fix for this regression from v6.11-rc1 will reach 6.12 -- and likely
also reach users of 6.11.y stable releases within a week, as Greg likely
will pick this up quickly and usually publishes new stable releases at
least once per week.

If the revert is only merged during the merge window, it will likely
take the fix at least two weeks more to reach end-users, unless somebody
asks Greg "please pick this up now" once it reached mainline. Three
weeks if we for some reason get a 6.12-rc8. And if the fix is not in the
first batch of fixes picked by Greg after 6.13-rc1, there might be even
one more week on top of those outcomes. So four additional weeks if we
are really unlucky.

Things like that bother me -- and thus, yes, sorry, also influence my
attitude, which lead to the quoted "sigh". It's just that "fixes are
there, they just take a long time to reach mainline and stable releases"
is the biggest problem I see during my regression tracking work.

The problem is, that bring this up in situations like this is making the
"maintainer burnout" problem worse. I hate that and feed sorry for that.
But well, ignoring this sounds like the wrong solution as well. I could
write private "hey Linus, I think the following regression is not
handled well" mails instead. But Linus already has a lot on his plate as
well. And people call me a snitch if I do that. Hmmm. :-/

Ciao, Thorsten

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Johannes Berg]

On Thu, 2024-11-14 at 16:30 +0100, Thorsten Leemhuis wrote:
> 
> My approach/attitude is based on actions from Linus and/or what I expect
> he wants me to do, so let's bring him in to give him a chance to state
> if I went to far here.

Umm, no. You own your attitude and communication behaviour. That's not
Linus's fault. Maybe he asked you to pay attention to how fixes enter
mainline, maybe he didn't, I don't know or don't even care. But the fact
is that for a very long time now pretty much every single interaction
we've had has been in one way or the other you complaining about how
I/we did something. It really does feel like all you care about is
piling on more pressure.

Also, FTR, I wasn't even involved in the patch.

> Well, Linus is known to have no problem at all with picking up fixes
> straight from lists if there are good reasons (like the unlucky timing
> we have here), as long as that does not become the norm. That approach
> was actually used in the situation I pointed to, as it really is quite
> similar.

And we've obviously done that in the past. For _important_ fixes. A
single user reported this, was able to use an older kernel while it's
getting fixed, and it's on a device that's somewhere around 15 years old
at this point.

But you seem to think it's *free* to do this. It isn't! It needs
coordination, asking, it needs to be managed, etc.
> 
> The problem is, that bring this up in situations like this is making the
> "maintainer burnout" problem worse. I hate that and feed sorry for that.

I don't buy it any more. That worked a long time ago, but it's clearly
the only thing you ever do, and you seem to be pretty comfortable doing
it over and over and over again.

johannes

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Toke Høiland-Jørgensen]

Hi Thorsten

Quoting a bit selectively and out of order to comment on a few unrelated
aspects of this (meta-)discussion:

Thorsten Leemhuis <regressions@leemhuis.info> writes:
> On 14.11.24 10:52, Johannes Berg wrote:
>> On Thu, 2024-11-14 at 10:24 +0100, Linux regression tracking (Thorsten
>> Leemhuis) wrote:
>>>
>>> Sigh.
>> 
>> Please. You really should consider adjusting your attitude on all this. 
>
> If the revert is only merged during the merge window, it will likely
> take [...] four additional weeks if we are really unlucky.
>
> Things like that bother me -- and thus, yes, sorry, also influence my
> attitude, which lead to the quoted "sigh". It's just that "fixes are
> there, they just take a long time to reach mainline and stable releases"
> is the biggest problem I see during my regression tracking work.

I understand your frustration, certainly, and I get that you are tasked
with a thankless job in trying to improve the regression handling
situation. However, I also agree with Johannes that by letting your
frustration shine through (which is basically what you're doing with
that "sigh"), you are turning a general frustration at a difficult
systemic problem into animosity towards the *person* you are directing
your email at. This comes across as incredibly condescending, and it
only fosters animosity in the other direction, even creating a perverse
incentive to just ignore regressions entirely, as Johannes also pointed
out in his follow-up email[0].

Or to put it another way: the actual patch being discussed here should
be held up as an example of a *successful* regression handling: A
relatively obscure issue was reported, and within a week a fix was
committed to the relevant subsystem tree. If we had been *anywhere* else
in the cycle than where we are right now, this would have percolated up
to Linus' tree in time for the next -rc, or at the very latest one week
later. Judging from the numbers you presented at LPC, this would be a
very timely fix in all cases.

So yes, it's unfortunate that this happened to land at an unlucky time
in the cycle, and yes, in the ideal world this would just have been
fast-tracked. But by reacting with frustration to something that, for
all intents and purposes, is a proper and timely handling of a
regression, you are antagonising people and hurting your cause, not
helping it.

As for a concrete suggestion for how this could have been handled
better, if you had followed up your "Many thx for taking care of this!"
(which was great!) with a "I noticed there's an opportunity to get this
in before 6.12 is released, which will mean this gets to users several
weeks faster. Would you mind asking Linus to (cherry-pick/pull) this out
of order, as he has indicated he is willing to do", instead of the
"sigh, you are doing this wrong", you would have a much higher chance of
actually getting results.


As a side note, on this point...

> My approach/attitude is based on actions from Linus and/or what I expect
> he wants me to do, so let's bring him in to give him a chance to state
> if I went to far here.

...I will add that I personally find the whole "appeal to authority"
approach you are taking with the regression handling processes to be
somewhat off-putting. "Because Linus says so" is a terrible argument for
doing anything (on its own). If you can't persuade people by the
strength of your argument, appealing to authority will at best get you
grudging least-effort compliance, and at worst it will be actively
harmful because you antagonise people.

And yes, convincing people to change the way they work is hard,
especially when the ask is for them to do *more* work. That doesn't mean
it is impossible, but it takes time and patience and a variety of
approaches to succeed in the long term.


Note that I am saying all of the above as someone who is generally
sympathetic to the goals of your work (as I am sure you are aware, since
you're using my exchange with Linus about this in your slides :)). I do
not believe you are trying to antagonise people on purpose, so please
view the above as a good faith attempt at providing some feedback on how
you can continue this work without ending up doing so unintentionally.

-Toke

[0] https://lore.kernel.org/r/35bee1c6146cf261ad6b47f585a5b454ad0763ec.camel@sipsolutions.net

Re: [REGRESSION] The iwl4965 driver broke somewhere between 6.10.10 and 6.11.5 (probably 6.11rc)

[Linux regression tracking (Thorsten Leemhuis)]

On 14.11.24 21:36, Toke Høiland-Jørgensen wrote:
> I understand your frustration, certainly, and I get that you are tasked
> with a thankless job in trying to improve the regression handling
> situation. However, I also agree with Johannes that by letting your
> frustration shine through [...]

Thx for the mail. Yeah, you are right, apparently my frustration took
more hold of me than I was aware of/willing to admit.

Apologies to everyone I steered up, I'll try to do better.

Ciao, Thorsten