All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chris Mason" <mason@kernel.org>
To: "Fuad Tabba" <fuad.tabba@linux.dev>
Cc: sashiko@lists.linux.dev,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	"Ihor Solodrai" <ihor.solodrai@linux.dev>,
	ast@kernel.org, "Jakub Kicinski" <kuba@kernel.org>,
	"Fuad Tabba" <tabba@google.com>
Subject: Re: [RFC] reworking the review-prompts subsystem guide
Date: Fri, 09 Oct 2026 13:57:53 -0400	[thread overview]
Message-ID: <c007a163-91a6-4e2a-b8de-057b61d369aa@app.fastmail.com> (raw)
In-Reply-To: <20261009141710.1871697-1-fuad.tabba@linux.dev>

On Fri, Oct 9, 2026, at 10:17 AM, Fuad Tabba wrote:
> Hi Chris,
>
> On Fri, 02 Oct 2026 20:04:00 +0100, "Chris Mason" <mason@kernel.org> wrote:
> [...]
>> My new branch builds subsystem guides by asking a long list of
>> questions, and then extensively reading the sources to find the
>> right answers.  The delta between the LLM's answers and the right
>> answers is the new guide.
>
> I went through the KVM/arm64 and protected KVM (pKVM) guides on the
> branch, and checked a sample of their claims against the tree. They're
> much more accurate and detailed than the hand-written ones, and the
> checking against the source is what makes the difference.

Great, I only did limited verification in that area, so its good to hear.

>
> Some of what a reviewer needs isn't in the code, though: which bugs
> matter and how much, and which reports are false alarms. The old pKVM
> guide said that a hypervisor crash only the host kernel can trigger is a
> hardening item, while one a guest can trigger is a real bug. The design
> deliberately keeps that kind of instruction out of the guides, so it's
> gone from the built one. Where should it live instead? The "Conventions
> for new code" files look closest: they already hold what maintainers ask
> for and no code states, and a review finds them by the directory a patch
> touches. Something like that for KVM/arm64 would work, and I can write
> it.

There's already a way to pass specific parts verbatim, which feels more
correct than under a "new code" label.  We can play around with a few
ideas though.

>
> Leaving out what the tested models already knew also tunes the guide to
> those models, and the model doing the review may not be one of them.

Yes.  I generated the current built prompts using both sonnet and opus
with the assumption that other models would get roughly the same
things wrong as one of the two of them.  This is obviously flawed, but
the prompts have a way to pick the best build directory based on kernel
version, so we could extend that to the model doing the
review (more below).

> Hand-written guides have the same problem, and I don't see an easy fix.
> Your mail points at builds per model, so one option is for each project
> to build for the model it runs. Another is for the build to keep the
> full checked answer rather than only the difference, now that the index
> makes length matter less. Which way are you leaning?

My original plan was to get the built guides small enough to be included
whole, in the same way the original guides were.  Adding the keyword
index was basically me admitting defeat, and it just didn't occur to me
that we might be able to index our way to victory on the full output.
IOW, great idea, I didn't think of that at all ;)

I'll do another run and put the full build up.  We can see if it's too big
to be useful or if we can make the index strong enough to get around
needing the per-model analysis.

>
>> This is both much less useful to human readers and much longer.  I'm
>> not sure what to say about the human reader part, but instead of
>> having LLMs read the whole subsystem guide, I shifted to an index
>> where they search for symbols.  This is a better fit for more
>> advanced models, which mostly need updates on how the kernel has
>> changed since they were trained.
>
> A common KVM patch adds a new hypercall. A symbol search finds the
> answers about the existing calls the patch uses, but not the rule for
> where a new one goes in the list: that answer is filed under a marker
> the diff never touches, and the header the diff edits isn't the source
> file of any index line. Could some answers be keyed to the directory as
> well, like the short table you kept for a few guides?
>

Absolutely.  The index is pretty dumb, we can do a lot better.

> [...]
>> What I know for sure is the existing review prompts have drifted
>> from mainline Linus.  It's impacting the quality of the reviews, so
>> I plan on working out something in the near future.
>
> Built guides drift too, just in a different way. These were built at
> 7.3-rc5, and one fix in rc6 made four of the KVM/arm64 claims wrong
> without renaming anything, so looking names up in the tree doesn't catch
> it. Most index lines already record a source file, so a review on a
> newer tree could check whether that file changed since the build, and
> re-check or flag the answer if it did.

A related question is how often do I need to rebuild in order for
the guides to be useful?  I'd assume the absolute minimum is every
final release, but every RC is also reasonable.

>
> Related: when a maintainer finds a wrong answer, the only fix is to
> change the question and rebuild, which needs model access and can come
> out differently each time. Who looks after the question files, and could
> there be a small override that a maintainer edits between rebuilds?
> Feedback from the list, as Jakub and Roman discussed, could take the
> same route: suggested questions that go through the same checks against
> the tree, rather than guide text.

The build script can rebuild a single guide, or people can just have
their agents hand edit the build?  It's a good point, we should have
AGENTS.md record some best practices.

>
> Separately, some pKVM questions were dropped for size, including the one
> on protected guest system registers, and the measurement notes say
> they're the first to bring back if the size limit is raised. There's no
> length limit now, so I could send a patch to bring them back.

Great, please do.

-chris

      reply	other threads:[~2026-10-09 17:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-02 19:04 [RFC] reworking the review-prompts subsystem guide Chris Mason
2026-10-02 21:18 ` Chuck Lever
2026-10-04  9:17   ` Chris Mason
2026-10-05 19:58 ` Jakub Kicinski
2026-10-05 21:14   ` Roman Gushchin
2026-10-05 21:53     ` Jakub Kicinski
2026-10-05 23:37     ` Ihor Solodrai
2026-10-06 21:22 ` Ihor Solodrai
2026-10-07  9:00   ` Chris Mason
2026-10-09 14:17 ` Fuad Tabba
2026-10-09 17:57   ` Chris Mason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c007a163-91a6-4e2a-b8de-057b61d369aa@app.fastmail.com \
    --to=mason@kernel.org \
    --cc=ast@kernel.org \
    --cc=fuad.tabba@linux.dev \
    --cc=ihor.solodrai@linux.dev \
    --cc=kuba@kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=sashiko@lists.linux.dev \
    --cc=tabba@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.