[PATCH v4 0/8] sysctl: Completely remove register_sysctl_table from sources

[PATCH v4 0/8] sysctl: Completely remove register_sysctl_table from sources

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. It contains 2 patchsets that were originally sent
separately. I have put them together because the second followed the first.

Parport driver uses the "CHILD" pointer in the ctl_table structure to create
its directory structure. We move to the newer register_sysctl call and remove
the pointer madness. I have separated the parport into 5 patches to clarify the
different changes needed for the 3 calls to register_sysctl_paths.

We no longer export the register_sysctl_table call as parport was the
last user from outside proc_sysctl.c. Also modified documentation slightly
so register_sysctl_table is no longer mentioned.

Replace register_sysctl_table with register_sysctl effectively effectively
transitioning 5 base paths ("kernel", "vm", "fs", "dev" and "debug") to the new
call. Besides removing the actual function, I also removed it from the checks
done in check-sysctl-docs. @mcgrof went a bit further and removed 2 more
functions.

Testing for this change was done in the same way as with previous sysctl
replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
was the same before and after the patchset.

V4:
* (mcgrof) : use of register_sysctl_init instead of register_sysctl
* (mcgrof) : removed register_sysctl_table and __register_sysctl_base
* Added a unregister call to properly unwind things when there is an error
* Added kernel proc subdirectories "kernel/usermodehelper" and "kernel/keys"

V3:
* Added a return error value when register fails
* Made sure to free the memory on error when calling parport_proc_register
* Added a bloat-o-meter output to measure bloat
* Replaced kmalloc with kzalloc
* Added comments about testing
* Improved readability when using snprintf

Have pushed this through 0-day. Waiting on results..

Best
Joel

Joel Granados (8):
  parport: Move magic number "15" to a define
  parport: Remove register_sysctl_table from parport_proc_register
  parport: Remove register_sysctl_table from
    parport_device_proc_register
  parport: Remove register_sysctl_table from
    parport_default_proc_register
  parport: Removed sysctl related defines
  sysctl: stop exporting register_sysctl_table
  sysctl: Refactor base paths registrations
  sysctl: Remove register_sysctl_table

 drivers/parport/procfs.c  | 174 ++++++++++++++++++++------------------
 drivers/parport/share.c   |   2 +-
 fs/proc/proc_sysctl.c     | 162 +----------------------------------
 fs/sysctls.c              |   5 +-
 include/linux/parport.h   |   2 +
 include/linux/sysctl.h    |  31 +------
 kernel/sysctl.c           |  30 ++-----
 scripts/check-sysctl-docs |  10 ---
 8 files changed, 110 insertions(+), 306 deletions(-)

-- 
2.30.2

[PATCH v4 1/8] parport: Move magic number "15" to a define

[Joel Granados]

