[PATCH v2 0/2] kernel/sysctl.c: remove to major base directories

[PATCH v2 0/2] kernel/sysctl.c: remove to major base directories

[Luis Chamberlain]

Changes on this v2:

  o remove header changes to architecture code. If they were already
    comiling this should not fail

Now that Joel has cleaned up and removed one of the routines which we wanted
to deprecate, remove two major arrays from kernel/sysctl.c which are empty or
almost empty. One of them, the debug one just needs moving to its source, so
do that.
                                                                                                                                                                                              
The move for the signal sysctl costs us 23 bytes but we have already saved
1465 bytes with the other recent cleanup Joel made. The next step is to
depreecate one more call and then we can simplify the registration to only
use ARRAY_SIZE() completely and remove the extra empty entries all over.
That should save us tons of bytes all around in the kernel and we'd then 
later kill for good all recursion possible sysctl registration calls.
                                                                                                                                                                                             
These patches apply on top of sysctl-next [0] which already carry Joel's
patches.                                                                                                             
                                                                                                                                                                                              
[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-next  

Luis Chamberlain (2):
  sysctl: remove empty dev table
  signal: move show_unhandled_signals sysctl to its own file

 kernel/signal.c | 23 +++++++++++++++++++++++
 kernel/sysctl.c | 19 -------------------
 2 files changed, 23 insertions(+), 19 deletions(-)

-- 
2.39.2

[PATCH v2 1/2] sysctl: remove empty dev table

[Luis Chamberlain]

Now that all the dev sysctls have been moved out we can remove the
dev sysctl base directory. We don't need to create base directories,
they are created for you as if using 'mkdir -p' with register_syctl()
and register_sysctl_init(). For details refer to sysctl_mkdir_p()
usage.

We save 90 bytes with this changes:

./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
Function                                     old     new   delta
sysctl_init_bases                            111      85     -26
dev_table                                     64       -     -64
Total: Before=21257057, After=21256967, chg -0.00%

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kernel/sysctl.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index fa2aa8bd32b6..a7fdb828afb6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
 	{ }
 };
 
-static struct ctl_table dev_table[] = {
-	{ }
-};
-
 int __init sysctl_init_bases(void)
 {
 	register_sysctl_init("kernel", kern_table);
 	register_sysctl_init("vm", vm_table);
 	register_sysctl_init("debug", debug_table);
-	register_sysctl_init("dev", dev_table);
 
 	return 0;
 }
-- 
2.39.2

[PATCH v2 2/2] signal: move show_unhandled_signals sysctl to its own file

[Luis Chamberlain]

The show_unhandled_signals sysctl is the only sysctl for debug
left on kernel/sysctl.c. We've been moving the syctls out from
kernel/sysctl.c so to help avoid merge conflicts as the shared
array gets out of hand.

This change incurs simplifies sysctl registration by localizing
it where it should go for a penalty in size of increasing the
kernel by 23 bytes, we accept this given recent cleanups have
actually already saved us 1465 bytes in the prior commits.

./scripts/bloat-o-meter vmlinux.3-remove-dev-table vmlinux.4-remove-debug-table
add/remove: 3/1 grow/shrink: 0/1 up/down: 177/-154 (23)
Function                                     old     new   delta
signal_debug_table                             -     128    +128
init_signal_sysctls                            -      33     +33
__pfx_init_signal_sysctls                      -      16     +16
sysctl_init_bases                             85      59     -26
debug_table                                  128       -    -128
Total: Before=21256967, After=21256990, chg +0.00%

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kernel/signal.c | 23 +++++++++++++++++++++++
 kernel/sysctl.c | 14 --------------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 8f6330f0e9ca..5ba4150c01a7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -45,6 +45,7 @@
 #include <linux/posix-timers.h>
 #include <linux/cgroup.h>
 #include <linux/audit.h>
+#include <linux/sysctl.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/signal.h>
@@ -4771,6 +4772,28 @@ static inline void siginfo_buildtime_checks(void)
 #endif
 }
 
