Re: [PATCH 6.1.y 0/1] Backporting patches with git-llm-pick

[Sasha Levin <sashal@kernel.org>]

On Mon, Sep 01, 2025 at 03:35:58PM +0000, Norbert Manthey wrote:
>Dear all,
>
>we looked into improving commit backporting workflows. This specific commit in
>this series caught our attention, as it states that backporting will not be
>trivial. The commit message has this part:
>
>Backport hint: this patch will have a trivial conflict applying to
>v6.6.y, and other trivial conflicts applying to stable kernels < v6.6.
>
>We want to automate backporting commits with a similar property. Therefore, we
>build a tool git-llm-pick that simplifies the backporting commit. After
>cherry-picking, we try to automatically detect dependency-commits. In case of
>failure, we then try to use the patch tool. If this process fails, we then take
>the rejected patch files, and feed their content with other context and a task
>description to backport to an LLM. The resulting code modification is then
>applied. In case validation is passed, a commit is created. The commit message
>is always extended by a description of the adapted code change, and with which
>technique a commit was applied.

Very nice!

I have a similar workflow, but it does a slightly different thing. My main
issue with having LLM just fix up conflicts and commit them is that it becomes
really hard to understand what the LLM actually did.

If you look at the generated commit text in your example message, it states
"Removed the warning message as per the patch" which is really confusing: does
this refer to something that the LLM changed? Is this different from what the
original commit was trying to do? Why is this explicitly called out?

The way I do it on my end, is that I create a "merge triangle" which contains:

  - The original commit, as is
  - The fixup
  - An explanation to the fixup

So using this commit as an example, it would generate:

$ git log --oneline --graph -5
*   5d59fa7c4b981 (HEAD) Explanation
|\  
| * 46beb491ffff2 fs/notify: fix merge conflict in fdinfo.c
| * b5031eb894d34 fs: relax assertions on failure to encode file handles
|/  
* f89b6e15694c1 (tag: v6.1.149, stable/linux-6.1.y) Linux 6.1.149
* 46643021596a6 alloc_fdtable(): change calling conventions.

Which gives me "atomic" git units to work with, but still allowing for easy review.

The original patch will be committed with the merge conflict:

diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index 55081ae3a6ec0..22adbf533855b 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -50,11 +50,15 @@ static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
         f.handle.handle_bytes = sizeof(f.pad);
         size = f.handle.handle_bytes >> 2;
  
+<<<<<<< HEAD
         ret = exportfs_encode_inode_fh(inode, (struct fid *)f.handle.f_handle, &size, NULL);
         if ((ret == FILEID_INVALID) || (ret < 0)) {
                 WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
+=======
+       ret = exportfs_encode_fid(inode, (struct fid *)f->f_handle, &size);
+       if ((ret == FILEID_INVALID) || (ret < 0))
+>>>>>>> 974e3fe0ac61d (fs: relax assertions on failure to encode file handles)
                 return;
-       }
  
         f.handle.handle_type = ret;
         f.handle.handle_bytes = size * sizeof(u32);

And then the following commit will address that:

diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index 22adbf533855b..dd5bc6ffae858 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -50,14 +50,8 @@ static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
         f.handle.handle_bytes = sizeof(f.pad);
         size = f.handle.handle_bytes >> 2;
  
-<<<<<<< HEAD
         ret = exportfs_encode_inode_fh(inode, (struct fid *)f.handle.f_handle, &size, NULL);
-       if ((ret == FILEID_INVALID) || (ret < 0)) {
-               WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
-=======
-       ret = exportfs_encode_fid(inode, (struct fid *)f->f_handle, &size);
         if ((ret == FILEID_INVALID) || (ret < 0))
->>>>>>> 974e3fe0ac61d (fs: relax assertions on failure to encode file handles)
                 return;
  
         f.handle.handle_type = ret;

In this way, it's easy for me to see what exactly the LLM changed, making for
way easier review of "fuzzy" backports.

And finally, the merge commit will contain a longer LLM generated explanation
that I can also validate. This way, I validate two things: the actual code
change, and the LLM's reasoning. If both are sane, then it helps be build trust
in the backport. Here's the explanation for this commit:

commit 5d59fa7c4b981ec18ac730b4d8629ec33619de6f (HEAD)
Merge: f89b6e15694c1 46beb491ffff2
Author: Sasha Levin <sashal@kernel.org>
Date:   Tue Sep 2 07:28:10 2025 -0400

     Explanation
     
     The Problem
     
     The HEAD commit (b5031eb894d3) contained an improperly resolved merge conflict
     in fs/notify/fdinfo.c.
     
     The conflict markers showed:
     
     - HEAD version: Used exportfs_encode_inode_fh() with a WARN_ONCE() assertion
     - Incoming change: Incorrectly used exportfs_encode_fid() (non-existent
       function) with wrong syntax
     
     The Resolution
     
     I fixed it by:
     
     1. Keeping the correct function: exportfs_encode_inode_fh() (not the
     non-existent exportfs_encode_fid())
     2. Removing the WARN_ONCE assertion: As intended by the original commit to
     "relax assertions"
     3. Preserving correct syntax: Using f.handle.f_handle (not f->f_handle)
     
     Why This is Correct
     
     The original commit's purpose was to relax assertions when file handle encoding
     fails, because these assertions were incorrect and could trigger in valid
     scenarios.
     
     The fix:
     
     - Removes the problematic WARN_ONCE() that could trigger incorrectly
     - Keeps the proper API function that actually exists in the kernel
     - Maintains clean error handling without warnings for legitimate failures
     
     The code now silently returns on encoding failures instead of triggering
     warnings, which aligns with the commit's stated goal of relaxing assertions for
     file handle encoding failures

At this point in my workflow, I can just squash the fix into the original
commit and decorate the new resulting commit as needed for -stable.

>We made the tool available on github: https://github.com/awslabs/Git_llm_pick/
>We currently use LLMs via the AWS Bedrock service, and default to the Nova Pro
>model. The patch in this series uses the vanilla version of the tool's current
>output when running git-llm-pick in its virtual test environment:

One note about the tool: in my experience, unless the tool can also act as an
agent and investigate the relevant git repo (and attempt builds and run tests)
on it's own, the results used to be very lackluster.

Without agentic access to more context/tooling, the LLM was guessing way too
much :)

-- 
Thanks,
Sasha