linux-kernel - Re: [PATCH v4 1/2] gpiolib: move all includes to the top of gpio/consumer.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z7X2We_IB2oShJ5w@smile.fi.intel.com>
Date: Wed, 19 Feb 2025 17:18:49 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Dipen Patel <dipenp@...dia.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	timestamp@...ts.linux.dev,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v4 1/2] gpiolib: move all includes to the top of
 gpio/consumer.h

On Wed, Feb 19, 2025 at 06:35:59AM -0800, Bartosz Golaszewski wrote:
> On Wed, 19 Feb 2025 13:32:49 +0100, Andy Shevchenko
> <andriy.shevchenko@...el.com> said:
> > On Mon, Feb 17, 2025 at 11:39:21AM +0100, Bartosz Golaszewski wrote:
> >> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >>
> >> We have several conditional includes depending on !CONFIG_GPIOLIB. This
> >> is supposed to reduce compilation time with CONFIG_GPIOLIB=y but in
> >> practice there's no difference on modern machines.
> >
> > It's not about modern machines. If every maintainer will think this way,
> > we end up in the complete and utter dead end with the headers.
> >
> > I believe you at least had read the cover letter for the infamous Ingo's series
> > about headers and how it speeds up the build (in some cases up to 70% on as you
> > said "modern machines").
> >
> >> It makes adding new stubs that depend on more than just GPIOLIB harder so
> >> move them all to the top, unduplicate them and replace asm/ with preferred
> >> linux/ alternatives.
> >
> > NAK.
> >
> > This makes dependency hell things much worse and this is a step back on the
> > untangling the current situation along with the slowing down the speed of the
> > build. Please. consider to revert or discard this patch.
> >
> > ...
> >
> >>  #include <linux/bits.h>
> >> +#include <linux/bug.h>
> >
> > Okay to replace, but not okay to move.
> >
> >>  #include <linux/err.h>
> >> +#include <linux/errno.h>
> >
> > Please, double check that it uses error codes from it, otherwise err.h includes
> > asm/errno.h with basic codes already.
> >
> >> +#include <linux/kernel.h>
> >
> > This is definitely no. Please, read what's written in the top of that file and
> > here is just a proxy for should come in the future a kind of might_sleep.h.
> > Do not move this one at all, please.
> >
> >>  #include <linux/types.h>


Fair enough. Does this look right to you?

For kernel.h definitely, for err.h you proved your point (but which was unclear
to me as the repetitions are already being in a number).

For bug.h looks also good. But I prefer to use asm/bug.h as it's the one that
provides the APIs. Note, the reference to the recommended linux/* headers
rather than asm/* applies to the C code or custom headers which are not under
include/*. The latter should be optimized to what it uses exactly.

So, summarizing the above I would return kernel.h to where it belongs and
move back to asm/bug.h.

-- 
With Best Regards,
Andy Shevchenko