[PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[Waiman Long]

This patch series is to be applied on top of the patch
series "ipc: Clamp *mni to the real IPCMNI limit"
(https://lkml.org/lkml/2018/3/1/716).

The purpose of this patch series is to enable easy addition of new
auxillary sysctl parameters that can be read to display ranges of
other sysctl parameters they are associated with.

With this patch series applied, a developer can add a range-showing
auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
flag in the ctl_table entry that has a range to show and add one more
to the number of reserved range entries in the CTL_RESERVE_RANGES()
macro before the terminating null entry. The new auxillary sysctl
parameters will have the "_range" suffix added.

The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
should match the number of reserved entries in the CTL_RESERVE_RANGES()
macro. Otherwise, warning or error like below will be logged in the
kernel ring buffer.

  Warning: Too many reserved ctl_table range entries ("shmmax")!

  Error: Insufficient reserved ctl_table range entries ("shmmax")!

The IPC sysctl parameters msgmni and shmmni are extended to have
those auxillary sysctl entries. As a result, one can now find out
the range of allowable value by looking at the corresponding
auxillary _range sysctl parameters. For example,

  % cat msgmni_range
  [0, 32768]

That means the msgmni sysctl parameter has to be within the 0 - 32768
range inclusively.

Patch 1 clarifies how the ctl_table.flags field should be used.

Patch 2 adds a new proc_show_minmax() handler to display a min/max
range.

Patch 3 defines the new CTL_FLAGS_SHOW_RANGE flag and the
CTL_RESERVE_RANGES() macro.

Patch 4 modifies the proc_sysctl.c file to handle the
CTL_FLAGS_SHOW_RANGE in ctl_table registration and un-registration.

Patch 5 extends msgmni and shmmni to have their corresponding range
showing sysctl parameters.

Waiman Long (5):
  sysctl: Clarify how the ctl_table.flags should be set
  sysctl: Add a new handler proc_show_minmax()
  sysctl: Add a new ctl_table flag to show min/max range
  proc/sysctl: Handle CTL_FLAGS_SHOW_RANGE ctl_table flag
  ipc: Show ranges of msgmni and shmmni with CTL_FLAGS_SHOW_RANGE

 fs/proc/proc_sysctl.c  | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/sysctl.h | 52 +++++++++++++++++++++++++-
 ipc/ipc_sysctl.c       |  5 ++-
 kernel/sysctl.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 238 insertions(+), 9 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] sysctl: Clarify how the ctl_table.flags should be set

[Waiman Long]

Additional comments are added to clarify how the ctl_table.flags
field should be set as well as precaution on what needs to be done
when additional flags are defined in the future.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/sysctl.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 3db57af..bc09361 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -125,7 +125,7 @@ struct ctl_table
 } __randomize_layout;
 
 /**
- * enum ctl_table_flags - flags for the ctl table
+ * enum ctl_table_flags - flags for the ctl table (struct ctl_table.flags)
  *
  * @CTL_FLAGS_CLAMP_RANGE: Set to indicate that the entry should be
  *	flexibly clamped to min/max range in case the user provided
@@ -134,10 +134,23 @@ struct ctl_table
  * 	had been issued for that entry.
  *
  * At most 16 different flags will be allowed.
+ *
+ * The flags can be set in two different ways. They can either be
+ * statically set at definition time or dynamically set at run time.
+ *
+ * Currently, only the CTL_FLAGS_OOR_WARNED flag is set at run time.
+ * No locking or atomic access to set @CTL_FLAGS_OOR_WARNED is needed
+ * in the current use case. If similar dynamic flags are defined in the
+ * future, we may need to revisit again to see if atomic access or
+ * locking is needed to prevent lost changes due to concurrent updates
+ * to the flags field.
  */
 enum ctl_table_flags {
+	/* Statically set flags */
 	CTL_FLAGS_CLAMP_RANGE		= BIT(0),
-	CTL_FLAGS_OOR_WARNED		= BIT(1),
+
+	/* Flags set at run time */
+	CTL_FLAGS_OOR_WARNED		= BIT(8),
 };
 
 struct ctl_node {
-- 
1.8.3.1

[PATCH 2/5] sysctl: Add a new handler proc_show_minmax()

[Waiman Long]

The new handler will show the minimum and maximum values specified
in the extra1 and extra2 fields of the sysctl table entry.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/sysctl.h |  2 ++
 kernel/sysctl.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index bc09361..decb71e 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -63,6 +63,8 @@ extern int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int,
 				      void __user *, size_t *, loff_t *);
 extern int proc_do_large_bitmap(struct ctl_table *, int,
 				void __user *, size_t *, loff_t *);
+extern int proc_show_minmax(struct ctl_table *table, int write,
+			    void __user *buffer, size_t *lenp, loff_t *ppos);
 
 /*
  * Register a set of sysctl names by calling register_sysctl_table
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6c68e77..486126f 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2700,6 +2700,97 @@ int proc_douintvec_minmax(struct ctl_table *table, int write,
 				 do_proc_douintvec_minmax_conv, &param);
 }
 
+/**
+ * proc_show_minmax - show the min/max values
+ * @table: the sysctl table
+ * @write: should always be %FALSE
+ * @buffer: the user buffer
+ * @lenp: the size of the user buffer
+ * @ppos: file position
+ *
+ * The table->data points to the original ctl_table which is used
+ * to determine the type of the min/max range as defined in the
+ * corresponding extra1 (min) and extra2 (max) field. The range
+ * will be displayed as "[min, max]".
+ *
+ * Returns 0 on success.
+ */
+int proc_show_minmax(struct ctl_table *table, int write,
+			  void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table *ctbl = (struct ctl_table *)table->data;
+	unsigned long min, max;
+	bool min_neg, max_neg;
+	int err = 0;
+	size_t left;
+
+	if (write || !ctbl || ((ctbl->maxlen != sizeof(int)) &&
+			       (ctbl->maxlen != sizeof(long))))
+		return -EINVAL;
+
+	if (*ppos) {
+		/* Not the first time */
+		*lenp = 0;
+		return 0;
+	}
+
+	min_neg = max_neg = false;
+	if (ctbl->proc_handler == proc_douintvec_minmax) {
+		min = ctbl->extra1 ? *(unsigned int *)ctbl->extra1 : 0;
+		max = ctbl->extra2 ? *(unsigned int *)ctbl->extra2 : UINT_MAX;
+	} else if (ctbl->maxlen == sizeof(int)) {
+		/* Assume to be signed integer */
+		unsigned int val;
+
+		val = ctbl->extra1 ? *(int *)ctbl->extra1 : -INT_MAX;
+		if (val < 0) {
+			min = -val;
+			min_neg = true;
+		} else {
+			min = val;
+			min_neg = false;
+		}
+		val = ctbl->extra2 ? *(int *)ctbl->extra2 : INT_MAX;
+		if (val < 0) {
+			max = -val;
+			max_neg = true;
+		} else {
+			max = val;
+			max_neg = false;
+		}
+	} else {
+		/* Assume to be unsigned long */
+		min = ctbl->extra1 ? *(unsigned long *)ctbl->extra1 : 0;
+		max = ctbl->extra2 ? *(unsigned long *)ctbl->extra2 : ULONG_MAX;
+	}
+
+	left = *lenp;
+
+	/*
+	 * Error checks are done only for proc_put_long() and the
+	 * last proc_put_char().
+	 */
+	proc_put_char(&buffer, &left, '[');
+	err = proc_put_long(&buffer, &left, min, min_neg);
+	if (err)
+		goto out;
+
+	proc_put_char(&buffer, &left, ',');
+	proc_put_char(&buffer, &left, ' ');
+	err = proc_put_long(&buffer, &left, max, max_neg);
+	if (err)
+		goto out;
+
+	proc_put_char(&buffer, &left, ']');
+	err = proc_put_char(&buffer, &left, '\n');
+
+out:
+	*lenp -= left;
+	*ppos += *lenp;
+
+	return err;
+}
+
 static int do_proc_dopipe_max_size_conv(unsigned long *lvalp,
 					unsigned int *valp,
 					int write, void *data)
-- 
1.8.3.1

[PATCH 3/5] sysctl: Add a new ctl_table flag to show min/max range

[Waiman Long]

A new ctl_table flag (CTL_FLAGS_SHOW_RANGE) is added which can be
used in the ctl_table definition for adding an auxillary read-only
sysctl parameter with the "_range" suffix to display the allowable
min/max range of that particular sysctl parameter.