+#if defined(CONFIG_SYSCTL)
+static struct ctl_table signal_debug_table[] = {
+#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
+	{
+		.procname	= "exception-trace",
+		.data		= &show_unhandled_signals,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
+#endif
+	{ }
+};
+
+static int __init init_signal_sysctls(void)
+{
+	register_sysctl_init("debug", signal_debug_table);
+	return 0;
+}
+early_initcall(init_signal_sysctls);
+#endif /* CONFIG_SYSCTL */
+
 void __init signals_init(void)
 {
 	siginfo_buildtime_checks();
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index a7fdb828afb6..43240955dcad 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2331,24 +2331,10 @@ static struct ctl_table vm_table[] = {
 	{ }
 };
 
-static struct ctl_table debug_table[] = {
-#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
-	{
-		.procname	= "exception-trace",
-		.data		= &show_unhandled_signals,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec
-	},
-#endif
-	{ }
-};
-
 int __init sysctl_init_bases(void)
 {
 	register_sysctl_init("kernel", kern_table);
 	register_sysctl_init("vm", vm_table);
-	register_sysctl_init("debug", debug_table);
 
 	return 0;
 }
-- 
2.39.2

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Joel Granados]

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

On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> Now that all the dev sysctls have been moved out we can remove the
> dev sysctl base directory. We don't need to create base directories,
> they are created for you as if using 'mkdir -p' with register_syctl()
> and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> usage.
> 
> We save 90 bytes with this changes:
> 
> ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> Function                                     old     new   delta
> sysctl_init_bases                            111      85     -26
> dev_table                                     64       -     -64
> Total: Before=21257057, After=21256967, chg -0.00%
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  kernel/sysctl.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index fa2aa8bd32b6..a7fdb828afb6 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
>  	{ }
>  };
>  
> -static struct ctl_table dev_table[] = {
> -	{ }
> -};
> -
>  int __init sysctl_init_bases(void)
>  {
>  	register_sysctl_init("kernel", kern_table);
>  	register_sysctl_init("vm", vm_table);
>  	register_sysctl_init("debug", debug_table);
> -	register_sysctl_init("dev", dev_table);
>  
>  	return 0;
>  }
> -- 
> 2.39.2
> 
LGTM.

Review
But why was dev there to begin with?

Best

-- 

Joel Granados

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

Re: [PATCH v2 2/2] signal: move show_unhandled_signals sysctl to its own file

[Joel Granados]

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

