Linux XFS filesystem development
 help / color / mirror / Atom feed
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

  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