Put the size of a parport name behind a define so we can use it in other
files. This is a preparation patch to be able to use this size in
parport/procfs.c.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/parport/share.c | 2 +-
 include/linux/parport.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 62f8407923d4..2d46b1d4fd69 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -467,7 +467,7 @@ struct parport *parport_register_port(unsigned long base, int irq, int dma,
 	atomic_set(&tmp->ref_count, 1);
 	INIT_LIST_HEAD(&tmp->full_list);
 
-	name = kmalloc(15, GFP_KERNEL);
+	name = kmalloc(PARPORT_NAME_MAX_LEN, GFP_KERNEL);
 	if (!name) {
 		kfree(tmp);
 		return NULL;
diff --git a/include/linux/parport.h b/include/linux/parport.h
index a0bc9e0267b7..243c82d7f852 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -180,6 +180,8 @@ struct ieee1284_info {
 	struct semaphore irq;
 };
 
+#define PARPORT_NAME_MAX_LEN 15
+
 /* A parallel port */
 struct parport {
 	unsigned long base;	/* base address */
-- 
2.30.2

[PATCH v4 2/8] parport: Remove register_sysctl_table from parport_proc_register

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. Register dev/parport/PORTNAME and
dev/parport/PORTNAME/devices. Temporary allocation for name is freed at
the end of the function. Remove all the struct elements that are no
longer used in the parport_device_sysctl_template struct. Add parport
specific defines that hide the base path sizes.

To make sure the resulting directory structure did not change we
made sure that `find /proc/sys/dev/ | sha1sum` was the same before and
after the change.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/parport/procfs.c | 93 ++++++++++++++++++++++++++--------------
 1 file changed, 61 insertions(+), 32 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index d740eba3c099..28a37e0ef98c 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -32,6 +32,13 @@
 #define PARPORT_MAX_TIMESLICE_VALUE ((unsigned long) HZ)
 #define PARPORT_MIN_SPINTIME_VALUE 1
 #define PARPORT_MAX_SPINTIME_VALUE 1000
+/*
+ * PARPORT_BASE_* is the size of the known parts of the sysctl path
+ * in dev/partport/%s/devices/%s. "dev/parport/"(12), "/devices/"(9
+ * and null char(1).
+ */
+#define PARPORT_BASE_PATH_SIZE 13
+#define PARPORT_BASE_DEVICES_PATH_SIZE 22
 
 static int do_active_device(struct ctl_table *table, int write,
 		      void *result, size_t *lenp, loff_t *ppos)
@@ -260,9 +267,6 @@ struct parport_sysctl_table {
 	struct ctl_table_header *sysctl_header;
 	struct ctl_table vars[12];
 	struct ctl_table device_dir[2];
-	struct ctl_table port_dir[2];
-	struct ctl_table parport_dir[2];
-	struct ctl_table dev_dir[2];
 };
 
 static const struct parport_sysctl_table parport_sysctl_template = {
@@ -305,7 +309,6 @@ static const struct parport_sysctl_table parport_sysctl_template = {
 			.mode		= 0444,
 			.proc_handler	= do_hardware_modes
 		},
-		PARPORT_DEVICES_ROOT_DIR,
 #ifdef CONFIG_PARPORT_1284
 		{
 			.procname	= "autoprobe",
@@ -355,18 +358,6 @@ static const struct parport_sysctl_table parport_sysctl_template = {
 		},
 		{}
 	},
-	{
-		PARPORT_PORT_DIR(NULL),
-		{}
-	},
-	{
-		PARPORT_PARPORT_DIR(NULL),
-		{}
-	},
-	{
-		PARPORT_DEV_DIR(NULL),
-		{}
-	}
 };
 
 struct parport_device_sysctl_table
@@ -473,11 +464,13 @@ parport_default_sysctl_table = {
 	}
 };
 
-
 int parport_proc_register(struct parport *port)
 {
 	struct parport_sysctl_table *t;
-	int i;
+	struct ctl_table_header *devices_h;
+	char *tmp_dir_path;
+	size_t tmp_path_len, port_name_len;
+	int bytes_written, i, err = 0;
 
 	t = kmemdup(&parport_sysctl_template, sizeof(*t), GFP_KERNEL);
 	if (t == NULL)
@@ -485,28 +478,64 @@ int parport_proc_register(struct parport *port)
 
 	t->device_dir[0].extra1 = port;
 
-	for (i = 0; i < 5; i++)
-		t->vars[i].extra1 = port;
-
 	t->vars[0].data = &port->spintime;
-	t->vars[5].child = t->device_dir;
-	
-	for (i = 0; i < 5; i++)
-		t->vars[6 + i].extra2 = &port->probe_info[i];
+	for (i = 0; i < 5; i++) {
+		t->vars[i].extra1 = port;
+		t->vars[5 + i].extra2 = &port->probe_info[i];
+	}
 
-	t->port_dir[0].procname = port->name;
+	port_name_len = strnlen(port->name, PARPORT_NAME_MAX_LEN);
+	/*
+	 * Allocate a buffer for two paths: dev/parport/PORT and dev/parport/PORT/devices.
+	 * We calculate for the second as that will give us enough for the first.
+	 */
+	tmp_path_len = PARPORT_BASE_DEVICES_PATH_SIZE + port_name_len;
+	tmp_dir_path = kzalloc(tmp_path_len, GFP_KERNEL);
+	if (!tmp_dir_path) {
+		err = -ENOMEM;
+		goto exit_free_t;
+	}
 
-	t->port_dir[0].child = t->vars;
-	t->parport_dir[0].child = t->port_dir;
-	t->dev_dir[0].child = t->parport_dir;
+	bytes_written = snprintf(tmp_dir_path, tmp_path_len,
+				 "dev/parport/%s/devices", port->name);
+	if (tmp_path_len <= bytes_written) {
+		err = -ENOENT;
+		goto exit_free_tmp_dir_path;
+	}
+	devices_h = register_sysctl(tmp_dir_path, t->device_dir);
+	if (devices_h == NULL) {
+		err = -ENOENT;
+		goto  exit_free_tmp_dir_path;
+	}
 
-	t->sysctl_header = register_sysctl_table(t->dev_dir);
+	tmp_path_len = PARPORT_BASE_PATH_SIZE + port_name_len;
+	bytes_written = snprintf(tmp_dir_path, tmp_path_len,
+				 "dev/parport/%s", port->name);
+	if (tmp_path_len <= bytes_written) {
+		err = -ENOENT;
+		goto unregister_devices_h;
+	}
+
+	t->sysctl_header = register_sysctl(tmp_dir_path, t->vars);
 	if (t->sysctl_header == NULL) {
-		kfree(t);
-		t = NULL;
+		err = -ENOENT;
+		goto unregister_devices_h;
 	}
+
 	port->sysctl_table = t;
+
+	kfree(tmp_dir_path);
 	return 0;
+
+unregister_devices_h:
+	unregister_sysctl_table(devices_h);
+
+exit_free_tmp_dir_path:
+	kfree(tmp_dir_path);
+
+exit_free_t:
+	kfree(t);
+	return err;
 }
 
 int parport_proc_unregister(struct parport *port)
-- 
2.30.2

[PATCH v4 3/8] parport: Remove register_sysctl_table from parport_device_proc_register

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. We use a temp allocation to include both port and
device name in proc. Allocated mem is freed at the end. The unused
parport_device_sysctl_template struct elements that are not used are
removed.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202305150948.pHgIh7Ql-lkp@intel.com/
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/parport/procfs.c | 56 +++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 28a37e0ef98c..22d211c95168 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -384,6 +384,7 @@ parport_device_sysctl_template = {
 			.extra1		= (void*) &parport_min_timeslice_value,
 			.extra2		= (void*) &parport_max_timeslice_value
 		},
+		{}
 	},
 	{
 		{
@@ -394,22 +395,6 @@ parport_device_sysctl_template = {
 			.child		= NULL
 		},
 		{}
-	},
-	{
-		PARPORT_DEVICES_ROOT_DIR,
-		{}
-	},
-	{
-		PARPORT_PORT_DIR(NULL),
-		{}
-	},
-	{
-		PARPORT_PARPORT_DIR(NULL),
-		{}
-	},
-	{
-		PARPORT_DEV_DIR(NULL),
-		{}
 	}
 };
 
@@ -551,30 +536,53 @@ int parport_proc_unregister(struct parport *port)
 
 int parport_device_proc_register(struct pardevice *device)
 {
+	int bytes_written, err = 0;
 	struct parport_device_sysctl_table *t;
 	struct parport * port = device->port;
+	size_t port_name_len, device_name_len, tmp_dir_path_len;
+	char *tmp_dir_path;
 	
 	t = kmemdup(&parport_device_sysctl_template, sizeof(*t), GFP_KERNEL);
 	if (t == NULL)
 		return -ENOMEM;
 
-	t->dev_dir[0].child = t->parport_dir;
-	t->parport_dir[0].child = t->port_dir;
-	t->port_dir[0].procname = port->name;
-	t->port_dir[0].child = t->devices_root_dir;
-	t->devices_root_dir[0].child = t->device_dir;
+	port_name_len = strnlen(port->name, PARPORT_NAME_MAX_LEN);
+	device_name_len = strnlen(device->name, PATH_MAX);
+
+	/* Allocate a buffer for two paths: dev/parport/PORT/devices/DEVICE. */
+	tmp_dir_path_len = PARPORT_BASE_DEVICES_PATH_SIZE + port_name_len + device_name_len;
+	tmp_dir_path = kzalloc(tmp_dir_path_len, GFP_KERNEL);
+	if (!tmp_dir_path) {
+		err = -ENOMEM;
+		goto exit_free_t;
+	}
+
+	bytes_written = snprintf(tmp_dir_path, tmp_dir_path_len, "dev/parport/%s/devices/%s",
+				 port->name, device->name);
+	if (tmp_dir_path_len <= bytes_written) {
+		err = -ENOENT;
+		goto exit_free_path;
+	}
 
-	t->device_dir[0].procname = device->name;
-	t->device_dir[0].child = t->vars;
 	t->vars[0].data = &device->timeslice;
 
-	t->sysctl_header = register_sysctl_table(t->dev_dir);
+	t->sysctl_header = register_sysctl(tmp_dir_path, t->vars);
 	if (t->sysctl_header == NULL) {
 		kfree(t);
 		t = NULL;
 	}
 	device->sysctl_table = t;
+
+	kfree(tmp_dir_path);
 	return 0;
+
+exit_free_path:
+	kfree(tmp_dir_path);
+
+exit_free_t:
+	kfree(t);
+
+	return err;
 }
 
 int parport_device_proc_unregister(struct pardevice *device)
-- 
2.30.2

[PATCH v4 4/8] parport: Remove register_sysctl_table from parport_default_proc_register

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. Simply change the full path "dev/parport/default"
to point to an already existing set of table entries (vars). We also
remove the unused elements from parport_default_table.

To make sure the resulting directory structure did not change we
made sure that `find /proc/sys/dev/ | sha1sum` was the same before and
after the change.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/parport/procfs.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 22d211c95168..1a26918d2cc8 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -430,22 +430,6 @@ parport_default_sysctl_table = {
 			.extra2		= (void*) &parport_max_spintime_value
 		},
 		{}
-	},
-	{
-		{
-			.procname	= "default",
-			.mode		= 0555,
-			.child		= parport_default_sysctl_table.vars
-		},
-		{}
-	},
-	{
-		PARPORT_PARPORT_DIR(parport_default_sysctl_table.default_dir),
-		{}
-	},
-	{
-		PARPORT_DEV_DIR(parport_default_sysctl_table.parport_dir),
-		{}
 	}
 };
 