On Fri, May 26, 2023 at 03:22:06PM -0700, Luis Chamberlain wrote:
> The show_unhandled_signals sysctl is the only sysctl for debug
> left on kernel/sysctl.c. We've been moving the syctls out from
> kernel/sysctl.c so to help avoid merge conflicts as the shared
> array gets out of hand.
> 
> This change incurs simplifies sysctl registration by localizing
> it where it should go for a penalty in size of increasing the
> kernel by 23 bytes, we accept this given recent cleanups have
> actually already saved us 1465 bytes in the prior commits.
> 
> ./scripts/bloat-o-meter vmlinux.3-remove-dev-table vmlinux.4-remove-debug-table
> add/remove: 3/1 grow/shrink: 0/1 up/down: 177/-154 (23)
> Function                                     old     new   delta
> signal_debug_table                             -     128    +128
> init_signal_sysctls                            -      33     +33
> __pfx_init_signal_sysctls                      -      16     +16
> sysctl_init_bases                             85      59     -26
> debug_table                                  128       -    -128
> Total: Before=21256967, After=21256990, chg +0.00%
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  kernel/signal.c | 23 +++++++++++++++++++++++
>  kernel/sysctl.c | 14 --------------
>  2 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 8f6330f0e9ca..5ba4150c01a7 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -45,6 +45,7 @@
>  #include <linux/posix-timers.h>
>  #include <linux/cgroup.h>
>  #include <linux/audit.h>
> +#include <linux/sysctl.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/signal.h>
> @@ -4771,6 +4772,28 @@ static inline void siginfo_buildtime_checks(void)
>  #endif
>  }
>  
> +#if defined(CONFIG_SYSCTL)
> +static struct ctl_table signal_debug_table[] = {
> +#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
> +	{
> +		.procname	= "exception-trace",
> +		.data		= &show_unhandled_signals,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec
> +	},
> +#endif
> +	{ }
> +};
> +
> +static int __init init_signal_sysctls(void)
> +{
> +	register_sysctl_init("debug", signal_debug_table);
> +	return 0;
> +}
> +early_initcall(init_signal_sysctls);
> +#endif /* CONFIG_SYSCTL */
> +
>  void __init signals_init(void)
>  {
>  	siginfo_buildtime_checks();
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index a7fdb828afb6..43240955dcad 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2331,24 +2331,10 @@ static struct ctl_table vm_table[] = {
>  	{ }
>  };
>  
> -static struct ctl_table debug_table[] = {
> -#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
> -	{
> -		.procname	= "exception-trace",
> -		.data		= &show_unhandled_signals,
> -		.maxlen		= sizeof(int),
> -		.mode		= 0644,
> -		.proc_handler	= proc_dointvec
> -	},
> -#endif
> -	{ }
> -};
> -
>  int __init sysctl_init_bases(void)
>  {
>  	register_sysctl_init("kernel", kern_table);
>  	register_sysctl_init("vm", vm_table);
> -	register_sysctl_init("debug", debug_table);
>  
>  	return 0;
>  }
> -- 
> 2.39.2
> 

LGTM
-- 

Joel Granados

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

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Luis Chamberlain]

On Mon, May 29, 2023 at 10:04:57PM +0200, Joel Granados wrote:
> On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> > Now that all the dev sysctls have been moved out we can remove the
> > dev sysctl base directory. We don't need to create base directories,
> > they are created for you as if using 'mkdir -p' with register_syctl()
> > and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> > usage.
> > 
> > We save 90 bytes with this changes:
> > 
> > ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> > add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> > Function                                     old     new   delta
> > sysctl_init_bases                            111      85     -26
> > dev_table                                     64       -     -64
> > Total: Before=21257057, After=21256967, chg -0.00%
> > 
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >  kernel/sysctl.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index fa2aa8bd32b6..a7fdb828afb6 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
> >  	{ }
> >  };
> >  
> > -static struct ctl_table dev_table[] = {
> > -	{ }
> > -};
> > -
> >  int __init sysctl_init_bases(void)
> >  {
> >  	register_sysctl_init("kernel", kern_table);
> >  	register_sysctl_init("vm", vm_table);
> >  	register_sysctl_init("debug", debug_table);
> > -	register_sysctl_init("dev", dev_table);
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.39.2
> > 
> LGTM.

BTW, please use proper tags like Reviewed-by, and so on even if you use
LGTM so that then if anyone uses things like b4 it can pick the tags for
you.

> But why was dev there to begin with?

I will enhance the commit log to mention that, it was there because
old APIs didn't create the directory for you, and now it is clear it
is not needed. I checked ant he dev table was there since the beginning
of sysctl.c on v2.5.0.

  Luis

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Joel Granados]

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

