| Message ID | 20240501-jag-sysctl_remset_net-v6-0-370b702b6b4a@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
2024-05-01, 11:29:24 +0200, Joel Granados via B4 Relay wrote: > From: Joel Granados <j.granados@samsung.com> > > What? > These commits remove the sentinel element (last empty element) from the > sysctl arrays of all the files under the "net/" directory that register > a sysctl array. The merging of the preparation patches [4] to mainline > allows us to just remove sentinel elements without changing behavior. > This is safe because the sysctl registration code (register_sysctl() and > friends) use the array size in addition to checking for a sentinel [1]. > > Why? > By removing the sysctl sentinel elements we avoid kernel bloat as > ctl_table arrays get moved out of kernel/sysctl.c into their own > respective subsystems. This move was started long ago to avoid merge > conflicts; the sentinel removal bit came after Mathew Wilcox suggested > it to avoid bloating the kernel by one element as arrays moved out. This > patchset will reduce the overall build time size of the kernel and run > time memory bloat by about ~64 bytes per declared ctl_table array (more > info here [5]). > > When are we done? > There are 4 patchest (25 commits [2]) that are still outstanding to > completely remove the sentinels: files under "net/" (this patchset), > files under "kernel/" dir, misc dirs (files under mm/ security/ and > others) and the final set that removes the unneeded check for ->procname > == NULL. > > Testing: > * Ran sysctl selftests (./tools/testing/selftests/sysctl/sysctl.sh) > * Ran this through 0-day with no errors or warnings > > Savings in vmlinux: > A total of 64 bytes per sentinel is saved after removal; I measured in > x86_64 to give an idea of the aggregated savings. The actual savings > will depend on individual kernel configuration. > * bloat-o-meter > - The "yesall" config saves 3976 bytes (bloat-o-meter output [6]) > - A reduced config [3] saves 1263 bytes (bloat-o-meter output [7]) > > Savings in allocated memory: > None in this set but will occur when the superfluous allocations are > removed from proc_sysctl.c. I include it here for context. The > estimated savings during boot for config [3] are 6272 bytes. See [8] > for how to measure it. > > Comments/feedback greatly appreciated > > Changes in v6: > - Rebased onto net-next/main. > - Besides re-running my cocci scripts, I ran a new find script [9]. > Found 0 hits in net/ > - Moved "i" variable declaraction out of for() in sysctl_core_net_init > - Removed forgotten sentinel in mpls_table > - Removed CONFIG_AX25_DAMA_SLAVE guard from net/ax25/ax25_ds_timer.c. It > is not needed because that file is compiled only when > CONFIG_AX25_DAMA_SLAVE is set. > - When traversing smc_table, stop on ARRAY_SIZE instead of ARRAY_SIZE-1. > - Link to v5: https://lore.kernel.org/r/20240426-jag-sysctl_remset_net-v5-0-e3b12f6111a6@samsung.com I pointed out a few tiny details in the ax25 patch but either way, the series looks good to me. Thanks! Series: Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Note that you could have kept the ack/reviewed-by on patch 4 since it was not modified. Jeff and Chuck, your reviews got lost in the repost.
On Wed, May 01, 2024 at 03:22:08PM +0200, Sabrina Dubroca wrote: > 2024-05-01, 11:29:24 +0200, Joel Granados via B4 Relay wrote: > > From: Joel Granados <j.granados@samsung.com> > > ... > > Changes in v6: > > - Rebased onto net-next/main. > > - Besides re-running my cocci scripts, I ran a new find script [9]. > > Found 0 hits in net/ > > - Moved "i" variable declaraction out of for() in sysctl_core_net_init > > - Removed forgotten sentinel in mpls_table > > - Removed CONFIG_AX25_DAMA_SLAVE guard from net/ax25/ax25_ds_timer.c. It > > is not needed because that file is compiled only when > > CONFIG_AX25_DAMA_SLAVE is set. > > - When traversing smc_table, stop on ARRAY_SIZE instead of ARRAY_SIZE-1. > > - Link to v5: https://lore.kernel.org/r/20240426-jag-sysctl_remset_net-v5-0-e3b12f6111a6@samsung.com > > I pointed out a few tiny details in the ax25 patch but either way, the > series looks good to me. Thanks! > > Series: > Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> Thx > > Note that you could have kept the ack/reviewed-by on patch 4 since it > was not modified. Jeff and Chuck, your reviews got lost in the repost. Indeed. I have been having issues with my b4. Have posted this https://lore.kernel.org/tools/20240503120506.p7g5nn6jocrrdlck@joelS2.panther.com/ which explains my situation. Best
diff --git i/fs/proc/proc_sysctl.c w/fs/proc/proc_sysctl.c index 37cde0efee57..896c498600e8 100644 --- i/fs/proc/proc_sysctl.c +++ w/fs/proc/proc_sysctl.c @@ -966,6 +966,7 @@ static struct ctl_dir *new_dir(struct ctl_table_set *set, table[0].procname = new_name; table[0].mode = S_IFDIR|S_IRUGO|S_IXUGO; init_header(&new->header, set->dir.header.root, set, node, table, 1); + printk("%ld sysctl saved mem kzalloc\n", sizeof(struct ctl_table)); return new; } @@ -1189,6 +1190,7 @@ static struct ctl_table_header *new_links(struct ctl_dir *dir, s> link_name += len; link++; } + printk("%ld sysctl saved mem kzalloc\n", sizeof(struct ctl_table)); init_header(links, dir->header.root, dir->header.set, node, link_table, head->ctl_table_size); links->nreg = nr_entries;