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

From: Luis Chamberlain <mcgrof@kernel.org>
To: Joel Granados <j.granados@samsung.com>
Cc: Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>,
	willy@infradead.org, josh@joshtriplett.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH 00/14] sysctl: Add a size argument to register functions in sysctl
Date: Wed, 26 Jul 2023 11:15:40 -0700	[thread overview]
Message-ID: <ZMFizKFkVxUFtSqa@bombadil.infradead.org> (raw)
In-Reply-To: <20230726140635.2059334-1-j.granados@samsung.com>

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.

> 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.

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.

> 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@samsung.com/.
>     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