Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays

From: Joel Granados <j.granados@samsung.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>, <willy@infradead.org>,
	<josh@joshtriplett.org>, Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Sven Schnelle <svens@linux.ibm.com>,
	Gerald Schaefer <gerald.schaefer@linux.ibm.com>,
	Russell King <linux@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andy Lutomirski <luto@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Guo Ren <guoren@kernel.org>, <linux-fsdevel@vger.kernel.org>,
	<linux-s390@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-riscv@lists.infradead.org>,
	<linuxppc-dev@lists.ozlabs.org>, <linux-ia64@vger.kernel.org>,
	<linux-csky@vger.kernel.org>
Subject: Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
Date: Thu, 7 Sep 2023 09:38:18 +0200	[thread overview]
Message-ID: <20230907073818.n5dcc7sqpvzkpeaa@localhost> (raw)
In-Reply-To: <d0d30ad4-7837-b0c4-39f4-3e317e35a41b@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 1822 bytes --]

On Wed, Sep 06, 2023 at 07:45:09AM -0700, Dave Hansen wrote:
> On 9/6/23 03:03, Joel Granados via B4 Relay wrote:
> > This commit comes at the tail end of a greater effort to remove the
> > empty elements at the end of the ctl_table arrays (sentinels) which
> > will reduce the overall build time size of the kernel and run time
> > memory bloat by ~64 bytes per sentinel (further information Link :
> > https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
> > 
> > Remove sentinel element from sld_sysctl and itmt_kern_table.
> 
> There's a *LOT* of content to read for a reviewer to figure out what's
> going on here between all the links.  I would have appreciated one more
> sentence here, maybe:
> 
> 	This is now safe because the sysctl registration code
> 	(register_sysctl()) implicitly uses ARRAY_SIZE() in addition
> 	to checking for a sentinel.
Thx for the feedback. This is a great sentence to add at the end of the
first paragraph instead of the link. I'll add it with a few changes as
there are more than just one register function and the use of ARRAY_SIZE
is implicit most of the time.

  This is now safe because the sysctl registration code
  (register_sysctl() and friends) use the array size in addition to
  checking for a sentinel.

I have changed my cover letter in case I send a V2 and for the other
batches that are coming after the architecture one.

> 
> That needs to be more prominent _somewhere_.  Maybe here, or maybe in
> the cover letter, but _somewhere_.
This is also a good point. I think having it in both the cover letter
and each of the commits is an added value.

> 
> That said, feel free to add this to the two x86 patches:
> 
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com> # for x86

Best

-- 

Joel Granados

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel