| Message ID | 20231107-jag-sysctl_remove_empty_elem_kernel-v1-7-e4ce1388dfa0@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] kernel misc: Remove the now superfluous sentinel elements from ctl_table array | expand |
On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote: > From: Joel Granados <j.granados@samsung.com> > > 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/) > > rm sentinel element from printk_sysctls > > Signed-off-by: Joel Granados <j.granados@samsung.com> I am a bit sceptical if the size and time reduction is worth the effort. I feel that this change makes the access a bit less secure. Well, almost all arrays are static so that it should just work. The patch does what it says. Feel free to use: Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
Hey Petr I missed this message somehow.... On Tue, Nov 28, 2023 at 03:07:43PM +0100, Petr Mladek wrote: > On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote: > > From: Joel Granados <j.granados@samsung.com> > > > > 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/) > > > > rm sentinel element from printk_sysctls > > > > Signed-off-by: Joel Granados <j.granados@samsung.com> > > I am a bit sceptical if the size and time reduction is worth the > effort. I feel that this change makes the access a bit less secure. In what way "less secure"? Can you expand on that? Notice that if you pass a pointer to the register functions, you will get a warning/error on compilation. > > Well, almost all arrays are static so that it should just work. > The patch does what it says. Feel free to use: Thx for the review. will do. > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > Best Regards, > Petr
On Mon 2023-12-04 09:56:28, Joel Granados wrote: > Hey Petr > > I missed this message somehow.... > > On Tue, Nov 28, 2023 at 03:07:43PM +0100, Petr Mladek wrote: > > On Tue 2023-11-07 14:45:07, Joel Granados via B4 Relay wrote: > > > From: Joel Granados <j.granados@samsung.com> > > > > > > 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/) > > > > > > rm sentinel element from printk_sysctls > > > > > > Signed-off-by: Joel Granados <j.granados@samsung.com> > > > > I am a bit sceptical if the size and time reduction is worth the > > effort. I feel that this change makes the access a bit less secure. > In what way "less secure"? Can you expand on that? > > Notice that if you pass a pointer to the register functions, you will > get a warning/error on compilation. I have vague memories that some arrays were not static or the length has been somehow manipulated. But I might be wrong. You are right that it should be safe with the static arrays. And the NULL sentinel might be more error-prone after all. Let's forget my mumbles. Best Regards, Petr
diff --git a/kernel/printk/sysctl.c b/kernel/printk/sysctl.c index c228343eeb97..3e47dedce9e5 100644 --- a/kernel/printk/sysctl.c +++ b/kernel/printk/sysctl.c @@ -76,7 +76,6 @@ static struct ctl_table printk_sysctls[] = { .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_TWO, }, - {} }; void __init printk_sysctl_init(void)