This new flag has to be used with the new CTL_RESERVE_RANGES macro at
the end of the table definition to reserve table entries for showing
ranges. The number of table entries reserved must match the number
of table entries with the CTL_FLAGS_SHOW_RANGE flag set.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/sysctl.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index decb71e..ca64d66 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -132,6 +132,9 @@ struct ctl_table
  * @CTL_FLAGS_CLAMP_RANGE: Set to indicate that the entry should be
  *	flexibly clamped to min/max range in case the user provided
  *	an incorrect value.
+ * @CTL_FLAGS_SHOW_RANGE: Set to indicate that an auxillary read-only
+ *	sysctl parameter with the "_range" suffix should be created to
+ *	report the allowable range of that parameter.
  * @CTL_FLAGS_OOR_WARNED: Set to indicate that an out of range warning
  * 	had been issued for that entry.
  *
@@ -150,11 +153,40 @@ struct ctl_table
 enum ctl_table_flags {
 	/* Statically set flags */
 	CTL_FLAGS_CLAMP_RANGE		= BIT(0),
+	CTL_FLAGS_SHOW_RANGE		= BIT(1),
 
 	/* Flags set at run time */
 	CTL_FLAGS_OOR_WARNED		= BIT(8),
 };
 
+/**
+ * CTL_RESERVE_RANGES - reserve ctl_table entries for showing ranges
+ * @n - number of ctl_table entries to be reserved
+ *
+ * This CTL_RESERVE_RANGES() macro is used to reserve table entries
+ * when the CTL_FLAGS_SHOW_RANGE flag is used in the ctl_table. It has
+ * to be placed just before the ending null entry of the ctl_table
+ * definition. The number of reserved entries should match the number of
+ * ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag set.
+ * Otherwise, error will be returned if there is insufficient reserved
+ * entries in the registration process. A warning will be issued if
+ * there are more reserved entries than are really necessary.
+ *
+ * This macro currently supports up to 10 reserved ctl_table range entries.
+ */
+#define CTL_RESERVE_RANGES(n)	__CTL_RANGES_ ## n
+#define __CTL_RANGES_0
+#define __CTL_RANGES_1	{ .proc_handler = proc_show_minmax, },
+#define __CTL_RANGES_2	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_1
+#define __CTL_RANGES_3	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_2
+#define __CTL_RANGES_4	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_3
+#define __CTL_RANGES_5	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_4
+#define __CTL_RANGES_6	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_5
+#define __CTL_RANGES_7	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_6
+#define __CTL_RANGES_8	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_7
+#define __CTL_RANGES_9	{ .proc_handler = proc_show_minmax, }, __CTL_RANGES_8
+#define __CTL_RANGES_10 { .proc_handler = proc_show_minmax, }, __CTL_RANGES_9
+
 struct ctl_node {
 	struct rb_node node;
 	struct ctl_table_header *header;
-- 
1.8.3.1

[PATCH 4/5] proc/sysctl: Handle CTL_FLAGS_SHOW_RANGE ctl_table flag

[Waiman Long]

When registering ctl_table entries with CTL_FLAGS_SHOW_RANGE flag set,
it will populate the reserved range table entries with the proper
sysctl parameters to show the range the those ctl_table entries.

When unregistering the ctl_table, we also need to clear the reserved
range table entries to avoid referencing memory that will be freed.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 fs/proc/proc_sysctl.c  | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/sysctl.h |  1 +
 2 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 493c975..5f8fde97 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -304,6 +304,16 @@ static void proc_sys_prune_dcache(struct ctl_table_header *head)
 static void start_unregistering(struct ctl_table_header *p)
 {
 	/*
+	 * Clear reserved range table entries before freeing.
+	 */
+	if (p->roffset) {
+		struct ctl_table *entry = p->ctl_table + p->roffset;
+
+		for (; entry->proc_handler == proc_show_minmax; entry++)
+			entry->procname = NULL;
+	}
+
+	/*
 	 * if p->used is 0, nobody will ever touch that entry again;
 	 * we'll eliminate all paths to it before dropping sysctl_lock
 	 */
@@ -1206,7 +1216,6 @@ static int insert_links(struct ctl_table_header *head)
 
 	if (head->set == root_set)
 		return 0;
-
 	core_parent = xlate_dir(root_set, head->parent);
 	if (IS_ERR(core_parent))
 		return 0;
@@ -1238,6 +1247,19 @@ static int insert_links(struct ctl_table_header *head)
 	return err;
 }
 
+static bool sysctl_show_range(struct ctl_table *entry)
+{
+	if (!(entry->flags & CTL_FLAGS_SHOW_RANGE))
+		return false;
+
+	if ((entry->maxlen == sizeof(int)) || (entry->maxlen == sizeof(long)))
+		return true;
+
+	pr_warn("Warning: ctl_table entry \"%s\" doesn't support CTL_FLAGS_SHOW_RANGE\n",
+		entry->procname);
+	return false;
+}
+
 /**
  * __register_sysctl_table - register a leaf sysctl table
  * @set: Sysctl tree to register on
@@ -1291,16 +1313,67 @@ struct ctl_table_header *__register_sysctl_table(
 	struct ctl_table *entry;
 	struct ctl_node *node;
 	int nr_entries = 0;
+	int nr_ranges = 0;	/* # of entries with CTL_FLAGS_SHOW_RANGE */
+	int namelen = 0;
+	int i, j;
 
-	for (entry = table; entry->procname; entry++)
+	for (entry = table; entry->procname; entry++) {
 		nr_entries++;
+		if (sysctl_show_range(entry)) {
+			nr_ranges++;
+			/* procname + "_range\0" suffix */
+			namelen += strlen(entry->procname) + 7;
+		}
+	}
+
+	if (nr_ranges) {
+		for (i = 0; entry->proc_handler == proc_show_minmax; entry++)
+			i++;
+
+		if (i < nr_ranges) {
+			pr_err("Error: Insufficient reserved ctl_table range entries (\"%s\")!\n",
+				table->procname);
+			return NULL;
+		} else if (i > nr_ranges) {
+			pr_warn("Warning: Too many reserved ctl_table range entries (\"%s\")!\n",
+				table->procname);
+		}
+	}
 
 	header = kzalloc(sizeof(struct ctl_table_header) +
-			 sizeof(struct ctl_node)*nr_entries, GFP_KERNEL);
+			 sizeof(struct ctl_node)*(nr_entries + nr_ranges) +
+			 namelen, GFP_KERNEL);
 	if (!header)
 		return NULL;
