[PATCH 0/2] sysctl: death to register_sysctl_paths()

[PATCH 0/2] sysctl: death to register_sysctl_paths()

[Luis Chamberlain]

Linus,

As mentioned on my first pull request for sysctl-next, for v6.4-rc1
we're very close to being able to deprecating register_sysctl_paths().
I was going to assess the situation after the first week of the merge
window.

That time is now and things are looking good. We only have one stragglers
on the patch which had already an ACK for so I'm picking this up here now and
the last patch is the one that uses an axe. Some careful eyeballing would
be appreciated by others. If this doesn't get properly reviewed I can also
just hold off on this in my tree for the next merge window. Either way is
fine by me.

I have boot tested the last patch and 0-day build is ongoing. You can give
it a day for a warm fuzzy build test result.

Luis Chamberlain (2):
  kernel: pid_namespace: simplify sysctls with register_sysctl()
  sysctl: remove register_sysctl_paths()

 fs/proc/proc_sysctl.c     | 55 +++------------------------------------
 include/linux/sysctl.h    | 12 ---------
 kernel/pid_namespace.c    |  3 +--
 kernel/pid_sysctl.h       |  3 +--
 scripts/check-sysctl-docs | 16 ------------
 5 files changed, 6 insertions(+), 83 deletions(-)

-- 
2.39.2

[PATCH 1/2] kernel: pid_namespace: simplify sysctls with register_sysctl()

[Luis Chamberlain]

register_sysctl_paths() is only required if your child (directories)
have entries and pid_namespace does not. So use register_sysctl_init()
instead where we don't care about the return value and use
register_sysctl() where we do.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Jeff Xu <jeffxu@google.com>
Link: https://lore.kernel.org/r/20230302202826.776286-9-mcgrof@kernel.org
---
 kernel/pid_namespace.c | 3 +--
 kernel/pid_sysctl.h    | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 46e0d5a3f91f..b43eee07b00c 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -314,7 +314,6 @@ static struct ctl_table pid_ns_ctl_table[] = {
 	},
 	{ }
 };
-static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
 #endif	/* CONFIG_CHECKPOINT_RESTORE */
 
 int reboot_pid_ns(struct pid_namespace *pid_ns, int cmd)
@@ -473,7 +472,7 @@ static __init int pid_namespaces_init(void)
 	pid_ns_cachep = KMEM_CACHE(pid_namespace, SLAB_PANIC | SLAB_ACCOUNT);
 
 #ifdef CONFIG_CHECKPOINT_RESTORE
-	register_sysctl_paths(kern_path, pid_ns_ctl_table);
+	register_sysctl_init("kernel", pid_ns_ctl_table);
 #endif
 
 	register_pid_ns_sysctl_table_vm();
diff --git a/kernel/pid_sysctl.h b/kernel/pid_sysctl.h
index e22d072e1e24..d67a4d45bb42 100644
--- a/kernel/pid_sysctl.h
+++ b/kernel/pid_sysctl.h
@@ -46,10 +46,9 @@ static struct ctl_table pid_ns_ctl_table_vm[] = {
 	},
 	{ }
 };
-static struct ctl_path vm_path[] = { { .procname = "vm", }, { } };
 static inline void register_pid_ns_sysctl_table_vm(void)
 {
-	register_sysctl_paths(vm_path, pid_ns_ctl_table_vm);
+	register_sysctl("vm", pid_ns_ctl_table_vm);
 }
 #else
 static inline void initialize_memfd_noexec_scope(struct pid_namespace *ns) {}
-- 
2.39.2

[PATCH 2/2] sysctl: remove register_sysctl_paths()

[Luis Chamberlain]

The deprecation for register_sysctl_paths() is over. We can rejoice as
we nuke register_sysctl_paths(). The routine register_sysctl_table()
was the only user left of register_sysctl_paths(), so we can now just
open code and move the implementation over to what used to be
to __register_sysctl_paths().

The old dynamic struct ctl_table_set *set is now the point to
sysctl_table_root.default_set.

The old dynamic const struct ctl_path *path was being used in the
routine register_sysctl_paths() with a static:

static const struct ctl_path null_path[] = { {} };

Since this is a null path we can now just simplfy the old routine
and remove its use as its always empty.

