Hi Vlastimil,
On Tue, 14 Apr 2020 13:32:19 +0200
Vlastimil Babka <[email protected]> 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,
--
Masami Hiramatsu <[email protected]>
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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
--
Michal Hocko
SUSE Labs
Hi Luis,
On Wed, 15 Apr 2020 06:30:41 +0000
Luis Chamberlain <[email protected]> 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,
--
Masami Hiramatsu <[email protected]>
On Wed, 15 Apr 2020 10:56:35 +0200
Vlastimil Babka <[email protected]> wrote:
> On 4/15/20 5:23 AM, Masami Hiramatsu wrote:
> > Hi Vlastimil,
> >
> > On Tue, 14 Apr 2020 13:32:19 +0200
> > Vlastimil Babka <[email protected]> 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,
>
--
Masami Hiramatsu <[email protected]>
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 <[email protected]> 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