-
 	node = (struct ctl_node *)(header + 1);
+
+	/*
+	 * Fill up reserved range entries for showing the ranges of those
+	 * sysctl parameters that have the CTL_FLAGS_SHOW_RANGE flag set.
+	 */
+	if (nr_ranges) {
+		int len;
+		char *namebuf = (char *)(node + nr_entries + nr_ranges);
+
+		header->roffset = nr_entries;
+		for (i = 0, j = nr_entries ; i < nr_entries; i++) {
+			if (!sysctl_show_range(&table[i]))
+				continue;
+
+			len = strlen(table[i].procname);
+			memcpy(namebuf, table[i].procname, len);
+			memcpy(namebuf + len, "_range\0", 7);
+			table[j].procname = namebuf;
+			table[j].data = (void *)&table[i];
+			table[j].mode = table[i].mode & 0444;	/* Read only */
+
+			namebuf += len + 7;
+			namelen -= len + 7;
+			j++;
+		}
+		WARN_ON((j != nr_entries + nr_ranges) || namelen);
+	}
+
 	init_header(header, root, set, node, table);
 	if (sysctl_check_table(path, table))
 		goto fail;
@@ -1313,7 +1386,6 @@ struct ctl_table_header *__register_sysctl_table(
 
 	/* Find the directory for the ctl_table */
 	for (name = path; name; name = nextname) {
-		int namelen;
 		nextname = strchr(name, '/');
 		if (nextname) {
 			namelen = nextname - name;
@@ -1342,6 +1414,13 @@ struct ctl_table_header *__register_sysctl_table(
 	drop_sysctl_table(&dir->header);
 	spin_unlock(&sysctl_lock);
 fail:
+	if (nr_ranges) {
+		/*
+		 * Clear procname of the reserved range table entries.
+		 */
+		for (i = nr_entries; i < nr_entries + nr_ranges; i++)
+			table[i].procname = NULL;
+	}
 	kfree(header);
 	dump_stack();
 	return NULL;
@@ -1654,6 +1733,16 @@ void unregister_sysctl_table(struct ctl_table_header * header)
 			unregister_sysctl_table(subh);
 			kfree(table);
 		}
+		/*
+		 * Clear reserved range table entries before freeing.
+		 */
+		if (header->roffset) {
+			struct ctl_table *entry = header->ctl_table +
+						  header->roffset;
+
+			for (; entry->proc_handler == proc_show_minmax; entry++)
+				entry->procname = NULL;
+		}
 		kfree(header);
 		return;
 	}
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index ca64d66..e922ee3 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -202,6 +202,7 @@ struct ctl_table_header
 			int used;
 			int count;
 			int nreg;
+			int roffset;	/* Offset of reserved range entries */
 		};
 		struct rcu_head rcu;
 	};