@@ -601,7 +585,7 @@ static int __init parport_default_proc_register(void)
 	int ret;
 
 	parport_default_sysctl_table.sysctl_header =
-		register_sysctl_table(parport_default_sysctl_table.dev_dir);
+		register_sysctl("dev/parport/default", parport_default_sysctl_table.vars);
 	if (!parport_default_sysctl_table.sysctl_header)
 		return -ENOMEM;
 	ret = parport_bus_init();
-- 
2.30.2

[PATCH v4 5/8] parport: Removed sysctl related defines

[Joel Granados]

The partport driver used to rely on defines to include different
directories in sysctl. Now that we have made the transition to
register_sysctl from regsiter_sysctl_table, they are no longer needed.

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/parport/procfs.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
index 1a26918d2cc8..cbb1fb5127ce 100644
--- a/drivers/parport/procfs.c
+++ b/drivers/parport/procfs.c
@@ -243,13 +243,6 @@ do {									\
 	return 0;
 }
 
-#define PARPORT_PORT_DIR(CHILD) { .procname = NULL, .mode = 0555, .child = CHILD }
-#define PARPORT_PARPORT_DIR(CHILD) { .procname = "parport", \
-                                     .mode = 0555, .child = CHILD }
-#define PARPORT_DEV_DIR(CHILD) { .procname = "dev", .mode = 0555, .child = CHILD }
-#define PARPORT_DEVICES_ROOT_DIR  {  .procname = "devices", \
-                                    .mode = 0555, .child = NULL }
-
 static const unsigned long parport_min_timeslice_value =
 PARPORT_MIN_TIMESLICE_VALUE;
 
