Linux-Doc Archive on lore.kernel.org
 help / color / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Michal Hocko <mhocko@kernel.org>,
	Ivan Teterevkov <ivan.teterevkov@nutanix.com>
Cc: "corbet@lwn.net" <corbet@lwn.net>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"mchehab+samsung@kernel.org" <mchehab+samsung@kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"jpoimboe@redhat.com" <jpoimboe@redhat.com>,
	"pawan.kumar.gupta@linux.intel.com" 
	<pawan.kumar.gupta@linux.intel.com>,
	"jgross@suse.com" <jgross@suse.com>,
	"oneukum@suse.com" <oneukum@suse.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH] mm/vmscan: add vm_swappiness configuration knobs
Date: Mon, 16 Mar 2020 15:53:21 +0100
Message-ID: <4ea2e014-17ea-6d1e-a6cd-775fb6550cd2@suse.cz> (raw)
In-Reply-To: <20200312132642.GW23944@dhcp22.suse.cz>

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





  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 17:45 Ivan Teterevkov
2020-03-11 19:31 ` David Rientjes
2020-03-12 12:48   ` Ivan Teterevkov
2020-03-12 13:36     ` Matthew Wilcox
2020-03-12 14:03       ` Chris Down
2020-03-13 10:49         ` Ivan Teterevkov
2020-03-13 21:50           ` David Rientjes
2020-03-16 16:03             ` Ivan Teterevkov
2020-03-12  9:25 ` Michal Hocko
2020-03-12 12:54   ` Ivan Teterevkov
2020-03-12 13:26     ` Michal Hocko
2020-03-16 14:53       ` Vlastimil Babka [this message]
2020-03-16 16:14         ` Ivan Teterevkov
2020-03-17  8:29         ` Michal Hocko
2020-03-17 14:51           ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ea2e014-17ea-6d1e-a6cd-775fb6550cd2@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=ivan.teterevkov@nutanix.com \
    --cc=jgross@suse.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mchehab+samsung@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=oneukum@suse.com \
    --cc=pawan.kumar.gupta@linux.intel.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Doc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-doc/0 linux-doc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-doc linux-doc/ https://lore.kernel.org/linux-doc \
		linux-doc@vger.kernel.org
	public-inbox-index linux-doc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-doc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git