This saves us a total of 230 bytes.

$ ./scripts/bloat-o-meter vmlinux.old vmlinux
add/remove: 2/7 grow/shrink: 1/1 up/down: 1015/-1245 (-230)
Function                                     old     new   delta
register_leaf_sysctl_tables.constprop          -     524    +524
register_sysctl_table                         22     497    +475
__pfx_register_leaf_sysctl_tables.constprop       -      16     +16
null_path                                      8       -      -8
__pfx_register_sysctl_paths                   16       -     -16
__pfx_register_leaf_sysctl_tables             16       -     -16
__pfx___register_sysctl_paths                 16       -     -16
__register_sysctl_base                        29      12     -17
register_sysctl_paths                         18       -     -18
register_leaf_sysctl_tables                  534       -    -534
__register_sysctl_paths                      620       -    -620
Total: Before=21259666, After=21259436, chg -0.00%

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 fs/proc/proc_sysctl.c     | 55 +++------------------------------------
 include/linux/sysctl.h    | 12 ---------
 scripts/check-sysctl-docs | 16 ------------
 3 files changed, 4 insertions(+), 79 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 81dbb175017e..8038833ff5b0 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1575,25 +1575,18 @@ static int register_leaf_sysctl_tables(const char *path, char *pos,
 }
 
 /**
- * __register_sysctl_paths - register a sysctl table hierarchy
- * @set: Sysctl tree to register on
- * @path: The path to the directory the sysctl table is in.
+ * 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.
- *
- * See __register_sysctl_table for more details.
  */
-struct ctl_table_header *__register_sysctl_paths(
-	struct ctl_table_set *set,
-	const struct ctl_path *path, struct ctl_table *table)
+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;
-	const struct ctl_path *component;
 	char *new_path, *pos;
 
 	pos = new_path = kmalloc(PATH_MAX, GFP_KERNEL);
@@ -1601,11 +1594,6 @@ struct ctl_table_header *__register_sysctl_paths(
 		return NULL;
 
 	pos[0] = '\0';
-	for (component = path; component->procname; component++) {
-		pos = append_path(new_path, pos, component->procname);
-		if (!pos)
-			goto out;
-	}
 	while (table->procname && table->child && !table[1].procname) {
 		pos = append_path(new_path, pos, table->procname);
 		if (!pos)
@@ -1613,7 +1601,7 @@ struct ctl_table_header *__register_sysctl_paths(
 		table = table->child;
 	}
 	if (nr_subheaders == 1) {
-		header = __register_sysctl_table(set, new_path, table);
+		header = __register_sysctl_table(&sysctl_table_root.default_set, new_path, table);
 		if (header)
 			header->ctl_table_arg = ctl_table_arg;
 	} else {
@@ -1627,7 +1615,7 @@ struct ctl_table_header *__register_sysctl_paths(
 		header->ctl_table_arg = ctl_table_arg;
 
 		if (register_leaf_sysctl_tables(new_path, pos, &subheader,
-						set, table))
+						&sysctl_table_root.default_set, table))
 			goto err_register_leaves;
 	}
 
@@ -1646,41 +1634,6 @@ struct ctl_table_header *__register_sysctl_paths(
 	header = NULL;
 	goto out;
 }
-
-/**
- * register_sysctl_paths - register a sysctl table hierarchy
- * @path: The path to the directory the sysctl table is in.
- * @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 caller so avoid future uses of it.
- *
- * See __register_sysctl_paths for more details.
- */
-struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
-						struct ctl_table *table)
-{
-	return __register_sysctl_paths(&sysctl_table_root.default_set,
-					path, table);
-}
-EXPORT_SYMBOL(register_sysctl_paths);
-
-/**
- * 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.
- *
- * See register_sysctl_paths for more details.
- */
-struct ctl_table_header *register_sysctl_table(struct ctl_table *table)
-{
-	static const struct ctl_path null_path[] = { {} };
-
-	return register_sysctl_paths(null_path, table);
-}
 EXPORT_SYMBOL(register_sysctl_table);
 
 int __register_sysctl_base(struct ctl_table *base_table)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 780690dc08cd..3d08277959af 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -221,14 +221,8 @@ extern void retire_sysctl_set(struct ctl_table_set *set);
 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_paths(
-	struct ctl_table_set *set,
-	const struct ctl_path *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);
-struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
-						struct ctl_table *table);
-
 void unregister_sysctl_table(struct ctl_table_header * table);
 
 extern int sysctl_init_bases(void);
