Re: [PATCH 00/17] Add memberof(), split some headers, and slightly simplify code

Hi Arnd,

On 11/19/21 16:57, Arnd Bergmann wrote:

From what I can tell, linux/stddef.h is tiny, I don't think it's really
worth optimizing this part. I have spent some time last year
trying to untangle some of the more interesting headers, but ended
up not completing this as there are some really hard problems
once you start getting to the interesting bits.

In this case it was not about being worth it or not,
but that the fact that adding memberof() would break,
unless I use 0 instead of NULL for the implementation of memberof(),
which I'm against,
or I split stddef.

If I don't do either of those,
I'm creating a circular dependency,
and it doesn't compile.

The approach I tried was roughly:

- For each header in the kernel, create a preprocessed version
  that includes all the indirect includes, from that start a set
  of lookup tables that record which header is eventually included
  by which ones, and the size of each preprocessed header in
  bytes

- For a given kernel configuration (e.g. defconfig or allmodconfig)
  that I'm most interested in, look at which files are built, and what
  the direct includes are in the source files.

- Sort the headers by the product of the number of direct includes
  and the preprocessed size: the largest ones are those that are
  worth looking at first.

- use graphviz to visualize the directed graph showing the includes
  between the top 100 headers in that list. You get something like
  I had in [1], or the version afterwards at [2].

- split out unneeded indirect includes from the headers in the center
  of that graph, typically by splitting out struct definitions.

- repeat.

The main problem with this approach is that as soon as you start
actually reducing the unneeded indirect includes, you end up with
countless .c files that no longer build because they are missing a
direct include for something that was always included somewhere
deep underneath, so I needed a second set of scripts to add
direct includes to every .c file.

On the plus side, I did see something on the order of a 30%
compile speed improvement with clang, which is insane
given that this only removed dead definitions.

Huh!

I'd like to see the kernel some day
not having _any_ hidden dependencies.

For the moment,
since my intent is familiarizing with kernel programming,
and not necessarily improving performance considerably
(at least not in the first rounds of changes),
I prefer starting where it more directly affects
what I initially intended to change in the kernel,
which in this case was adding memberof().

quoted

But I'll note that linux/fs.h, linux/sched.h, linux/mm.h are
interesting headers for further splitting.


BTW, I also have a longstanding doubt about
how header files are organized in the kernel,
and which headers can and cannot be included
from which other files.

For example I see that files in samples or scripts or tools,
that redefine many things such as offsetof() or ARRAY_SIZE(),
and I don't know if there's a good reason for that,
or if I should simply remove all that stuff and
include <linux/offsetof.h> everywhere I see offsetof() being used.

The main issue here is that user space code should not
include anything outside of include/uapi/ and arch/*/include/uapi/

Okay.  That's good to know.

So everything can use uapi code,
and uapi code can only use uapi code,
right?

Every duplicate definition of something outside of uapi
should/could be removed.

offsetof() is defined in include/linux/stddef.h, so this is by
definition not accessible here. It appears that there is also
an include/uapi/linux/stddef.h that is really strange because
it includes linux/compiler_types.h, which in turn is outside
of uapi/. This should probably be fixed.

I see.
Then,
perhaps it would be better to define offsetof() _only_ inside uapi/,
and use that definition from everywhere else,
and therefore remove the non-uapi version,
right?

Thanks,
Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/