On Tue, May 30, 2023 at 09:42:04AM -0700, Luis Chamberlain wrote:
> On Mon, May 29, 2023 at 10:04:57PM +0200, Joel Granados wrote:
> > On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> > > Now that all the dev sysctls have been moved out we can remove the
> > > dev sysctl base directory. We don't need to create base directories,
> > > they are created for you as if using 'mkdir -p' with register_syctl()
> > > and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> > > usage.
> > > 
> > > We save 90 bytes with this changes:
> > > 
> > > ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> > > add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> > > Function                                     old     new   delta
> > > sysctl_init_bases                            111      85     -26
> > > dev_table                                     64       -     -64
> > > Total: Before=21257057, After=21256967, chg -0.00%
> > > 
> > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > > ---
> > >  kernel/sysctl.c | 5 -----
> > >  1 file changed, 5 deletions(-)
> > > 
> > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > index fa2aa8bd32b6..a7fdb828afb6 100644
> > > --- a/kernel/sysctl.c
> > > +++ b/kernel/sysctl.c
> > > @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
> > >  	{ }
> > >  };
> > >  
> > > -static struct ctl_table dev_table[] = {
> > > -	{ }
> > > -};
> > > -
> > >  int __init sysctl_init_bases(void)
> > >  {
> > >  	register_sysctl_init("kernel", kern_table);
> > >  	register_sysctl_init("vm", vm_table);
> > >  	register_sysctl_init("debug", debug_table);
> > > -	register_sysctl_init("dev", dev_table);
> > >  
> > >  	return 0;
> > >  }
> > > -- 
> > > 2.39.2
> > > 
> > LGTM.
> 
> BTW, please use proper tags like Reviewed-by, and so on even if you use
> LGTM so that then if anyone uses things like b4 it can pick the tags for
> you.
Sure thing. Will send the reviewed-by for the patches

> 
> > But why was dev there to begin with?
> 
> I will enhance the commit log to mention that, it was there because
> old APIs didn't create the directory for you, and now it is clear it
> is not needed. I checked ant he dev table was there since the beginning
> of sysctl.c on v2.5.0.
That would be great!

> 
>   Luis

-- 

Joel Granados

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

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Joel Granados]

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

On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> Now that all the dev sysctls have been moved out we can remove the
> dev sysctl base directory. We don't need to create base directories,
> they are created for you as if using 'mkdir -p' with register_syctl()
> and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> usage.
> 
> We save 90 bytes with this changes:
> 
> ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> Function                                     old     new   delta
> sysctl_init_bases                            111      85     -26
> dev_table                                     64       -     -64
> Total: Before=21257057, After=21256967, chg -0.00%
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  kernel/sysctl.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index fa2aa8bd32b6..a7fdb828afb6 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
>  	{ }
>  };
>  
> -static struct ctl_table dev_table[] = {
> -	{ }
> -};
> -
>  int __init sysctl_init_bases(void)
>  {
>  	register_sysctl_init("kernel", kern_table);
>  	register_sysctl_init("vm", vm_table);
>  	register_sysctl_init("debug", debug_table);
> -	register_sysctl_init("dev", dev_table);
>  
>  	return 0;
>  }
> -- 
> 2.39.2
> 
Reviewed-by: Joel Granados <j.granados@samsung.com>

-- 

Joel Granados

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

Re: [PATCH v2 2/2] signal: move show_unhandled_signals sysctl to its own file

[Joel Granados]

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

