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

[Vlastimil Babka]

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