Re: checkpatch encouragement improvements (was RE: [MAINTAINERS / KERNEL SUMMIT] AI patch review tools)

[Rob Herring <robh@kernel.org>]

On Fri, Oct 10, 2025 at 11:03 AM Bird, Tim <Tim.Bird@sony.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Joe Perches <joe@perches.com>
> > On Fri, 2025-10-10 at 14: 15 +0000, Bird, Tim wrote: > I have ideas to address the false positive rate, based on features that checkpatch. pl >
> > already has, as well as ideas for handling some of the concerns that running > checkpatch. pl
> > On Fri, 2025-10-10 at 14:15 +0000, Bird, Tim wrote:
> > > I have ideas to address the false positive rate, based on features that checkpatch.pl
> > > already has, as well as ideas for handling some of the concerns that running
> > > checkpatch.pl (or an equivalent) at build time would raise.  Some of these might
> > > apply to AI review as well.  Let me know if you want me to elaborate, or if we
> > > should just discuss in Tokyo.
> >
> > Please elaborate.
>
> Before I rattled off my ideas, I did a quick check to see if anything like what I had
> in mind had already been suggested, and I found this:
> https://lore.kernel.org/all/cover.3dd1d0c1f15a7a7c418f20ebafc9719afc1c2704.1510840787.git-series.knut.omang@oracle.com/
>
> That's a submission by Knut Omang from 2017 that adds support for running
> checkpatch.pl on the whole kernel source, integrated into kbuild.
>
> It looks like it was not adopted.
>
> That patchset allows:
> 1) fine-tuning the set of items that checkpatch checks for
> 2) using a config file to control the fine tuning
> 3) allowing selection of the groups of checks (so that preferences of
> individual maintainers or sub-systems could be adhered to)
>
> My ideas for tackling false-positives was along those same lines.
> 1) determine a list of "must-have" items, that everyone supports as true-positives
> 2) having a way to designate groups of items that were deemed as true-positives
>      - by creating a profile of items, specific to a subsystem or maintainer
>      - having the 'true-positives' profile under the control of the maintainer or subsystem
> 3) creating a mechanism to tell checkpatch which profile to use
>
> In terms of runtime issues, there are two ways to "encourage (/force?)" users
> to run the checks, while still managing overhead:
> 1) add a checkpatch.pl invocation sometime during a regular build (either
> before or after the compiler invocation)
>      - My idea to avoid overhead here is to add a command-line option to the kernel
>        build, to indicate the number of commits to include in patch review (default to 1), and only
>        run checkpatch on either a) each commit itself, or b) the files touched by each commit.
>        - OR,  only run checkpatch on source files that differ from the git index.
> 2) alternatively, add a gcc plugin that performs some set of checks similar to checkpatch.pl,
>     maybe calling checkpatch itself, but maybe just directly checking for definite true-positives
>     itself.  This would involve migrating the true-positives check from checkpatch into a gcc plugin.
>     - then dealing with clang builds
>
> I think there would be too much noise, and too much latency, to run
> checkpatch on the entire kernel source on every build.
>
> But my goal here is to have checkpatch issues show up like gcc errors - early in development,
> as the errors are made, so they never make it into patch emails.

Running on the source wouldn't expose things which only show up in the
patch. Missing or other S-o-b errors, missing commit msg, using 'This
patch/commit...', etc. I think it would be good if we ran checkpatch
on every patch, but start with only checks with no false positives and
ones every maintainer agrees on. Then we can add to it from there
perhaps enabling per subsystem checks.

Rob