On Fri, May 26, 2023 at 03:22:06PM -0700, Luis Chamberlain wrote:
> The show_unhandled_signals sysctl is the only sysctl for debug
> left on kernel/sysctl.c. We've been moving the syctls out from
> kernel/sysctl.c so to help avoid merge conflicts as the shared
> array gets out of hand.
> 
> This change incurs simplifies sysctl registration by localizing
> it where it should go for a penalty in size of increasing the
> kernel by 23 bytes, we accept this given recent cleanups have
> actually already saved us 1465 bytes in the prior commits.
> 
> ./scripts/bloat-o-meter vmlinux.3-remove-dev-table vmlinux.4-remove-debug-table
> add/remove: 3/1 grow/shrink: 0/1 up/down: 177/-154 (23)
> Function                                     old     new   delta
> signal_debug_table                             -     128    +128
> init_signal_sysctls                            -      33     +33
> __pfx_init_signal_sysctls                      -      16     +16
> sysctl_init_bases                             85      59     -26
> debug_table                                  128       -    -128
> Total: Before=21256967, After=21256990, chg +0.00%
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  kernel/signal.c | 23 +++++++++++++++++++++++
>  kernel/sysctl.c | 14 --------------
>  2 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 8f6330f0e9ca..5ba4150c01a7 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -45,6 +45,7 @@
>  #include <linux/posix-timers.h>
>  #include <linux/cgroup.h>
>  #include <linux/audit.h>
> +#include <linux/sysctl.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/signal.h>
> @@ -4771,6 +4772,28 @@ static inline void siginfo_buildtime_checks(void)
>  #endif
>  }
>  
> +#if defined(CONFIG_SYSCTL)
> +static struct ctl_table signal_debug_table[] = {
> +#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
> +	{
> +		.procname	= "exception-trace",
> +		.data		= &show_unhandled_signals,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec
> +	},
> +#endif
> +	{ }
> +};
> +
> +static int __init init_signal_sysctls(void)
> +{
> +	register_sysctl_init("debug", signal_debug_table);
> +	return 0;
> +}
> +early_initcall(init_signal_sysctls);
> +#endif /* CONFIG_SYSCTL */
> +
>  void __init signals_init(void)
>  {
>  	siginfo_buildtime_checks();
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index a7fdb828afb6..43240955dcad 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2331,24 +2331,10 @@ static struct ctl_table vm_table[] = {
>  	{ }
>  };
>  
> -static struct ctl_table debug_table[] = {
> -#ifdef CONFIG_SYSCTL_EXCEPTION_TRACE
> -	{
> -		.procname	= "exception-trace",
> -		.data		= &show_unhandled_signals,
> -		.maxlen		= sizeof(int),
> -		.mode		= 0644,
> -		.proc_handler	= proc_dointvec
> -	},
> -#endif
> -	{ }
> -};
> -
>  int __init sysctl_init_bases(void)
>  {
>  	register_sysctl_init("kernel", kern_table);
>  	register_sysctl_init("vm", vm_table);
> -	register_sysctl_init("debug", debug_table);
>  
>  	return 0;
>  }
> -- 
> 2.39.2
> 

Reviewed-by: Joel Granados <j.granados@samsung.com>
-- 

Joel Granados

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

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Luis Chamberlain]

On Tue, May 30, 2023 at 09:42:04AM -0700, Luis Chamberlain wrote:
> On Mon, May 29, 2023 at 10:04:57PM +0200, Joel Granados wrote:
> > On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> > > Now that all the dev sysctls have been moved out we can remove the
> > > dev sysctl base directory. We don't need to create base directories,
> > > they are created for you as if using 'mkdir -p' with register_syctl()
> > > and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> > > usage.
> > > 
> > > We save 90 bytes with this changes:
> > > 
> > > ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> > > add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> > > Function                                     old     new   delta
> > > sysctl_init_bases                            111      85     -26
> > > dev_table                                     64       -     -64
> > > Total: Before=21257057, After=21256967, chg -0.00%
> > > 
> > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > > ---
> > >  kernel/sysctl.c | 5 -----
> > >  1 file changed, 5 deletions(-)
> > > 
> > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > index fa2aa8bd32b6..a7fdb828afb6 100644
> > > --- a/kernel/sysctl.c
> > > +++ b/kernel/sysctl.c
> > > @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
> > >  	{ }
> > >  };
> > >  
> > > -static struct ctl_table dev_table[] = {
> > > -	{ }
> > > -};
> > > -
> > >  int __init sysctl_init_bases(void)
> > >  {
> > >  	register_sysctl_init("kernel", kern_table);
> > >  	register_sysctl_init("vm", vm_table);
> > >  	register_sysctl_init("debug", debug_table);
> > > -	register_sysctl_init("dev", dev_table);
> > >  
> > >  	return 0;
> > >  }
> > > -- 
> > > 2.39.2
> > > 
> > LGTM.
> 
> BTW, please use proper tags like Reviewed-by, and so on even if you use
> LGTM so that then if anyone uses things like b4 it can pick the tags for
> you.
> 
> > But why was dev there to begin with?
> 
> I will enhance the commit log to mention that, it was there because
> old APIs didn't create the directory for you, and now it is clear it
> is not needed. I checked ant he dev table was there since the beginning
> of sysctl.c on v2.5.0.

I've extended the commmit log with this very importance piece of
information:

