From: "Darrick J. Wong" <djwong@kernel.org>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: cem@kernel.org, linux-xfs@vger.kernel.org, dgc@kernel.org, hch@lst.de
Subject: Re: [RFC PATCH] xfs: add new policy guidelines for llm-assisted patches
Date: Thu, 9 Jul 2026 09:33:52 -0700 [thread overview]
Message-ID: <20260709163352.GD15210@frogsfrogsfrogs> (raw)
In-Reply-To: <ab8c9529-103d-4e25-b062-1700544a0bdb@sandeen.net>
On Thu, Jul 09, 2026 at 07:30:41AM -0500, Eric Sandeen wrote:
> On 7/9/26 6:59 AM, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > Hi, this idea came from some observations on the current inflow of patches
> > sent to xfs, amount of time we've been spending reviewing patches, lack of
> > testing coverage for them and sporadically bollocks patches that make no
> > sense or even do not compile.
> >
> > A talk I had with Dave earlier today made me come up with an INITIAL
> > DRAFT of what should IMHO make 'reviewable' any LLM-assisted patch
> > submitted to the list.
> >
> > Most of the information there is also valid for non-LLM assisted code,
> > but LLM-assisted code makes these policies exceptionally important
> > giving LLMs make the code generation way faster and easier than we have
> > time to follow through.
> >
> > We do have tooling now like Sashiko to help with a gross review of
> > patches and some general policies, but none of those tooling/policies
> > target xfs specifically so I thought we ought to have a specific policy
> > in place, specially regarding testing-coverage as submitting
> > LLM-assisted patches also implies the same tooling can be used to create
> > fully-functional testing coverage in xfstests.
> >
> > I'll appreciate your thoughts on this.
>
> I like it.
>
> (applying my pedantic liberal arts native English speaker editorial preferences,
> you can take it or leave it)
>
> > Cheers
> >
> > Signed-off-by: Carlos Maiolino <cem@kernel.org>
> > ---
> > ...m-assisted-patch-submission-guidelines.rst | 59 +++++++++++++++++++
> > 1 file changed, 59 insertions(+)
> > create mode 100644 Documentation/filesystems/xfs/xfs-llm-assisted-patch-submission-guidelines.rst
> >
> > diff --git a/Documentation/filesystems/xfs/xfs-llm-assisted-patch-submission-guidelines.rst b/Documentation/filesystems/xfs/xfs-llm-assisted-patch-submission-guidelines.rst
> > new file mode 100644
> > index 000000000000..1f7921789988
> > --- /dev/null
> > +++ b/Documentation/filesystems/xfs/xfs-llm-assisted-patch-submission-guidelines.rst
> > @@ -0,0 +1,59 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +.. _xfs_llm_assisted_patch_submission_guidelines:
> > +
> > +============================
> > +XFS LLM-Assisted patch submission guidelines
> > +============================
> > +
> > +Introduction
> > +============
>
> > +LLMs are a great tool for improving code quality when well used. But they also
I'd change that to "...when used well." I dunno if this is a local
colloquialism but to me "used well" means "used in a beneficial manner"
whereas "well used" just means "used very much".
> > +have been creating a lot of extra workload for developers with the increasing
>
> have the potential to create an extra workload for the XFS developer community with
"But the increasing patch flow creates a lot of extra work for the XFS
developer community." ?
> > +patch flow. Requiring much more time with reviewing and testing changes.
> > +
> > +Some patches submited fixes obvious bugs and are welcome, while other patches
>
> Some LLM generated patches fix obvious bugs and are welcome, while others
> have obvious flaws, create regressions caught by xfstests, fix theoretical bugs
> that may never be hit in the real world, and sometimes do not even build.
I agree.
> > +being submitted have obviously flaws, create regressions caught by xfstests,
> > +fixes theoretical bugs that may never be hit in real world (even though are
> > +worth fixing) and sometimes do not even build.
> > +
> > +So the goal of the policies described by this document is two-fold:
>
> The goal of the policies described by
Agree here too.
> > +
> > + - Increase XFS's code quality ensuring all code modifications are
> > + properly tested and have extra coverage
>
"Increase XFS's code quality by ensuring..." ?
> sufficient coverage?
Yes.
> > + - Reduce developers/maintainers workload with the extra income of
> > + machine-generated patches.
>
> Reduce developer / maintainer workload with the extra influx of
Hah, I wish I got extra income from all this LOLLM slop. ;)
> > +
> > +Patch description
> > +-----------------
> > +
> > +Patches description should be carefully trimmed by the patch submitter removing
> > +all extra and unnecessary data from it.
>
> Patch descriptions should be succinct and clear.
"The patch description should state clearly the specific reasons why the
change is being made. It should not be a summary of the changes in the
diff." ?
> > +LLMs tend to generate extra-long documentation full of unnecessary information
> > +that won't help neither the reviewer nor anybody looking into the git history
>
> that won't help the reviewer or anyone reading git history in the future,
(Agree)
> > +in the future, and these consumes a lot of time during review.
>
> and these consume a lot of time during review.
"Reading the unnecessarily wordy documentation consumes too much time
during reviews." ?
> > +It's the patch submitter responsibility to trim it down to a concise, easily
>
> It's the patch submitter's responsibility to ...
(Agree)
> > +readable document, removing all the extra unnecessary information from it.
>
> Strike "removing all the extra unnecessary information from it" which is extra
> and unnecessary. ;)
Yes.
> > +
> > +This also helps adding extra guardrails that the patch submitter fully understands
> > +what the patch is doing without letting the LLM loose.
>
> (this is a little unclear to me)
How about:
"These guidelines are a means for the person submitting the patch to
demonstrate that they fully understand the changes in the diff.
Reviewers may ask follow-up questions if they are not convinced of this
point. The person submitting the change is always fully responsible for
those changes."
(and then drop the next section)
> > +
> > +Patch changes
Do you mean the diff part?
> > +-------------
> > +
> > +The patch submitter is fully responsible for the changes and must understand what
> > +the paitch does. And it should be in full agreement with the patch description.
Don't start a sentence with "And".
I also can't tell what "it" refers to -- does that mean the diff should
agree with the description? Or that the submitter themself should?
"The patch diff must match the patch description."
> "patch" - but also not entirely sure what this means or how to better word it.
> "Patch changes" is a bit of an odd heading. In general I 100% agree with
> "you, the human, had better understand what the patch is doing before you submit it."
> > +
> > +Testing the changes
> > +---------------
> > +
> > +LLM-generated patches should be coupled with a fully-functional xfstests test case
"...should be submitted with..." ?
(also, don't we call it fstests nowadays?)
> > +which exercises the bug being fixed by the patch. This will not only improve testing
> > +coverage but also provide extra help for reviewers and the maintainer to properly
> > +review and test the changes being made.
>
> This will also help you, the submitter, have confidence that your patch is doing
> what you expect it to do.
"...the person submitting the patch..." ?
> > +Also, every patch/series submitted must be exercised through xfstests suite
> > +- at least - through the auto group (and others depending on the change) as a way
> > +to add extra coverage through the already existing regression cases and help
> > +reviewers/maintainers through the integration process.
What about patches to online fsck, in which case the auto group may or
may not cover it other than incidentally through xfs/28[56]? ;)
How about:
"Every patch submitted must be exercised through the fstests suite
because good test coverage makes review and maintenance processes
easier. Ideally, the change should be exercised by a fstest case in the
"auto" group, but another group may be more appropriate depending on the
change."
--D
next prev parent reply other threads:[~2026-07-09 16:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-09 10:59 [RFC PATCH] xfs: add new policy guidelines for llm-assisted patches cem
2026-07-09 12:30 ` Eric Sandeen
2026-07-09 16:33 ` Darrick J. Wong [this message]
2026-07-14 12:42 ` Brian Foster
2026-07-14 16:03 ` Darrick J. Wong
2026-07-14 19:30 ` Brian Foster
2026-07-14 20:01 ` Darrick J. Wong
2026-07-14 20:25 ` Darrick J. Wong
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=20260709163352.GD15210@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=dgc@kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@sandeen.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox