Re: [PATCH] lib/string: shrink lib/string.i via IWYU

From: Greg KH <greg@kroah.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	tanzirh@google.com, Kees Cook <keescook@chromium.org>,
	Andy Shevchenko <andy@kernel.org>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nick DeSaulniers <nnn@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	llvm@lists.linux.dev
Subject: Re: [PATCH] lib/string: shrink lib/string.i via IWYU
Date: Wed, 6 Dec 2023 08:46:50 +0900	[thread overview]
Message-ID: <2023120657-henna-spongy-9ef6@gregkh> (raw)
In-Reply-To: <CAKwvOdmFJ=ZGN8ZScS5oQpXnAL0wwtTDCeNNGpBKZXzQ4kRAVA@mail.gmail.com>

On Tue, Dec 05, 2023 at 02:14:55PM -0800, Nick Desaulniers wrote:
> On Tue, Dec 5, 2023 at 1:59 PM Greg KH <greg@kroah.com> wrote:
> >
> > On Tue, Dec 05, 2023 at 01:51:10PM -0800, Nick Desaulniers wrote:
> > > On Tue, Dec 5, 2023 at 1:38 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > >
> > > > It also breeds includes of asm/*.h, by the look of the output, which is
> > > > not a good thing in general ;-/  E.g. #include <asm/uaccess.h> *anywhere*
> > > > outside of linux/uaccess.h is a bad idea.
> > >
> > > It's not clear to me when it's ok to #include <asm/*.h>.  Is there a
> > > convention here that I'm missing?
> >
> > General rule, NEVER include asm/*.h, there should be a include/*.h
> > instead that works.  So much so that checkpatch.pl should catch this,
> > right?
> 
> ah, shoot, I was showing Tanzir how to use `b4` for patch development,
> and forgot to check this.  Indeed it does.
> 
> I can see how the check works (scripts/checkpatch.pl L5881).  Decoding
> that will probably help us improve the tooling.
> 
> >
> > But of course, it doesn't always hold true, there are a few minor
> > exceptions, but they are rare.
> 
> $ grep -r \\#include lib | grep asm
> 
> shows quite a few exceptions, and just in lib/.
> 
> For example, lib/math/int_log.c includes asm/bug.h.  Is that a case
> where lib/math/int_log.c should be #include 'ing linux/bug.h rather
> than asm/bug.h?

Probably yes, but we don't normally go back and take coding style fixes
for old files like this as it doesn't make much sense to do so.

But, if you are cleaning up the headers for large portions with the goal
of faster builds, that's a good reason.

Good luck!

greg k-h