On 7/9/2025 12:23 PM, Eric W. Biederman wrote:
Sasha Levin <[email protected]> writes:
On Tue, Jul 08, 2025 at 04:46:19PM -0500, Eric W. Biederman wrote:
Sasha Levin <[email protected]> writes:
On Tue, Jul 08, 2025 at 02:32:02PM -0500, Eric W. Biederman wrote:
Wow!
Sasha I think an impersonator has gotten into your account, and
is just making nonsense up.
https://lore.kernel.org/all/aDXQaq-bq5BMMlce@lappy/
It is nice it is giving explanations for it's backporting decisions.
It would be nicer if those explanations were clearly marked as
coming from a non-human agent, and did not read like a human being
impatient for a patch to be backported.
Thats a fair point. I'll add "LLM Analysis:" before the explanation to
future patches.
Further the machine given explanations were clearly wrong. Do you have
plans to do anything about that? Using very incorrect justifications
for backporting patches is scary.
Just like in the past 8 years where AUTOSEL ran without any explanation
whatsoever, the patches are manually reviewed and tested prior to being
included in the stable tree.
I believe there is some testing done. However for a lot of what I see
go by I would be strongly surprised if there is actually much manual
review.
I expect there is a lot of the changes are simply ignored after a quick
glance because people don't know what is going on, or they are of too
little consequence to spend time on.
I don't make a point to go back and correct the justification, it's
there more to give some idea as to why this patch was marked for
review and may be completely bogus (in which case I'll drop the patch).
For that matter, I'd often look at the explanation only if I don't fully
understand why a certain patch was selected. Most often I just use it as
a "Yes/No" signal.
In this instance I honestly haven't read the LLM explanation. I agree
with you that the explanation is flawed, but the patch clearly fixes a
problem:
"On AMD dGPUs this can lead to failed suspends under memory
pressure situations as all VRAM must be evicted to system memory
or swap."
So it was included in the AUTOSEL patchset.
Do you have an objection to this patch being included in -stable? So far
your concerns were about the LLM explanation rather than actual patch.
Several objections.
- The explanation was clearly bogus.
- The maintainer takes alarm.
- The patch while small, is not simple and not obviously correct.
- The patch has not been thoroughly tested.
I object because the code does not appear to have been well tested
outside of the realm of fixing the issue.
There is no indication that the kexec code path has ever been exercised.
So this appears to be one of those changes that was merged under
the banner of "Let's see if this causes a regression".>
To the original authors. I would have appreciated it being a little
more clearly called out in the change description that this came in
under "Let's see if this causes a regression".
As the original author of this patch I don't feel this patch is any
different than any other patch in that regard.
I don't write in a commit message the expected risk of a patch.
There are always people that find interesting ways to exercise it and
they could find problems that I didn't envision.
Such changes should not be backported automatically. They should be
backported with care after the have seen much more usage/testing of
the kernel they were merged into. Probably after a kernel release or
so. This is something that can take some actual judgment to decide,
when a backport is reasonable.
TBH - I didn't include stable in the commit message with the intent that
after this baked a cycle or so that we could bring it back later if
AUTOSEL hadn't picked it up by then.
It's a real issue people have complained about for years that is
non-obvious where the root cause is.
Once we're all confident on this I'd love to discuss bringing it back
even further to LTS kernels if it's viable.
I still highly recommend that you get your tool to not randomly
cut out bits from links it references, making them unfollowable.
Good point. I'm not really sure what messes up the line wraps. I'll take
a look.
It was a bit more than line wraps. At first glance I thought
it was just removing a prefix from the links. On second glance
it appears it is completely making a hash of links:
The links in question:
https://github.com/ROCm/ROCK-Kernel-Driver/issues/174
https://gitlab.freedesktop.org/drm/amd/-/issues/2362
The unusable restatement of those links:
ROCm/ROCK-Kernel-Driver#174
freedesktop.org/drm/amd#2362
Short of knowing to look up into the patch to find the links,
those references are completely junk.
At best all of this appears to be an effort to get someone else to
do necessary thinking for you. As my time for kernel work is very
limited I expect I will auto-nack any such future attempts to outsource
someone else's thinking on me.
I've gone ahead and added you to the list of people who AUTOSEL will
skip, so no need to worry about wasting your time here.
Thank you for that.
I assume going forward that AUTOSEL will not consider any patches
involving the core kernel and the user/kernel ABI going forward. The
areas I have been involved with over the years, and for which my review
might be interesting.
The filter is based on authorship and SoBs. Individual maintainers of a
subsystem can elect to have their entire subsystem added to the ignore
list.
As I said. I expect that the process looking at the output of
get_maintainers.pl and ignoring a change when my name is returned
will result in effectively the entire core kernel and the user/kernel
ABI not being eligible for backport.
I bring this up because I was not an author and I did not have any
signed-off-by's on the change in question, and yet I was still selected
for the review.
Eric
