diff mbox series

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

Message ID 20230526222207.982107-2-mcgrof@kernel.org
State New
Headers
Series kernel/sysctl.c: remove to major base directories |

Commit Message

Luis Chamberlain May 26, 2023, 10:22 p.m. UTC 
  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(-)

Comments

Joel Granados May 29, 2023, 8:04 p.m. UTC | #1 
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
Luis Chamberlain May 30, 2023, 4:42 p.m. UTC | #2 
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
Joel Granados May 30, 2023, 9:10 p.m. UTC | #3 
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 May 30, 2023, 9:13 p.m. UTC | #4 
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>
Luis Chamberlain May 30, 2023, 10:55 p.m. UTC | #5 
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
Joel Granados June 1, 2023, 5:35 a.m. UTC | #6 
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
diff mbox series

Patch

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;
 }