Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl

[Joel Granados]

On Wed, Jul 26, 2023 at 11:15:40AM -0700, Luis Chamberlain wrote:
> On Wed, Jul 26, 2023 at 04:06:20PM +0200, Joel Granados wrote:
> > What?
> > These commits set things up so we can start removing the sentinel elements.
> 
> Yes but the why must explained right away.
My intention of putting the "what" first was to explain the chunking and
clarify right away that what was contained in the "why" was not for this
chunk but for the *whole* patchset.

I have swapped them in my cover letter as I can see that the ambiguity
is gone once you start reading the "what"

> 
> > Why?
> > This is part of the push to trim down kernel/sysctl.c by moving the large array
> > that was causing merge conflicts. 
> 
> Let me elaborate on that:
> 
> While the move moving over time of array elements out of kernel/sysctl.c
> to their own place helps merge conflicts this patch set does not help
> with that in and of itself, what it does is help make sure the move of
> sysctls to their own files does not bloat the kernel more, and in fact
> helps reduce the overall build time size of the kernel and run time
> memory consumed by the kernel by about ~64 bytes per array.
> 
> Without this patch set each time we moved a set of sysctls out of
> kernel/sysctl.c to its own subsystem we'd have to add a new sentinel
> element (an empty sysctl entry), and while that helps clean up
> kernel/sysctl.c to avoid merge conflicts, it also bloats the kernel
> by about 64 bytes on average each time.
> 
> We can do better. We can make those moves *not* have a size penalty, and
> all around also reduce the build / run time of the kernel.
> 
> *This* is the why, that if we don't do this the cleanup of
> kernel/sysctl.c ends up slowly bloating the kernel. Willy had
> suggested we instead remove the sentinel so that each move does not
> incur a size penalty, but also that in turn reduces the size of the
> kernel at build time / run time by a ballpark about ~64 bytes per
> array.
Thx for this.
This is a more clear wording for the "Why". Do you mind if I copy/paste
it (with some changes to make it flow) into my next cover letter?

> 
> Then the following is more details about estimates of overall size
> savings, it's not miscellaneous information at all, it's very relevant
> information to this patch set.
Did not mean to downplay the importance here. Just did not have a good
title for the section. I'll change it to "Size saving estimates".
> 
> > Misc:
> > A consequence of eventually removing all the sentinels (64 bytes per sentinel)
> > is the bytes we save. Here I include numbers for when all sentinels are removed
> > to contextualize this chunk
> >   * bloat-o-meter:
> >     The "yesall" configuration results save 9158 bytes (you can see the output here
> >     https://lore.kernel.org/all/20230621091000.424843-1-j.granados@xxxxxxxxxxx/.
> >     The "tiny" configuration + CONFIG_SYSCTL save 1215 bytes (you can see the
> >     output here [2])
> >   * memory usage:
> >     As we no longer need the sentinel element within proc_sysctl.c, we save some
> >     bytes in main memory as well. In my testing kernel I measured a difference of
> >     6720 bytes. I include the way to measure this in [1]
> 
>   Luis

-- 

best

Joel Granados

Attachment: signature.asc
Description: PGP signature