| Message ID | 20200414113222.16959-1-vbabka@suse.cz (mailing list archive) |
|---|---|
| Headers | show |
| Series | support setting sysctl parameters from kernel command line | expand |
Hi Vlastimil, On Tue, 14 Apr 2020 13:32:19 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > This series adds support for something that seems like many people always > wanted but nobody added it yet, so here's the ability to set sysctl parameters > via kernel command line options in the form of sysctl.vm.something=1 Sounds good. And would you consider to use the bootconfig instead of (or in addition to) the kernel command line, because it is too short to describe the sysctl options? With the bootconfig, you can describe the sysctl parameters in an independent file as same as /etc/sysctl.conf. It is easy to convert form sysctl.conf to bootconfig because bootconfig format is simply enhanced structured sysctl.conf :). What we just need is; (echo "sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf bootconfig -a sysctl.bconf /boot/initrd.img Even with only your patch, since bootconfig can pass the options which start with "kernel." prefix to kernel command line, so; (echo "kernel.sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf bootconfig -a sysctl.bconf /boot/initrd.img should work. Thank you,
On Wed, Apr 15, 2020 at 12:23:59PM +0900, Masami Hiramatsu wrote: > Hi Vlastimil, > > On Tue, 14 Apr 2020 13:32:19 +0200 > Vlastimil Babka <vbabka@suse.cz> wrote: > > > This series adds support for something that seems like many people always > > wanted but nobody added it yet, so here's the ability to set sysctl parameters > > via kernel command line options in the form of sysctl.vm.something=1 > > Sounds good. And would you consider to use the bootconfig instead of (or > in addition to) the kernel command line, because it is too short to describe > the sysctl options? FWIW for the lazy: The cmdline limitation: Documentation/admin-guide/kernel-parameters.rst ``` The number of kernel parameters is not limited, but the length of the complete command line (parameters including spaces etc.) is limited to a fixed number of characters. This limit depends on the architecture and is between 256 and 4096 characters. It is defined in the file ./include/asm/setup.h as COMMAND_LINE_SIZE. ``` The bootconfig limitation: Documentation/admin-guide/bootconfig.rst ``` Currently the maximum config size size is 32KB and the total key-words (not key-value entries) must be under 1024 nodes. Note: this is not the number of entries but nodes, an entry must consume more than 2 nodes (a key-word and a value). So theoretically, it will be up to 512 key-value pairs. If keys contains 3 words in average, it can contain 256 key-value pairs. In most cases, the number of config items will be under 100 entries and smaller than 8KB, so it would be enough. If the node number exceeds 1024, parser returns an error even if the file size is smaller than 32KB. Anyway, since bootconfig command verifies it when appending a boot config to initrd image, user can notice it before boot. ``` *recommending* bootconfig due to the limitation of cmdline seems sensible, however if we advise that.. wouldn't the space for 512 theoretical entries full up rather fast? Luis
On 4/15/20 5:23 AM, Masami Hiramatsu wrote: > Hi Vlastimil, > > On Tue, 14 Apr 2020 13:32:19 +0200 > Vlastimil Babka <vbabka@suse.cz> wrote: > >> This series adds support for something that seems like many people always >> wanted but nobody added it yet, so here's the ability to set sysctl parameters >> via kernel command line options in the form of sysctl.vm.something=1 > > Sounds good. And would you consider to use the bootconfig instead of (or > in addition to) the kernel command line, because it is too short to describe > the sysctl options? "Instead of" - no, as that would defeat the scenario of "I just want to set this one sysctl in grub (possibly interactively) and not update initrd for that". If constructing bootconfig is of similar effort of loading sysctl.conf from initrd, then I see little benefit? "in addition to" - sure! but I hoped that's what already happens as it seemed to me that options from bootconfig are appended to the command line that's then parsed by everyone else, no? But I'll try it to be sure. > With the bootconfig, you can describe the sysctl parameters in an > independent file as same as /etc/sysctl.conf. It is easy to convert > form sysctl.conf to bootconfig because bootconfig format is simply > enhanced structured sysctl.conf :). What we just need is; > > (echo "sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf > bootconfig -a sysctl.bconf /boot/initrd.img > > Even with only your patch, since bootconfig can pass the options which > start with "kernel." prefix to kernel command line, so; > > (echo "kernel.sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf > bootconfig -a sysctl.bconf /boot/initrd.img Hmm I hope I figure out if the way virtme creates initrd on the fly supports hooking a bootconfig addition :) > should work. > > Thank you, > >
On Wed 15-04-20 10:56:35, Vlastimil Babka wrote: > On 4/15/20 5:23 AM, Masami Hiramatsu wrote: > > Hi Vlastimil, > > > > On Tue, 14 Apr 2020 13:32:19 +0200 > > Vlastimil Babka <vbabka@suse.cz> wrote: > > > >> This series adds support for something that seems like many people always > >> wanted but nobody added it yet, so here's the ability to set sysctl parameters > >> via kernel command line options in the form of sysctl.vm.something=1 > > > > Sounds good. And would you consider to use the bootconfig instead of (or > > in addition to) the kernel command line, because it is too short to describe > > the sysctl options? > > "Instead of" - no, as that would defeat the scenario of "I just want to set this > one sysctl in grub (possibly interactively) and not update initrd for that". If > constructing bootconfig is of similar effort of loading sysctl.conf from initrd, > then I see little benefit? > > "in addition to" - sure! but I hoped that's what already happens as it seemed to > me that options from bootconfig are appended to the command line that's then > parsed by everyone else, no? But I'll try it to be sure. Completely agreed! Btw. patches look sensible to me so feel free to add Acked-by: Michal Hocko <mhocko@suse.com>
Hi Luis, On Wed, 15 Apr 2020 06:30:41 +0000 Luis Chamberlain <mcgrof@kernel.org> wrote: > Currently the maximum config size size is 32KB and the total key-words > (not key-value entries) must be under 1024 nodes. Note: this is not > the number of entries but nodes, an entry must consume more than 2 nodes > (a key-word and a value). So theoretically, it will be up to 512 > key-value pairs. If keys contains 3 words in average, it can contain 256 > key-value pairs. In most cases, the number of config items will be under > 100 entries and smaller than 8KB, so it would be enough. If the node > number exceeds 1024, parser returns an error even if the file size > is smaller than 32KB. Anyway, since bootconfig command verifies it when > appending a boot config to initrd image, user can notice it before > boot. > ``` > *recommending* bootconfig due to the limitation of cmdline seems > sensible, however if we advise that.. wouldn't the space for 512 > theoretical entries full up rather fast? Yeah, I think it is easier to hit the node number limitation rather than fill up the space. However, since the bootconfig supports comments, if user writes enough readable config file, I think it's probably the right balance :) If you think the 512 entries is too small, it is easy to expand it upto 32K (64K nodes). But it may consume 512KB memory only for the node (meta) data. Current 1024 nodes consumes 8KB (8bytes/node), so compared with the max data size (32KB), I think it is a better balance. Thank you,
On Wed, 15 Apr 2020 10:56:35 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > On 4/15/20 5:23 AM, Masami Hiramatsu wrote: > > Hi Vlastimil, > > > > On Tue, 14 Apr 2020 13:32:19 +0200 > > Vlastimil Babka <vbabka@suse.cz> wrote: > > > >> This series adds support for something that seems like many people always > >> wanted but nobody added it yet, so here's the ability to set sysctl parameters > >> via kernel command line options in the form of sysctl.vm.something=1 > > > > Sounds good. And would you consider to use the bootconfig instead of (or > > in addition to) the kernel command line, because it is too short to describe > > the sysctl options? > > "Instead of" - no, as that would defeat the scenario of "I just want to set this > one sysctl in grub (possibly interactively) and not update initrd for that". If > constructing bootconfig is of similar effort of loading sysctl.conf from initrd, > then I see little benefit? > > "in addition to" - sure! but I hoped that's what already happens as it seemed to > me that options from bootconfig are appended to the command line that's then > parsed by everyone else, no? But I'll try it to be sure. Yes, all configurations under "kernel" key are passed to kernel command line, so you don't need to change anything :) > > With the bootconfig, you can describe the sysctl parameters in an > > independent file as same as /etc/sysctl.conf. It is easy to convert > > form sysctl.conf to bootconfig because bootconfig format is simply > > enhanced structured sysctl.conf :). What we just need is; > > > > (echo "sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf > > bootconfig -a sysctl.bconf /boot/initrd.img > > > > Even with only your patch, since bootconfig can pass the options which > > start with "kernel." prefix to kernel command line, so; > > > > (echo "kernel.sysctl {"; cat "/etc/sysctl.conf"; echo "}") >> sysctl.bconf > > bootconfig -a sysctl.bconf /boot/initrd.img > > Hmm I hope I figure out if the way virtme creates initrd on the fly supports > hooking a bootconfig addition :) Would you mean how to hook the mkinitrd to add /etc/bootconfig? Thank you, >
On Thu, Apr 16, 2020 at 03:02:06PM +0900, Masami Hiramatsu wrote: > Hi Luis, > > On Wed, 15 Apr 2020 06:30:41 +0000 > Luis Chamberlain <mcgrof@kernel.org> wrote: > > Currently the maximum config size size is 32KB and the total key-words > > (not key-value entries) must be under 1024 nodes. Note: this is not > > the number of entries but nodes, an entry must consume more than 2 nodes > > (a key-word and a value). So theoretically, it will be up to 512 > > key-value pairs. If keys contains 3 words in average, it can contain 256 > > key-value pairs. In most cases, the number of config items will be under > > 100 entries and smaller than 8KB, so it would be enough. If the node > > number exceeds 1024, parser returns an error even if the file size > > is smaller than 32KB. Anyway, since bootconfig command verifies it when > > appending a boot config to initrd image, user can notice it before > > boot. > > ``` > > *recommending* bootconfig due to the limitation of cmdline seems > > sensible, however if we advise that.. wouldn't the space for 512 > > theoretical entries full up rather fast? > > Yeah, I think it is easier to hit the node number limitation rather > than fill up the space. However, since the bootconfig supports comments, > if user writes enough readable config file, I think it's probably the > right balance :) > If you think the 512 entries is too small, it is easy to expand it > upto 32K (64K nodes). But it may consume 512KB memory only for the > node (meta) data. Current 1024 nodes consumes 8KB (8bytes/node), so > compared with the max data size (32KB), I think it is a better balance. Yeah, and well at least x86 / x86_64 sets COMMAND_LINE_SIZE to 2048 right now (2 KB), that'd hit the limit of abuse of cmdline pretty fast too. I see no way to scale this reasonably if people abuse syctls on the command line but to use bootconfig and bite the bullet on size, to keep sanity. Luis
This series adds support for something that seems like many people always wanted but nobody added it yet, so here's the ability to set sysctl parameters via kernel command line options in the form of sysctl.vm.something=1 The important part is Patch 1. The second, not so important part is an attempt to clean up legacy one-off parameters that do the same thing as a sysctl. I don't want to remove them completely for compatibility reasons, but with generic sysctl support the idea is to remove the one-off param handlers and treat the parameters as aliases for the sysctl variants. I have identified several parameters that mention sysctl counterparts in Documentation/admin-guide/kernel-parameters.txt but there might be more. The conversion also has varying level of success: - numa_zonelist_order is converted in Patch 2 together with adding the necessary infrastructure. It's easy as it doesn't really do anything but warn on deprecated value these days. - hung_task_panic is converted in Patch 3, but there's a downside that now it only accepts 0 and 1, while previously it was any integer value - nmi_watchdog maps to two sysctls nmi_watchdog and hardlockup_panic, so there's no straighforward conversion possible - traceoff_on_warning is a flag without value and it would be required to handle that somehow in the conversion infractructure, which seems pointless for a single flag Vlastimil Babka (3): kernel/sysctl: support setting sysctl parameters from kernel command line kernel/sysctl: support handling command line aliases kernel/hung_task convert hung_task_panic boot parameter to sysctl Changes since v1: - add missing newlines on printk's - more adjustments to proc mount param passing (Kees) - rebase to 5.7-rc1 - add acks/reviews - test driver (how to pass a testing boot parameter without bootloader specific steps) still under discussion - new Kconfig? bootconfig? .../admin-guide/kernel-parameters.txt | 11 +- fs/proc/proc_sysctl.c | 142 ++++++++++++++++++ include/linux/sysctl.h | 4 + init/main.c | 2 + kernel/hung_task.c | 10 -- mm/page_alloc.c | 9 -- 6 files changed, 158 insertions(+), 20 deletions(-)