-- 
1.8.3.1

[PATCH 5/5] ipc: Show ranges of msgmni and shmmni with CTL_FLAGS_SHOW_RANGE

[Waiman Long]

The CTL_FLAGS_SHOW_RANGE flag is added to the msgmni and shmmni
ctl_table entries to show their ranges. Two range table entries are
reserved for the new *_range sysctl parameters.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 ipc/ipc_sysctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 2c03f57..3b4d7cd 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -143,7 +143,7 @@ static int proc_ipc_sem_dointvec(struct ctl_table *table, int write,
 		.proc_handler	= proc_ipc_dointvec_minmax,
 		.extra1		= &zero,
 		.extra2		= &ipc_mni,
-		.flags		= CTL_FLAGS_CLAMP_RANGE,
+		.flags		= CTL_FLAGS_CLAMP_RANGE|CTL_FLAGS_SHOW_RANGE,
 	},
 	{
 		.procname	= "shm_rmid_forced",
@@ -171,7 +171,7 @@ static int proc_ipc_sem_dointvec(struct ctl_table *table, int write,
 		.proc_handler	= proc_ipc_dointvec_minmax,
 		.extra1		= &zero,
 		.extra2		= &ipc_mni,
-		.flags		= CTL_FLAGS_CLAMP_RANGE,
+		.flags		= CTL_FLAGS_CLAMP_RANGE|CTL_FLAGS_SHOW_RANGE,
 	},
 	{
 		.procname	= "auto_msgmni",
@@ -228,6 +228,7 @@ static int proc_ipc_sem_dointvec(struct ctl_table *table, int write,
 		.extra2		= &int_max,
 	},
 #endif
+	CTL_RESERVE_RANGES(2)
 	{}
 };
 
-- 
1.8.3.1

Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[Andrew Morton]

On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long <longman@redhat.com> wrote:

> This patch series is to be applied on top of the patch
> series "ipc: Clamp *mni to the real IPCMNI limit"
> (https://lkml.org/lkml/2018/3/1/716).
> 
> The purpose of this patch series is to enable easy addition of new
> auxillary sysctl parameters that can be read to display ranges of
> other sysctl parameters they are associated with.
> 
> With this patch series applied, a developer can add a range-showing
> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
> flag in the ctl_table entry that has a range to show and add one more
> to the number of reserved range entries in the CTL_RESERVE_RANGES()
> macro before the terminating null entry. The new auxillary sysctl
> parameters will have the "_range" suffix added.
> 
> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
> should match the number of reserved entries in the CTL_RESERVE_RANGES()
> macro. Otherwise, warning or error like below will be logged in the
> kernel ring buffer.
> 
>   Warning: Too many reserved ctl_table range entries ("shmmax")!
> 
>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
> 
> The IPC sysctl parameters msgmni and shmmni are extended to have
> those auxillary sysctl entries. As a result, one can now find out
> the range of allowable value by looking at the corresponding
> auxillary _range sysctl parameters. For example,
> 
>   % cat msgmni_range
>   [0, 32768]
> 
> That means the msgmni sysctl parameter has to be within the 0 - 32768
> range inclusively.
> 
> ...
> 
>  fs/proc/proc_sysctl.c  | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
>  include/linux/sysctl.h | 52 +++++++++++++++++++++++++-
>  ipc/ipc_sysctl.c       |  5 ++-
>  kernel/sysctl.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 238 insertions(+), 9 deletions(-)

That quite a bunch of new code and it's not clear to me that we get a
lot of value from it all.  Perhaps I'm missing the point.

A worked example would help, along the lines of:


a) Here's how we do X at present and here's the output

b) Here's how we X after these patches and here's the new output