@@ -277,12 +271,6 @@ static inline struct ctl_table_header *register_sysctl_mount_point(const char *p
 	return NULL;
 }
 
-static inline struct ctl_table_header *register_sysctl_paths(
-			const struct ctl_path *path, struct ctl_table *table)
-{
-	return NULL;
-}
-
 static inline struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table)
 {
 	return NULL;
diff --git a/scripts/check-sysctl-docs b/scripts/check-sysctl-docs
index 8bcb9e26c7bc..edc9a629d79e 100755
--- a/scripts/check-sysctl-docs
+++ b/scripts/check-sysctl-docs
@@ -156,22 +156,6 @@ curtable && /\.procname[\t ]*=[\t ]*".+"/ {
     }
 }
 
-/register_sysctl_paths\(.*\)/ {
-    match($0, /register_sysctl_paths\(([^)]+), ([^)]+)\)/, tables)
-    if (debug) print "Attaching table " tables[2] " to path " tables[1]
-    if (paths[tables[1]] == table) {
-	for (entry in entries[tables[2]]) {
-	    printentry(entry)
-	}
-    }
-    split(paths[tables[1]], components, "/")
-    if (length(components) > 1 && components[1] == table) {
-	# Count the first subdirectory as seen
-	seen[components[2]]++
-    }
-}
-
-
 END {
     for (entry in documented) {
 	if (!seen[entry]) {
-- 
2.39.2

Re: [PATCH 0/2] sysctl: death to register_sysctl_paths()

[Luis Chamberlain]

On Tue, May 02, 2023 at 07:33:27PM -0700, Luis Chamberlain wrote:
> You can give it a day for a warm fuzzy build test result.

0-day gives its blessings.

  Luis

Re: [PATCH 0/2] sysctl: death to register_sysctl_paths()

[Linus Torvalds]

On Wed, May 3, 2023 at 9:24 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Tue, May 02, 2023 at 07:33:27PM -0700, Luis Chamberlain wrote:
> > You can give it a day for a warm fuzzy build test result.
>
> 0-day gives its blessings.

Well, it's not like I can pull anyway, since you didn't actually say
where to pull *from*. And I don't want to randomly apply patches when
I know you have a git tree for this.

So please do a proper pull request.

              Linus

Re: [PATCH 0/2] sysctl: death to register_sysctl_paths()

[Luis Chamberlain]

On Wed, May 03, 2023 at 11:29:10AM -0700, Linus Torvalds wrote:
> On Wed, May 3, 2023 at 9:24 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > On Tue, May 02, 2023 at 07:33:27PM -0700, Luis Chamberlain wrote:
> > > You can give it a day for a warm fuzzy build test result.
> >
> > 0-day gives its blessings.
> 
> Well, it's not like I can pull anyway, since you didn't actually say
> where to pull *from*. And I don't want to randomly apply patches when
> I know you have a git tree for this.
> 
> So please do a proper pull request.

Sorry thought you don't mind a few patches, so ditched the formalities
for the pull. Now I know you prefer to pull over a couple of patches,
will send up next!

  Luis

Re: [PATCH 0/2] sysctl: death to register_sysctl_paths()

[Linus Torvalds]

On Wed, May 3, 2023 at 12:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> Sorry thought you don't mind a few patches, so ditched the formalities
> for the pull.

So I don't mind patches per se, and when there's a reason for them I
have no problem at all taking them.

The reason is typically something like "let's short-circuit the normal
channels just to get this trivial thing sorted out and we can forget
about it", but it can also be just a practical thing like "I'm
traveling so it would be easier if you'd just pick up this patch
directly from the mailing list".

Or it could be "I don't have a git tree since I'm not a main
developer, so I just send patches".

All of those are situations where I'll happily take patches directly.

But on the whole, when there isn't any real reason to avoid a pull
request, I'd much rather have the full thing with signature and
everything...

               Linus