The empty dev table has been in place since the v2.5.0 days because back
then ordering was essentialy. But later commit 7ec66d06362d ("sysctl:
Stop requiring explicit management of sysctl directories"), merged as of
v3.4-rc1, the entire ordering of directories was replaced by allowing
sysctl directory autogeneration. This new mechanism introduced on v3.4
allows for sysctl directories to automatically be created for sysctl
tables when they are needed and automatically removes them when no
sysctl tables use them.  That commit also added a dedicated struct
ctl_dir as a new type for these autogenerated directories.

  Luis

Re: [PATCH v2 1/2] sysctl: remove empty dev table

[Joel Granados]

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

On Tue, May 30, 2023 at 03:55:10PM -0700, Luis Chamberlain wrote:
> On Tue, May 30, 2023 at 09:42:04AM -0700, Luis Chamberlain wrote:
> > On Mon, May 29, 2023 at 10:04:57PM +0200, Joel Granados wrote:
> > > On Fri, May 26, 2023 at 03:22:05PM -0700, Luis Chamberlain wrote:
> > > > Now that all the dev sysctls have been moved out we can remove the
> > > > dev sysctl base directory. We don't need to create base directories,
> > > > they are created for you as if using 'mkdir -p' with register_syctl()
> > > > and register_sysctl_init(). For details refer to sysctl_mkdir_p()
> > > > usage.
> > > > 
> > > > We save 90 bytes with this changes:
> > > > 
> > > > ./scripts/bloat-o-meter vmlinux.2.remove-sysctl-table vmlinux.3-remove-dev-table
> > > > add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-90 (-90)
> > > > Function                                     old     new   delta
> > > > sysctl_init_bases                            111      85     -26
> > > > dev_table                                     64       -     -64
> > > > Total: Before=21257057, After=21256967, chg -0.00%
> > > > 
> > > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > > > ---
> > > >  kernel/sysctl.c | 5 -----
> > > >  1 file changed, 5 deletions(-)
> > > > 
> > > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > > index fa2aa8bd32b6..a7fdb828afb6 100644
> > > > --- a/kernel/sysctl.c
> > > > +++ b/kernel/sysctl.c
> > > > @@ -2344,16 +2344,11 @@ static struct ctl_table debug_table[] = {
> > > >  	{ }
> > > >  };
> > > >  
> > > > -static struct ctl_table dev_table[] = {
> > > > -	{ }
> > > > -};
> > > > -
> > > >  int __init sysctl_init_bases(void)
> > > >  {
> > > >  	register_sysctl_init("kernel", kern_table);
> > > >  	register_sysctl_init("vm", vm_table);
> > > >  	register_sysctl_init("debug", debug_table);
> > > > -	register_sysctl_init("dev", dev_table);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > -- 
> > > > 2.39.2
> > > > 
> > > LGTM.
> > 
> > BTW, please use proper tags like Reviewed-by, and so on even if you use
> > LGTM so that then if anyone uses things like b4 it can pick the tags for
> > you.
> > 
> > > But why was dev there to begin with?
> > 
> > I will enhance the commit log to mention that, it was there because
> > old APIs didn't create the directory for you, and now it is clear it
> > is not needed. I checked ant he dev table was there since the beginning
> > of sysctl.c on v2.5.0.
> 
> I've extended the commmit log with this very importance piece of
> information:
Awesome!

> 
> The empty dev table has been in place since the v2.5.0 days because back
> then ordering was essentialy. But later commit 7ec66d06362d ("sysctl:
*essential ?

> Stop requiring explicit management of sysctl directories"), merged as of
> v3.4-rc1, the entire ordering of directories was replaced by allowing
> sysctl directory autogeneration. This new mechanism introduced on v3.4
> allows for sysctl directories to automatically be created for sysctl
> tables when they are needed and automatically removes them when no
> sysctl tables use them.  That commit also added a dedicated struct
> ctl_dir as a new type for these autogenerated directories.
> 
>   Luis

-- 

Joel Granados

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