c) b) is better than a) for <reasons>

Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[Luis R. Rodriguez]

On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
> 
>   % cat msgmni_range
>   [0, 32768]

All that sounds promising but I think you are jumping a few
steps ahead of what needs to get done. Let's first sort out the
first series well.

  Luis

Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[Waiman Long]

On 03/08/2018 01:34 PM, Luis R. Rodriguez wrote:
> On Wed, Mar 07, 2018 at 03:34:24PM -0500, Waiman Long wrote:
>>   % cat msgmni_range
>>   [0, 32768]
> All that sounds promising but I think you are jumping a few
> steps ahead of what needs to get done. Let's first sort out the
> first series well.
>
>   Luis

Yes, you are right. I will focus on the getting the first series right
first, before considering that.

In fact, I have a slight change of mind on how to do it. Instead of
adding a new _range parameter to show just the range of a particular
sysctl parameter, I am thinking of a generic sysctl_ranges parameter
that iterates all the ctl_table entries and list out the ranges of all
the parameters that are flagged to have a min/max range. That will
eliminate the interface clutter due to a bunch of new *_range /proc/sys
files.

-Longman

Re: [PATCH 0/5] sysctl: Enable easy addition of range showing sysctl parameters

[Waiman Long]

On 03/07/2018 05:48 PM, Andrew Morton wrote:
> On Wed,  7 Mar 2018 15:34:24 -0500 Waiman Long <longman@redhat.com> wrote:
>
>> This patch series is to be applied on top of the patch
>> series "ipc: Clamp *mni to the real IPCMNI limit"
>> (https://lkml.org/lkml/2018/3/1/716).
>>
>> The purpose of this patch series is to enable easy addition of new
>> auxillary sysctl parameters that can be read to display ranges of
>> other sysctl parameters they are associated with.
>>
>> With this patch series applied, a developer can add a range-showing
>> auxillary sysctl entries by just setting the CTL_FLAGS_SHOW_RANGE
>> flag in the ctl_table entry that has a range to show and add one more
>> to the number of reserved range entries in the CTL_RESERVE_RANGES()
>> macro before the terminating null entry. The new auxillary sysctl
>> parameters will have the "_range" suffix added.
>>
>> The number of ctl_table entries that have the CTL_FLAGS_SHOW_RANGE flag
>> should match the number of reserved entries in the CTL_RESERVE_RANGES()
>> macro. Otherwise, warning or error like below will be logged in the
>> kernel ring buffer.
>>
>>   Warning: Too many reserved ctl_table range entries ("shmmax")!
>>
>>   Error: Insufficient reserved ctl_table range entries ("shmmax")!
>>
>> The IPC sysctl parameters msgmni and shmmni are extended to have
>> those auxillary sysctl entries. As a result, one can now find out
>> the range of allowable value by looking at the corresponding
>> auxillary _range sysctl parameters. For example,
>>
>>   % cat msgmni_range
>>   [0, 32768]
>>
>> That means the msgmni sysctl parameter has to be within the 0 - 32768
>> range inclusively.
>>
>> ...
>>
>>  fs/proc/proc_sysctl.c  | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
>>  include/linux/sysctl.h | 52 +++++++++++++++++++++++++-
>>  ipc/ipc_sysctl.c       |  5 ++-
>>  kernel/sysctl.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 238 insertions(+), 9 deletions(-)
> That quite a bunch of new code and it's not clear to me that we get a
> lot of value from it all.  Perhaps I'm missing the point.
>
> A worked example would help, along the lines of:
>
>
> a) Here's how we do X at present and here's the output
>
> b) Here's how we X after these patches and here's the new output
>
> c) b) is better than a) for <reasons>

We do have a big customer asking for ways to find out what are the
actual ranges of sysctl parameters that they are using other than
digging into the source code to figure it out. This patch series and
previous one are part of the effort to fulfill this customer need.

Please ignore this patch series. I will focus on getting the first one
right and upstream first before working on a followup.

Thanks,
Longman