Re: [PATCH v3 00/35] Fast kernel headers: reduce header dependencies

From: Mark Rutland
Date: Mon Feb 12 2024 - 09:06:35 EST

Hi Max,

On Sun, Feb 11, 2024 at 01:29:25PM +0100, Max Kellermann wrote:
> This patch set aims to reduce the dependencies between headers, in
> order to have cleaner code and speed up the build. It continues
> previous efforts by other developers.

While the intent of the series is admirable, as it currently stands it's very
painful to review:

* Patch 1 is *gigantic*. 3MiB+ of plain text cannot reasonable be reviewed by a
human. This needs to be split into smaller patches, and that needs a more
thorough commit message (e.g. *how* you determined specific headers were

This could probably be a series on its own, or could be split up along more
logical lines (e.g. have a series cleaning up a *particular* case of indirect

* There have been three versions of the series in two days. We usually expect
several says (e.g. a week) between versions, and posting multiple versions so
quickly just spams reviewers' inboxes and doesn't give people sufficient time
to provide any meaningful review.

> As a preparation, the first patch adds "#include" directives to source
> files that were missing previously, but due to indirect includes, this
> was never noticed. After the cleanup, many missing directives would
> result in a compiler failure.
> The second patch removes superfluous "#include" directives, some of
> which may be a leftover from refactoring patches.
> The third patch replaces existing "#include" directives with narrower
> ones, e.g. use "spinlock_types.h" instead of "spinlock.h". This
> continues the work others have done over the years.
> The remaining patches add new "XXX_types.h" headers with lighter
> dependencies. They have only basic struct/enum/const/macro
> definitions and maybe a few trivial inline functions, but no "extern"
> functions and no complex header dependencies.
> Just like the other attempts to reduce header dependencies in the
> past, this is just the beginning. There are still too many
> dependencies, and the speedup gained by this large patch set is not
> yet impressive.
> Prior to this patch set:
> real 0m34.677s
> user 23m13.045s
> sys 2m26.007s
> With this patch set:
> real 0m34.464s
> user 22m19.073s
> sys 2m15.246s
> (Building the directories kernel,lib,mm on ARM64 "allyesconfig".)
> I have tested this patch set with:
> - ARCH=arm allyesconfig
> - ARCH=arm defconfig
> - ARCH=arm64 allyesconfig
> - ARCH=arm64 defconfig
> - ARCH=mips defconfig
> - ARCH=riscv defconfig
> - ARCH=x86_64 allyesconfig
> - ARCH=xtensa defconfig
> Pretty sure, other architectures may fail to build, but before I test
> all of them, I'd like to get some feedback on whether my approach
> would be accepted.
> For more gains, huge headers like "linux/mm.h", "linux/fs.h" and
> "linux/sched.h" would need to be optimized. Nearly everybody includes
> them, and they include nearly everything.

IIUC the same is true of kernel.h.

How have you analyzed this? Are you using any specific tooling, or has this all
been manual analysis?