Re: [PATCH] mm/vmscan: add vm_swappiness configuration knobs

From: Vlastimil Babka
Date: Mon Mar 16 2020 - 10:53:26 EST


On 3/12/20 2:26 PM, Michal Hocko wrote:
> On Thu 12-03-20 12:54:19, Ivan Teterevkov wrote:
>>
>> Absolutely agree, the semantics of the vm_swappiness is perplexing.
>> Moreover, the same get_scan_count treats vm_swappiness and cgroups
>> memory.swappiness differently, in particular, 0 disables the memcg swap.
>>
>> Certainly, the patch adds some additional exposure to a parameter that
>> is not trivial to tackle but it's already getting created with a magic
>> number which is also confusing. Is there any harm to be done by the patch
>> considering the already existing sysctl interface to that knob?
>
> Like any other config option/kernel parameter. It is adding the the
> overall config space size problem and unless this is really needed I
> would rather not make it worse.

Setting the vm_swappiness specific case aside, I wonder if if would be
useful to be able to emulate any sysctl with a kernel parameter,
i.e. boot the kernel with sysctl.vm.swappiness=X
There are already some options that provide kernel parameter as well
as sysctl, why not just support all.
Quick and dirty proof of concept:

----8<-----
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 02fa84493f23..62ae963a5c0c 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -206,6 +206,7 @@ struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
void unregister_sysctl_table(struct ctl_table_header * table);

extern int sysctl_init(void);
+int process_sysctl_arg(char *param, char *val, const char *unused, void *arg);

extern struct ctl_table sysctl_mount_point[];

diff --git a/init/main.c b/init/main.c
index ee4947af823f..c1544ff4ec5b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1345,6 +1345,23 @@ void __weak free_initmem(void)
free_initmem_default(POISON_FREE_INITMEM);
}

+static void do_sysctl_args(void)
+{
+ size_t len = strlen(saved_command_line) + 1;
+ char *command_line;
+
+ command_line = kzalloc(len, GFP_KERNEL);
+ if (!command_line)
+ panic("%s: Failed to allocate %zu bytes\n", __func__, len);
+
+ strcpy(command_line, saved_command_line);
+
+ parse_args("Setting sysctl args", command_line,
+ NULL, 0, -1, -1, NULL, process_sysctl_arg);
+
+ kfree(command_line);
+}
+
static int __ref kernel_init(void *unused)
{
int ret;
@@ -1367,6 +1384,8 @@ static int __ref kernel_init(void *unused)

rcu_end_inkernel_boot();

+ do_sysctl_args();
+
if (ramdisk_execute_command) {
ret = run_init_process(ramdisk_execute_command);
if (!ret)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ad5b88a53c5a..5b3b520d29a8 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1980,6 +1980,66 @@ int __init sysctl_init(void)
return 0;
}

+int process_sysctl_arg(char *param, char *val,
+ const char *unused, void *arg)
+{
+ size_t count;
+ char *tmp;
+ int err;
+ loff_t ppos = 0;
+ struct ctl_table *base, *child = NULL, *found = NULL;
+
+ if (strncmp(param, "sysctl.", sizeof("sysctl.") - 1))
+ return 0;
+
+ param += (sizeof("sysctl.") - 1);
+
+ pr_notice("sysctl: %s=%s", param, val);
+
+ tmp = strchr(param, '.');
+ if (!tmp) {
+ pr_notice("invalid sysctl param '%s' on command line", param);
+ return 0;
+ }
+
+ *tmp = '\0';
+
+ for (base = &sysctl_base_table[0]; base->procname != 0; base++) {
+ if (strcmp(param, base->procname) == 0) {
+ child = base->child;
+ break;
+ }
+ }
+
+ if (!child) {
+ pr_notice("unknown sysctl prefix '%s' on command line", param);
+ return 0;
+ }
+
+ tmp++;
+
+ for (; child->procname != 0; child++) {
+ if (strcmp(tmp, child->procname) == 0) {
+ found = child;
+ break;
+ }
+ }
+
+ if (!found) {
+ pr_notice("unknown sysctl param '%s.%s' on command line", param, tmp);
+ return 0;
+ }
+
+ count = strlen(val);
+ err = found->proc_handler(found, 1, val, &count, &ppos);
+
+ if (err)
+ pr_notice("error %d setting sysctl '%s.%s' from command line",
+ err, param, tmp);
+
+ return 0;
+}
+
#endif /* CONFIG_SYSCTL */

/*
--
2.25.1