-- 
2.30.2

[PATCH v4 6/8] sysctl: stop exporting register_sysctl_table

[Joel Granados]

We make register_sysctl_table static because the only function calling
it is in fs/proc/proc_sysctl.c (__register_sysctl_base). We remove it
from the sysctl.h header and modify the documentation in both the header
and proc_sysctl.c files to mention "register_sysctl" instead of
"register_sysctl_table".

This plus the commits that remove register_sysctl_table from parport
save 217 bytes:

./scripts/bloat-o-meter .bsysctl/vmlinux.old .bsysctl/vmlinux.new
add/remove: 0/1 grow/shrink: 5/1 up/down: 458/-675 (-217)
Function                                     old     new   delta
__register_sysctl_base                         8     286    +278
parport_proc_register                        268     379    +111
parport_device_proc_register                 195     247     +52
kzalloc.constprop                            598     608     +10
parport_default_proc_register                 62      69      +7
register_sysctl_table                        291       -    -291
parport_sysctl_template                     1288     904    -384
Total: Before=8603076, After=8602859, chg -0.00%

Signed-off-by: Joel Granados <j.granados@samsung.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 fs/proc/proc_sysctl.c  | 5 ++---
 include/linux/sysctl.h | 8 +-------
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 8038833ff5b0..f8f19e000d76 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1582,7 +1582,7 @@ static int register_leaf_sysctl_tables(const char *path, char *pos,
  * array. A completely 0 filled entry terminates the table.
  * We are slowly deprecating this call so avoid its use.
  */
-struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
+static struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
 {
 	struct ctl_table *ctl_table_arg = table;
 	int nr_subheaders = count_subheaders(table);
@@ -1634,7 +1634,6 @@ struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
 	header = NULL;
 	goto out;
 }
-EXPORT_SYMBOL(register_sysctl_table);
 
 int __register_sysctl_base(struct ctl_table *base_table)
 {
@@ -1700,7 +1699,7 @@ static void drop_sysctl_table(struct ctl_table_header *header)
 
 /**
  * unregister_sysctl_table - unregister a sysctl table hierarchy
- * @header: the header returned from register_sysctl_table
+ * @header: the header returned from register_sysctl or __register_sysctl_table
  *
  * Unregisters the sysctl table and all children. proc entries may not
  * actually be removed until they are no longer used by anyone.
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 3d08277959af..218e56a26fb0 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -89,7 +89,7 @@ int proc_do_static_key(struct ctl_table *table, int write, void *buffer,
 		size_t *lenp, loff_t *ppos);
 
 /*
- * Register a set of sysctl names by calling register_sysctl_table
+ * Register a set of sysctl names by calling register_sysctl
  * with an initialised array of struct ctl_table's.  An entry with 
  * NULL procname terminates the table.  table->de will be
  * set up by the registration and need not be initialised in advance.
@@ -222,7 +222,6 @@ struct ctl_table_header *__register_sysctl_table(
 	struct ctl_table_set *set,
 	const char *path, struct ctl_table *table);
 struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table);
-struct ctl_table_header *register_sysctl_table(struct ctl_table * table);
 void unregister_sysctl_table(struct ctl_table_header * table);
 
 extern int sysctl_init_bases(void);
@@ -257,11 +256,6 @@ static inline int __register_sysctl_base(struct ctl_table *base_table)
 
 #define register_sysctl_base(table) __register_sysctl_base(table)
 
-static inline struct ctl_table_header *register_sysctl_table(struct ctl_table * table)
-{
-	return NULL;
-}
-
 static inline void register_sysctl_init(const char *path, struct ctl_table *table)
 {
 }
-- 
2.30.2

[PATCH v4 7/8] sysctl: Refactor base paths registrations

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. The old way of doing this through
register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
call to register_sysctl_init. The 5 base paths affected are: "kernel",
"vm", "debug", "dev" and "fs".

We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
macro since they are no longer needed.

In order to quickly acertain that the paths did not actually change I
executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
same before and after the commit.

We end up saving 563 bytes with this change:

./scripts/bloat-o-meter vmlinux.0.base vmlinux.1.refactor-base-paths
add/remove: 0/5 grow/shrink: 2/0 up/down: 77/-640 (-563)
Function                                     old     new   delta
sysctl_init_bases                             55     111     +56
init_fs_sysctls                               12      33     +21
vm_base_table                                128       -    -128
kernel_base_table                            128       -    -128
fs_base_table                                128       -    -128
dev_base_table                               128       -    -128
debug_base_table                             128       -    -128
Total: Before=21258215, After=21257652, chg -0.00%

Signed-off-by: Joel Granados <j.granados@samsung.com>
[mcgrof: modified to use register_sysctl_init() over register_sysctl()
 and add bloat-o-meter stats]
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/sysctls.c           |  5 ++---
 include/linux/sysctl.h | 23 -----------------------
 kernel/sysctl.c        | 30 +++++++++---------------------
 3 files changed, 11 insertions(+), 47 deletions(-)

diff --git a/fs/sysctls.c b/fs/sysctls.c
index c701273c9432..76a0aee8c229 100644
--- a/fs/sysctls.c
+++ b/fs/sysctls.c
@@ -29,11 +29,10 @@ static struct ctl_table fs_shared_sysctls[] = {
 	{ }
 };
 
-DECLARE_SYSCTL_BASE(fs, fs_shared_sysctls);
-
 static int __init init_fs_sysctls(void)
 {
-	return register_sysctl_base(fs);
+	register_sysctl_init("fs", fs_shared_sysctls);
+	return 0;
 }
 
 early_initcall(init_fs_sysctls);
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 218e56a26fb0..653b66c762b1 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -197,20 +197,6 @@ struct ctl_path {
 
 #ifdef CONFIG_SYSCTL
 
-#define DECLARE_SYSCTL_BASE(_name, _table)				\
-static struct ctl_table _name##_base_table[] = {			\
-	{								\
-		.procname	= #_name,				\
-		.mode		= 0555,					\
-		.child		= _table,				\
-	},								\
-	{ },								\
-}
-
-extern int __register_sysctl_base(struct ctl_table *base_table);
-
-#define register_sysctl_base(_name) __register_sysctl_base(_name##_base_table)
-
 void proc_sys_poll_notify(struct ctl_table_poll *poll);
 
 extern void setup_sysctl_set(struct ctl_table_set *p,
@@ -247,15 +233,6 @@ extern struct ctl_table sysctl_mount_point[];
 
 #else /* CONFIG_SYSCTL */
 
-#define DECLARE_SYSCTL_BASE(_name, _table)
-
-static inline int __register_sysctl_base(struct ctl_table *base_table)
-{
-	return 0;
-}
-
-#define register_sysctl_base(table) __register_sysctl_base(table)
-
 static inline void register_sysctl_init(const char *path, struct ctl_table *table)
 {
 }
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index bfe53e835524..73fa9cf7ee11 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1782,11 +1782,6 @@ static struct ctl_table kern_table[] = {
 		.mode		= 0644,
 		.proc_handler	= sysctl_max_threads,
 	},
-	{
-		.procname	= "usermodehelper",
-		.mode		= 0555,
-		.child		= usermodehelper_table,
-	},
 	{
 		.procname	= "overflowuid",
 		.data		= &overflowuid,
@@ -1962,13 +1957,6 @@ static struct ctl_table kern_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 #endif
-#ifdef CONFIG_KEYS
-	{
-		.procname	= "keys",
-		.mode		= 0555,
-		.child		= key_sysctls,
-	},
-#endif
 #ifdef CONFIG_PERF_EVENTS
 	/*
 	 * User-space scripts rely on the existence of this file
@@ -2348,17 +2336,17 @@ static struct ctl_table dev_table[] = {
 	{ }
 };
 
-DECLARE_SYSCTL_BASE(kernel, kern_table);
-DECLARE_SYSCTL_BASE(vm, vm_table);
-DECLARE_SYSCTL_BASE(debug, debug_table);
-DECLARE_SYSCTL_BASE(dev, dev_table);
-
 int __init sysctl_init_bases(void)
 {
-	register_sysctl_base(kernel);
-	register_sysctl_base(vm);
-	register_sysctl_base(debug);
-	register_sysctl_base(dev);
+	register_sysctl_init("kernel", kern_table);
+	register_sysctl_init("kernel/usermodehelper", usermodehelper_table);
+#ifdef CONFIG_KEYS
+	register_sysctl_init("kernel/keys", key_sysctls);
+#endif
+
+	register_sysctl_init("vm", vm_table);
+	register_sysctl_init("debug", debug_table);
+	register_sysctl_init("dev", dev_table);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH v4 7/8] sysctl: Refactor base paths registrations

[Christian Brauner]

On Tue, May 23, 2023 at 02:22:19PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. The old way of doing this through
> register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
> call to register_sysctl_init. The 5 base paths affected are: "kernel",
> "vm", "debug", "dev" and "fs".
> 
> We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
> macro since they are no longer needed.
> 
> In order to quickly acertain that the paths did not actually change I
> executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
> same before and after the commit.
> 
> We end up saving 563 bytes with this change:
> 
> ./scripts/bloat-o-meter vmlinux.0.base vmlinux.1.refactor-base-paths
> add/remove: 0/5 grow/shrink: 2/0 up/down: 77/-640 (-563)
> Function                                     old     new   delta
> sysctl_init_bases                             55     111     +56
> init_fs_sysctls                               12      33     +21
> vm_base_table                                128       -    -128
> kernel_base_table                            128       -    -128
> fs_base_table                                128       -    -128
> dev_base_table                               128       -    -128
> debug_base_table                             128       -    -128
> Total: Before=21258215, After=21257652, chg -0.00%
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> [mcgrof: modified to use register_sysctl_init() over register_sysctl()
>  and add bloat-o-meter stats]
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---

Acked-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH v4 7/8] sysctl: Refactor base paths registrations

[Dan Carpenter]

On Tue, May 23, 2023 at 02:22:19PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. The old way of doing this through
> register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
> call to register_sysctl_init. The 5 base paths affected are: "kernel",
> "vm", "debug", "dev" and "fs".
> 
> We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
> macro since they are no longer needed.
> 
> In order to quickly acertain that the paths did not actually change I
> executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
> same before and after the commit.
> 
> We end up saving 563 bytes with this change:
> 
> ./scripts/bloat-o-meter vmlinux.0.base vmlinux.1.refactor-base-paths
> add/remove: 0/5 grow/shrink: 2/0 up/down: 77/-640 (-563)
> Function                                     old     new   delta
> sysctl_init_bases                             55     111     +56
> init_fs_sysctls                               12      33     +21
> vm_base_table                                128       -    -128
> kernel_base_table                            128       -    -128
> fs_base_table                                128       -    -128
> dev_base_table                               128       -    -128
> debug_base_table                             128       -    -128
> Total: Before=21258215, After=21257652, chg -0.00%
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> [mcgrof: modified to use register_sysctl_init() over register_sysctl()
>  and add bloat-o-meter stats]
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>

This needs a Fixes tag so it doesn't get backported by some weird fluke.
Or you could just fold it in with the original patch which introduced
the bug.

Probably add a copy of the output from dmesg?  Maybe add some
Reported-by tags?

regards,
dan carpenter
> 

Re: [PATCH v4 7/8] sysctl: Refactor base paths registrations

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

On Thu, May 25, 2023 at 11:37:47AM +0300, Dan Carpenter wrote:
> On Tue, May 23, 2023 at 02:22:19PM +0200, Joel Granados wrote:
> > This is part of the general push to deprecate register_sysctl_paths and
> > register_sysctl_table. The old way of doing this through
> > register_sysctl_base and DECLARE_SYSCTL_BASE macro is replaced with a
> > call to register_sysctl_init. The 5 base paths affected are: "kernel",
> > "vm", "debug", "dev" and "fs".
> > 
> > We remove the register_sysctl_base function and the DECLARE_SYSCTL_BASE
> > macro since they are no longer needed.
> > 
> > In order to quickly acertain that the paths did not actually change I
> > executed `find /proc/sys/ | sha1sum` and made sure that the sha was the
> > same before and after the commit.
> > 
> > We end up saving 563 bytes with this change:
> > 
> > ./scripts/bloat-o-meter vmlinux.0.base vmlinux.1.refactor-base-paths
> > add/remove: 0/5 grow/shrink: 2/0 up/down: 77/-640 (-563)
> > Function                                     old     new   delta
> > sysctl_init_bases                             55     111     +56
> > init_fs_sysctls                               12      33     +21
> > vm_base_table                                128       -    -128
> > kernel_base_table                            128       -    -128
> > fs_base_table                                128       -    -128
> > dev_base_table                               128       -    -128
> > debug_base_table                             128       -    -128
> > Total: Before=21258215, After=21257652, chg -0.00%
> > 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > [mcgrof: modified to use register_sysctl_init() over register_sysctl()
> >  and add bloat-o-meter stats]
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> This needs a Fixes tag so it doesn't get backported by some weird fluke.
> Or you could just fold it in with the original patch which introduced
I folded it into the original patch

thx

Best
> the bug.
> 
> Probably add a copy of the output from dmesg?  Maybe add some
> Reported-by tags?
> 
> regards,
> dan carpenter
> > 

-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v4 8/8] sysctl: Remove register_sysctl_table

[Joel Granados]

This is part of the general push to deprecate register_sysctl_paths and
register_sysctl_table. After removing all the calling functions, we
remove both the register_sysctl_table function and the documentation
check that appeared in check-sysctl-docs awk script.

We save 595 bytes with this change:

./scripts/bloat-o-meter vmlinux.1.refactor-base-paths vmlinux.2.remove-sysctl-table
add/remove: 2/8 grow/shrink: 1/0 up/down: 1154/-1749 (-595)
Function                                     old     new   delta
count_subheaders                               -     983    +983
unregister_sysctl_table                       29     184    +155
__pfx_count_subheaders                         -      16     +16
__pfx_unregister_sysctl_table.part            16       -     -16
__pfx_register_leaf_sysctl_tables.constprop   16       -     -16
__pfx_count_subheaders.part                   16       -     -16
__pfx___register_sysctl_base                  16       -     -16
unregister_sysctl_table.part                 136       -    -136
__register_sysctl_base                       478       -    -478
register_leaf_sysctl_tables.constprop        524       -    -524
count_subheaders.part                        547       -    -547
Total: Before=21257652, After=21257057, chg -0.00%

Signed-off-by: Joel Granados <j.granados@samsung.com>
[mcgrof: remove register_leaf_sysctl_tables and append_path too and
 add bloat-o-meter stats]
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 fs/proc/proc_sysctl.c     | 159 --------------------------------------
 scripts/check-sysctl-docs |  10 ---
 2 files changed, 169 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index f8f19e000d76..8873812d22f3 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1466,19 +1466,6 @@ void __init __register_sysctl_init(const char *path, struct ctl_table *table,
 	kmemleak_not_leak(hdr);
 }
 
-static char *append_path(const char *path, char *pos, const char *name)
-{
-	int namelen;
-	namelen = strlen(name);
-	if (((pos - path) + namelen + 2) >= PATH_MAX)
-		return NULL;
-	memcpy(pos, name, namelen);
-	pos[namelen] = '/';
-	pos[namelen + 1] = '\0';
-	pos += namelen + 1;
-	return pos;
-}
-
 static int count_subheaders(struct ctl_table *table)
 {
 	int has_files = 0;
@@ -1498,152 +1485,6 @@ static int count_subheaders(struct ctl_table *table)
 	return nr_subheaders + has_files;
 }
 
-static int register_leaf_sysctl_tables(const char *path, char *pos,
-	struct ctl_table_header ***subheader, struct ctl_table_set *set,
-	struct ctl_table *table)
-{
-	struct ctl_table *ctl_table_arg = NULL;
-	struct ctl_table *entry, *files;
-	int nr_files = 0;
-	int nr_dirs = 0;
-	int err = -ENOMEM;
-
-	list_for_each_table_entry(entry, table) {
-		if (entry->child)
-			nr_dirs++;
-		else
-			nr_files++;
-	}
-
-	files = table;
-	/* If there are mixed files and directories we need a new table */
-	if (nr_dirs && nr_files) {
-		struct ctl_table *new;
-		files = kcalloc(nr_files + 1, sizeof(struct ctl_table),
-				GFP_KERNEL);
-		if (!files)
-			goto out;
-
-		ctl_table_arg = files;
-		new = files;
-
-		list_for_each_table_entry(entry, table) {
-			if (entry->child)
-				continue;
-			*new = *entry;
-			new++;
-		}
-	}
-
-	/* Register everything except a directory full of subdirectories */
-	if (nr_files || !nr_dirs) {
-		struct ctl_table_header *header;
-		header = __register_sysctl_table(set, path, files);
-		if (!header) {
-			kfree(ctl_table_arg);
-			goto out;
-		}
-
-		/* Remember if we need to free the file table */
-		header->ctl_table_arg = ctl_table_arg;
-		**subheader = header;
-		(*subheader)++;
-	}
-
-	/* Recurse into the subdirectories. */
-	list_for_each_table_entry(entry, table) {
-		char *child_pos;
-
-		if (!entry->child)
-			continue;
-
-		err = -ENAMETOOLONG;
-		child_pos = append_path(path, pos, entry->procname);
-		if (!child_pos)
-			goto out;
-
-		err = register_leaf_sysctl_tables(path, child_pos, subheader,
-						  set, entry->child);
-		pos[0] = '\0';
-		if (err)
-			goto out;
-	}
-	err = 0;
-out:
-	/* On failure our caller will unregister all registered subheaders */
-	return err;
-}
-
-/**
- * register_sysctl_table - register a sysctl table hierarchy
- * @table: the top-level table structure
- *
- * Register a sysctl table hierarchy. @table should be a filled in ctl_table
- * array. A completely 0 filled entry terminates the table.
- * We are slowly deprecating this call so avoid its use.
- */
-static struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
-{
-	struct ctl_table *ctl_table_arg = table;
-	int nr_subheaders = count_subheaders(table);
-	struct ctl_table_header *header = NULL, **subheaders, **subheader;
-	char *new_path, *pos;
-
-	pos = new_path = kmalloc(PATH_MAX, GFP_KERNEL);
-	if (!new_path)
-		return NULL;
-
-	pos[0] = '\0';
-	while (table->procname && table->child && !table[1].procname) {
-		pos = append_path(new_path, pos, table->procname);
-		if (!pos)
-			goto out;
-		table = table->child;
-	}
-	if (nr_subheaders == 1) {
-		header = __register_sysctl_table(&sysctl_table_root.default_set, new_path, table);
-		if (header)
-			header->ctl_table_arg = ctl_table_arg;
-	} else {
-		header = kzalloc(sizeof(*header) +
-				 sizeof(*subheaders)*nr_subheaders, GFP_KERNEL);
-		if (!header)
-			goto out;
-
-		subheaders = (struct ctl_table_header **) (header + 1);
-		subheader = subheaders;
-		header->ctl_table_arg = ctl_table_arg;
-
-		if (register_leaf_sysctl_tables(new_path, pos, &subheader,
-						&sysctl_table_root.default_set, table))
-			goto err_register_leaves;
-	}
-
-out:
-	kfree(new_path);
-	return header;
-
-err_register_leaves:
-	while (subheader > subheaders) {
-		struct ctl_table_header *subh = *(--subheader);
-		struct ctl_table *table = subh->ctl_table_arg;
-		unregister_sysctl_table(subh);
-		kfree(table);
-	}
-	kfree(header);
-	header = NULL;
-	goto out;
-}
-
-int __register_sysctl_base(struct ctl_table *base_table)
-{
-	struct ctl_table_header *hdr;
-
-	hdr = register_sysctl_table(base_table);
-	kmemleak_not_leak(hdr);
-	return 0;
-}
-
 static void put_links(struct ctl_table_header *header)
 {
 	struct ctl_table_set *root_set = &sysctl_table_root.default_set;
diff --git a/scripts/check-sysctl-docs b/scripts/check-sysctl-docs
index edc9a629d79e..4f163e0bf6a4 100755
--- a/scripts/check-sysctl-docs
+++ b/scripts/check-sysctl-docs
@@ -146,16 +146,6 @@ curtable && /\.procname[\t ]*=[\t ]*".+"/ {
     children[curtable][curentry] = child
 }
 
-/register_sysctl_table\(.*\)/ {
-    match($0, /register_sysctl_table\(([^)]+)\)/, tables)
-    if (debug) print "Registering table " tables[1]
-    if (children[tables[1]][table]) {
-	for (entry in entries[children[tables[1]][table]]) {
-	    printentry(entry)
-	}
-    }
-}
-
 END {
     for (entry in documented) {
 	if (!seen[entry]) {
-- 
2.30.2

Re: [PATCH v4 8/8] sysctl: Remove register_sysctl_table

[Christian Brauner]

On Tue, May 23, 2023 at 02:22:20PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. After removing all the calling functions, we
> remove both the register_sysctl_table function and the documentation
> check that appeared in check-sysctl-docs awk script.
> 
> We save 595 bytes with this change:
> 
> ./scripts/bloat-o-meter vmlinux.1.refactor-base-paths vmlinux.2.remove-sysctl-table
> add/remove: 2/8 grow/shrink: 1/0 up/down: 1154/-1749 (-595)
> Function                                     old     new   delta
> count_subheaders                               -     983    +983
> unregister_sysctl_table                       29     184    +155
> __pfx_count_subheaders                         -      16     +16
> __pfx_unregister_sysctl_table.part            16       -     -16
> __pfx_register_leaf_sysctl_tables.constprop   16       -     -16
> __pfx_count_subheaders.part                   16       -     -16
> __pfx___register_sysctl_base                  16       -     -16
> unregister_sysctl_table.part                 136       -    -136
> __register_sysctl_base                       478       -    -478
> register_leaf_sysctl_tables.constprop        524       -    -524
> count_subheaders.part                        547       -    -547
> Total: Before=21257652, After=21257057, chg -0.00%
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> [mcgrof: remove register_leaf_sysctl_tables and append_path too and
>  add bloat-o-meter stats]
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---

Acked-by: Christian Brauner <brauner@kernel.org>

Re: [PATCH v4 0/8] sysctl: Completely remove register_sysctl_table from sources

[Luis Chamberlain]

On Tue, May 23, 2023 at 02:22:12PM +0200, Joel Granados wrote:
> This is part of the general push to deprecate register_sysctl_paths and
> register_sysctl_table. It contains 2 patchsets that were originally sent
> separately. I have put them together because the second followed the first.
> 
> Parport driver uses the "CHILD" pointer in the ctl_table structure to create
> its directory structure. We move to the newer register_sysctl call and remove
> the pointer madness. I have separated the parport into 5 patches to clarify the
> different changes needed for the 3 calls to register_sysctl_paths.
> 
> We no longer export the register_sysctl_table call as parport was the
> last user from outside proc_sysctl.c. Also modified documentation slightly
> so register_sysctl_table is no longer mentioned.
> 
> Replace register_sysctl_table with register_sysctl effectively effectively
> transitioning 5 base paths ("kernel", "vm", "fs", "dev" and "debug") to the new
> call. Besides removing the actual function, I also removed it from the checks
> done in check-sysctl-docs. @mcgrof went a bit further and removed 2 more
> functions.
> 
> Testing for this change was done in the same way as with previous sysctl
> replacement patches: I made sure that the result of `find /proc/sys/ | sha1sum`
> was the same before and after the patchset.

Thanks! Queued up onto sysctl-next!

  Luis