| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <64fd22df-616e-9f5a-26fb-44c4b3423b0c@csgroup.eu>
Date: Mon, 2 Oct 2023 12:27:18 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "j.granados@...sung.com" <j.granados@...sung.com>,
Luis Chamberlain <mcgrof@...nel.org>,
"willy@...radead.org" <willy@...radead.org>,
"josh@...htriplett.org" <josh@...htriplett.org>,
Kees Cook <keescook@...omium.org>,
Phillip Potter <phil@...lpotter.co.uk>,
Clemens Ladisch <clemens@...isch.de>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Juergen Gross <jgross@...e.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
Jiri Slaby <jirislaby@...nel.org>,
"James E.J. Bottomley" <jejb@...ux.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Doug Gilbert <dgilbert@...erlog.com>,
Sudip Mukherjee <sudipm.mukherjee@...il.com>,
Jason Gunthorpe <jgg@...pe.ca>,
Leon Romanovsky <leon@...nel.org>,
Corey Minyard <minyard@....org>, Theodore Ts'o <tytso@....edu>,
"Jason A. Donenfeld" <Jason@...c4.com>,
David Ahern <dsahern@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Robin Holt <robinmholt@...il.com>,
Steve Wahl <steve.wahl@....com>,
Russ Weight <russell.h.weight@...el.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Song Liu <song@...nel.org>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"openipmi-developer@...ts.sourceforge.net"
<openipmi-developer@...ts.sourceforge.net>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH v2 00/15] sysctl: Remove sentinel elements from drivers
Le 02/10/2023 à 10:55, Joel Granados via B4 Relay a écrit :
> From: Joel Granados <j.granados@...sung.com>
>
> What?
> These commits remove the sentinel element (last empty element) from the
> sysctl arrays of all the files under the "drivers/" directory that use a
> sysctl array for registration. The merging of the preparation patches
> (in https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
> to mainline allows us to just remove sentinel elements without changing
> behavior (more info here [1]).
>
> These commits are part of a bigger set (here
> https://github.com/Joelgranados/linux/tree/tag/sysctl_remove_empty_elem_V4)
> that remove the ctl_table sentinel. Make the review process easier by
> chunking the commits into manageable pieces. Each chunk can be reviewed
> separately without noise from parallel sets.
>
> Now that the architecture chunk has been mostly reviewed [6], we send
> the "drivers/" directory. Once this one is done, it will be follwed by
> "fs/*", "kernel/*", "net/*" and miscellaneous. The final set will remove
> the unneeded check for ->procname == NULL.
>
> 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. I
> have consolidated some links that shed light on the history of this
> effort [2].
>
> Testing:
> * Ran sysctl selftests (./tools/testing/selftests/sysctl/sysctl.sh)
> * Ran this through 0-day with no errors or warnings
>
> Size saving after removing all sentinels:
> These are the bytes that we save after removing all the sentinels
> (this plus all the other chunks). I included them to get an idea of
> how much memory we are talking about.
> * bloat-o-meter:
> - The "yesall" configuration results save 9158 bytes
> https://lore.kernel.org/all/20230621091000.424843-1-j.granados@samsung.com/
> - The "tiny" config + CONFIG_SYSCTL save 1215 bytes
> https://lore.kernel.org/all/20230809105006.1198165-1-j.granados@samsung.com/
> * memory usage:
> In memory savings are measured to be 7296 bytes. (here is how to
> measure [3])
>
> Size saving after this patchset:
> * bloat-o-meter
> - The "yesall" config saves 2432 bytes [4]
> - The "tiny" config saves 64 bytes [5]
> * memory usage:
> In this case there were no bytes saved because I do not have any
> of the drivers in the patch. To measure it comment the printk in
> `new_dir` and uncomment the if conditional in `new_links` [3].
>
> ---
> Changes in v2:
> - Left the dangling comma in the ctl_table arrays.
> - Link to v1: https://lore.kernel.org/r/20230928-jag-sysctl_remove_empty_elem_drivers-v1-0-e59120fca9f9@samsung.com
>
> Comments/feedback greatly appreciated
Same problem on powerpc CI tests, all boot target failed, most of them
with similar OOPS, see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20231002-jag-sysctl_remove_empty_elem_drivers-v2-15-02dd0d46f71e@samsung.com/
What is strange is that I pushed your series into my github account, and
got no failure, see https://github.com/chleroy/linux/actions/runs/6378951278
Christophe
>
> Best
>
> Joel
>
> [1]
> We are able to remove a sentinel table without behavioral change by
> introducing a table_size argument in the same place where procname is
> checked for NULL. The idea is for it to keep stopping when it hits
> ->procname == NULL, while the sentinel is still present. And when the
> sentinel is removed, it will stop on the table_size. You can go to
> (https://lore.kernel.org/all/20230809105006.1198165-1-j.granados@samsung.com/)
> for more information.
>
> [2]
> Links Related to the ctl_table sentinel removal:
> * Good summary from Luis sent with the "pull request" for the
> preparation patches.
> https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/
> * Another very good summary from Luis.
> https://lore.kernel.org/all/ZMFizKFkVxUFtSqa@bombadil.infradead.org/
> * This is a patch set that replaces register_sysctl_table with register_sysctl
> https://lore.kernel.org/all/20230302204612.782387-1-mcgrof@kernel.org/
> * Patch set to deprecate register_sysctl_paths()
> https://lore.kernel.org/all/20230302202826.776286-1-mcgrof@kernel.org/
> * Here there is an explicit expectation for the removal of the sentinel element.
> https://lore.kernel.org/all/20230321130908.6972-1-frank.li@vivo.com
> * The "ARRAY_SIZE" approach was mentioned (proposed?) in this thread
> https://lore.kernel.org/all/20220220060626.15885-1-tangmeng@uniontech.com
>
> [3]
> To measure the in memory savings apply this on top of this patchset.
>
> "
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index c88854df0b62..e0073a627bac 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -976,6 +976,8 @@ 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);
> + // Counts additional sentinel used for each new dir.
> + printk("%ld sysctl saved mem kzalloc \n", sizeof(struct ctl_table));
>
> return new;
> }
> @@ -1199,6 +1201,9 @@ static struct ctl_table_header *new_links(struct ctl_dir *dir, struct ctl_table_
> link_name += len;
> link++;
> }
> + // Counts additional sentinel used for each new registration
> + //if ((head->ctl_table + head->ctl_table_size)->procname)
> + 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;
> "
> and then run the following bash script in the kernel:
>
> accum=0
> for n in $(dmesg | grep kzalloc | awk '{print $3}') ; do
> echo $n
> accum=$(calc "$accum + $n")
> done
> echo $accum
>
> [4]
> add/remove: 0/0 grow/shrink: 0/21 up/down: 0/-2432 (-2432)
> Function old new delta
> xpc_sys_xpc_hb 192 128 -64
> xpc_sys_xpc 128 64 -64
> vrf_table 128 64 -64
> ucma_ctl_table 128 64 -64
> tty_table 192 128 -64
> sg_sysctls 128 64 -64
> scsi_table 128 64 -64
> random_table 448 384 -64
> raid_table 192 128 -64
> oa_table 192 128 -64
> mac_hid_files 256 192 -64
> iwcm_ctl_table 128 64 -64
> ipmi_table 128 64 -64
> hv_ctl_table 128 64 -64
> hpet_table 128 64 -64
> firmware_config_table 192 128 -64
> cdrom_table 448 384 -64
> balloon_table 128 64 -64
> parport_sysctl_template 912 720 -192
> parport_default_sysctl_table 584 136 -448
> parport_device_sysctl_template 776 136 -640
> Total: Before=429940038, After=429937606, chg -0.00%
>
> [5]
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-64 (-64)
> Function old new delta
> random_table 448 384 -64
> Total: Before=1885527, After=1885463, chg -0.00%
>
> [6] https://lore.kernel.org/all/20230913-jag-sysctl_remove_empty_elem_arch-v2-0-d1bd13a29bae@samsung.com/
>
> Signed-off-by: Joel Granados <j.granados@...sung.com>
>
> To: Luis Chamberlain <mcgrof@...nel.org>
> To: willy@...radead.org
> To: josh@...htriplett.org
> To: Kees Cook <keescook@...omium.org>
> To: Phillip Potter <phil@...lpotter.co.uk>
> To: Clemens Ladisch <clemens@...isch.de>
> To: Arnd Bergmann <arnd@...db.de>
> To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> To: Juergen Gross <jgross@...e.com>
> To: Stefano Stabellini <sstabellini@...nel.org>
> To: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
> To: Jiri Slaby <jirislaby@...nel.org>
> To: "James E.J. Bottomley" <jejb@...ux.ibm.com>
> To: "Martin K. Petersen" <martin.petersen@...cle.com>
> To: Doug Gilbert <dgilbert@...erlog.com>
> To: Sudip Mukherjee <sudipm.mukherjee@...il.com>
> To: Jason Gunthorpe <jgg@...pe.ca>
> To: Leon Romanovsky <leon@...nel.org>
> To: Corey Minyard <minyard@....org>
> To: Theodore Ts'o <tytso@....edu>
> To: "Jason A. Donenfeld" <Jason@...c4.com>
> To: David Ahern <dsahern@...nel.org>
> To: "David S. Miller" <davem@...emloft.net>
> To: Eric Dumazet <edumazet@...gle.com>
> To: Jakub Kicinski <kuba@...nel.org>
> To: Paolo Abeni <pabeni@...hat.com>
> To: Robin Holt <robinmholt@...il.com>
> To: Steve Wahl <steve.wahl@....com>
> To: Russ Weight <russell.h.weight@...el.com>
> To: "Rafael J. Wysocki" <rafael@...nel.org>
> To: Song Liu <song@...nel.org>
> To: "K. Y. Srinivasan" <kys@...rosoft.com>
> To: Haiyang Zhang <haiyangz@...rosoft.com>
> To: Wei Liu <wei.liu@...nel.org>
> To: Dexuan Cui <decui@...rosoft.com>
> To: Jani Nikula <jani.nikula@...ux.intel.com>
> To: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
> To: Rodrigo Vivi <rodrigo.vivi@...el.com>
> To: Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>
> To: David Airlie <airlied@...il.com>
> To: Daniel Vetter <daniel@...ll.ch>
> Cc: linux-kernel@...r.kernel.org
> Cc: xen-devel@...ts.xenproject.org
> Cc: linux-serial@...r.kernel.org
> Cc: linux-scsi@...r.kernel.org
> Cc: linuxppc-dev@...ts.ozlabs.org
> Cc: linux-rdma@...r.kernel.org
> Cc: openipmi-developer@...ts.sourceforge.net
> Cc: netdev@...r.kernel.org
> Cc: linux-raid@...r.kernel.org
> Cc: linux-hyperv@...r.kernel.org
> Cc: intel-gfx@...ts.freedesktop.org
> Cc: dri-devel@...ts.freedesktop.org
>
> ---
>
> ---
> Joel Granados (15):
> cdrom: Remove now superfluous sentinel element from ctl_table array
> hpet: Remove now superfluous sentinel element from ctl_table array
> xen: Remove now superfluous sentinel element from ctl_table array
> tty: Remove now superfluous sentinel element from ctl_table array
> scsi: Remove now superfluous sentinel element from ctl_table array
> parport: Remove the now superfluous sentinel element from ctl_table array
> macintosh: Remove the now superfluous sentinel element from ctl_table array
> infiniband: Remove the now superfluous sentinel element from ctl_table array
> char-misc: Remove the now superfluous sentinel element from ctl_table array
> vrf: Remove the now superfluous sentinel element from ctl_table array
> sgi-xp: Remove the now superfluous sentinel element from ctl_table array
> fw loader: Remove the now superfluous sentinel element from ctl_table array
> raid: Remove now superfluous sentinel element from ctl_table array
> Drivers: hv: Remove now superfluous sentinel element from ctl_table array
> intel drm: Remove now superfluous sentinel element from ctl_table array
>
> drivers/base/firmware_loader/fallback_table.c | 1 -
> drivers/cdrom/cdrom.c | 1 -
> drivers/char/hpet.c | 1 -
> drivers/char/ipmi/ipmi_poweroff.c | 1 -
> drivers/char/random.c | 1 -
> drivers/gpu/drm/i915/i915_perf.c | 1 -
> drivers/hv/hv_common.c | 1 -
> drivers/infiniband/core/iwcm.c | 1 -
> drivers/infiniband/core/ucma.c | 1 -
> drivers/macintosh/mac_hid.c | 1 -
> drivers/md/md.c | 1 -
> drivers/misc/sgi-xp/xpc_main.c | 2 --
> drivers/net/vrf.c | 1 -
> drivers/parport/procfs.c | 28 +++++++++++----------------
> drivers/scsi/scsi_sysctl.c | 1 -
> drivers/scsi/sg.c | 1 -
> drivers/tty/tty_io.c | 1 -
> drivers/xen/balloon.c | 1 -
> 18 files changed, 11 insertions(+), 35 deletions(-)
> ---
> base-commit: 0e945134b680040b8613e962f586d91b6d40292d
> change-id: 20230927-jag-sysctl_remove_empty_elem_drivers-f034962a0d8c
>
> Best regards,
Powered by blists - more mailing lists