Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
On Wed, Sep 06, 2023 at 11:58:47PM +0200, Ingo Molnar wrote: > > * 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%2f...@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. > > > > That needs to be more prominent _somewhere_. Maybe here, or maybe in > > the cover letter, but _somewhere_. > > > > That said, feel free to add this to the two x86 patches: > > > > Acked-by: Dave Hansen # for x86 > > Absolutely needs to be in the title as well, something like: > >arch/x86: Remove now superfluous sentinel elem from ctl_table arrays Done. Will wait to see if other ppl have more comments to send out V2 Thx. > > With that propagated into the whole series: > >Reviewed-by: Ingo Molnar > > Thanks, > > Ingo -- Joel Granados signature.asc Description: PGP signature
Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
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%2f...@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 # for x86 Best -- Joel Granados signature.asc Description: PGP signature
Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
* 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%2f...@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. > > That needs to be more prominent _somewhere_. Maybe here, or maybe in > the cover letter, but _somewhere_. > > That said, feel free to add this to the two x86 patches: > > Acked-by: Dave Hansen # for x86 Absolutely needs to be in the title as well, something like: arch/x86: Remove now superfluous sentinel elem from ctl_table arrays With that propagated into the whole series: Reviewed-by: Ingo Molnar Thanks, Ingo
Re: [PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
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%2f...@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. That needs to be more prominent _somewhere_. Maybe here, or maybe in the cover letter, but _somewhere_. That said, feel free to add this to the two x86 patches: Acked-by: Dave Hansen # for x86
[PATCH 3/8] arch/x86: Remove sentinel elem from ctl_table arrays
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel element from sld_sysctl and itmt_kern_table.
Signed-off-by: Joel Granados
---
arch/x86/kernel/cpu/intel.c | 3 +--
arch/x86/kernel/itmt.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index be4045628fd3..e63391b82624 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1015,8 +1015,7 @@ static struct ctl_table sld_sysctls[] = {
.proc_handler = proc_douintvec_minmax,
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
- },
- {}
+ }
};
static int __init sld_mitigate_sysctl_init(void)
diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
index ee4fe8cdb857..5f2ccff38297 100644
--- a/arch/x86/kernel/itmt.c
+++ b/arch/x86/kernel/itmt.c
@@ -73,8 +73,7 @@ static struct ctl_table itmt_kern_table[] = {
.proc_handler = sched_itmt_update_handler,
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
- },
- {}
+ }
};
static struct ctl_table_header *itmt_sysctl_header;
--
2.30.2
