[PATCH v2 0/5] sunrpc: simplfy sysctl registrations

[PATCH v2 0/5] sunrpc: simplfy sysctl registrations

[Luis Chamberlain]

This is my v2 series to simplify sysctl registration for sunrpc. The
first series was posted just yesterday [0]. On this v2 I address the
only compilation issues found by 0day through my entire tree of
sysctl conversions.

Changes sincce v1:

   o Fix compilation when CONFIG_SUNRPC_DEBUG is enabled, I forgot to move the
    proc routines above, and so the 4th patch now does that too.

Feel free to take these patches or let me know and I'm happy to also
take these in through sysctl-next. Typically I use sysctl-next for
core sysctl changes or for kernel/sysctl.c cleanup to avoid conflicts.
All these syctls however are well contained to sunrpc so they can also
go in separately. Let me know how you'd like to go about these patches.

[0] https://lkml.kernel.org/r/20230310225236.3939443-1-mcgrof@kernel.org

Luis Chamberlain (5):
  sunrpc: simplify two-level sysctl registration for tsvcrdma_parm_table
  sunrpc: simplify one-level sysctl registration for xr_tunables_table
  sunrpc: simplify one-level sysctl registration for xs_tunables_table
  sunrpc: move sunrpc_table and proc routines above
  sunrpc: simplify one-level sysctl registration for debug_table

 net/sunrpc/sysctl.c             | 44 ++++++++++++---------------------
 net/sunrpc/xprtrdma/svc_rdma.c  | 21 ++--------------
 net/sunrpc/xprtrdma/transport.c | 11 +--------
 net/sunrpc/xprtsock.c           | 13 ++--------
 4 files changed, 21 insertions(+), 68 deletions(-)

-- 
2.39.1

[PATCH v2 1/5] sunrpc: simplify two-level sysctl registration for tsvcrdma_parm_table

[Luis Chamberlain]

There is no need to declare two tables to just create directories,
this can be easily be done with a prefix path with register_sysctl().

Simplify this registration.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 net/sunrpc/xprtrdma/svc_rdma.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma.c b/net/sunrpc/xprtrdma/svc_rdma.c
index 5bc20e9d09cd..f0d5eeed4c88 100644
--- a/net/sunrpc/xprtrdma/svc_rdma.c
+++ b/net/sunrpc/xprtrdma/svc_rdma.c
@@ -212,24 +212,6 @@ static struct ctl_table svcrdma_parm_table[] = {
 	{ },
 };
 
-static struct ctl_table svcrdma_table[] = {
-	{
-		.procname	= "svc_rdma",
-		.mode		= 0555,
-		.child		= svcrdma_parm_table
-	},
-	{ },
-};
-
-static struct ctl_table svcrdma_root_table[] = {
-	{
-		.procname	= "sunrpc",
-		.mode		= 0555,
-		.child		= svcrdma_table
-	},
-	{ },
-};
-
 static void svc_rdma_proc_cleanup(void)
 {
 	if (!svcrdma_table_header)
@@ -263,7 +245,8 @@ static int svc_rdma_proc_init(void)
 	if (rc)
 		goto out_err;
 
-	svcrdma_table_header = register_sysctl_table(svcrdma_root_table);
+	svcrdma_table_header = register_sysctl("sunrpc/svc_rdma",
+					       svcrdma_parm_table);
 	return 0;
 
 out_err:
-- 
2.39.1

[PATCH v2 2/5] sunrpc: simplify one-level sysctl registration for xr_tunables_table

[Luis Chamberlain]

There is no need to declare an extra tables to just create directory,
this can be easily be done with a prefix path with register_sysctl().

Simplify this registration.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 net/sunrpc/xprtrdma/transport.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 10bb2b929c6d..29b0562d62e7 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -140,15 +140,6 @@ static struct ctl_table xr_tunables_table[] = {
 	{ },
 };
 
-static struct ctl_table sunrpc_table[] = {
-	{
-		.procname	= "sunrpc",
-		.mode		= 0555,
-		.child		= xr_tunables_table
-	},
-	{ },
-};
-
 #endif
 
 static const struct rpc_xprt_ops xprt_rdma_procs;
@@ -799,7 +790,7 @@ int xprt_rdma_init(void)
 
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 	if (!sunrpc_table_header)
-		sunrpc_table_header = register_sysctl_table(sunrpc_table);
+		sunrpc_table_header = register_sysctl("sunrpc", xr_tunables_table);
 #endif
 	return 0;
 }
-- 
2.39.1

[PATCH v2 3/5] sunrpc: simplify one-level sysctl registration for xs_tunables_table

[Luis Chamberlain]

There is no need to declare an extra tables to just create directory,
this can be easily be done with a prefix path with register_sysctl().

Simplify this registration.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 net/sunrpc/xprtsock.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index aaa5b2741b79..46bbd6230650 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -77,7 +77,7 @@ static unsigned int xs_tcp_fin_timeout __read_mostly = XS_TCP_LINGER_TO;
 
 /*
  * We can register our own files under /proc/sys/sunrpc by
- * calling register_sysctl_table() again.  The files in that
+ * calling register_sysctl() again.  The files in that
  * directory become the union of all files registered there.
  *
  * We simply need to make sure that we don't collide with
@@ -157,15 +157,6 @@ static struct ctl_table xs_tunables_table[] = {
 	{ },
 };
 
-static struct ctl_table sunrpc_table[] = {
-	{
-		.procname	= "sunrpc",
-		.mode		= 0555,
-		.child		= xs_tunables_table
-	},
-	{ },
-};
-
 /*
  * Wait duration for a reply from the RPC portmapper.
  */
@@ -3174,7 +3165,7 @@ static struct xprt_class	xs_bc_tcp_transport = {
 int init_socket_xprt(void)
 {
 	if (!sunrpc_table_header)
-		sunrpc_table_header = register_sysctl_table(sunrpc_table);
+		sunrpc_table_header = register_sysctl("sunrpc", xs_tunables_table);
 
 	xprt_register_transport(&xs_local_transport);
 	xprt_register_transport(&xs_udp_transport);
-- 
2.39.1

[PATCH v2 4/5] sunrpc: move sunrpc_table and proc routines above

[Luis Chamberlain]

No need to do a forward declaration for sunrpc_table, just move
the sysctls up as everyone else does it. This will make the next
change easier to read. This change produces no functional changes.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 net/sunrpc/sysctl.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index 3aad6ef18504..a54438d68d1b 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -40,25 +40,6 @@ EXPORT_SYMBOL_GPL(nlm_debug);
 
 #if IS_ENABLED(CONFIG_SUNRPC_DEBUG)
 
-static struct ctl_table_header *sunrpc_table_header;
-static struct ctl_table sunrpc_table[];
-
-void
-rpc_register_sysctl(void)
-{
-	if (!sunrpc_table_header)
-		sunrpc_table_header = register_sysctl_table(sunrpc_table);
-}
-
-void
-rpc_unregister_sysctl(void)
-{
-	if (sunrpc_table_header) {
-		unregister_sysctl_table(sunrpc_table_header);
-		sunrpc_table_header = NULL;
-	}
-}
-
 static int proc_do_xprt(struct ctl_table *table, int write,
 			void *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -141,7 +122,9 @@ proc_dodebug(struct ctl_table *table, int write, void *buffer, size_t *lenp,
 	*ppos += *lenp;
 	return 0;
 }
+#endif
 
+static struct ctl_table_header *sunrpc_table_header;
 
 static struct ctl_table debug_table[] = {
 	{
@@ -190,4 +173,19 @@ static struct ctl_table sunrpc_table[] = {
 	{ }
 };
 
-#endif
+
+void
+rpc_register_sysctl(void)
+{
+	if (!sunrpc_table_header)
+		sunrpc_table_header = register_sysctl_table(sunrpc_table);
+}
+
+void
+rpc_unregister_sysctl(void)
+{
+	if (sunrpc_table_header) {
+		unregister_sysctl_table(sunrpc_table_header);
+		sunrpc_table_header = NULL;
+	}
+}
-- 
2.39.1

Re: [PATCH v2 4/5] sunrpc: move sunrpc_table and proc routines above

[kernel test robot]

Hi Luis,

I love your patch! Perhaps something to improve:

[auto build test WARNING on trondmy-nfs/linux-next]
[also build test WARNING on linus/master v6.3-rc1 next-20230310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Luis-Chamberlain/sunrpc-simplify-two-level-sysctl-registration-for-tsvcrdma_parm_table/20230312-045320
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
patch link:    https://lore.kernel.org/r/20230311205148.293375-5-mcgrof%40kernel.org
patch subject: [PATCH v2 4/5] sunrpc: move sunrpc_table and proc routines above
config: parisc-defconfig (https://download.01.org/0day-ci/archive/20230312/202303120646.T9Vnw7y8-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ab6eabde979082754f93fa8556379b39585a5d04
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Luis-Chamberlain/sunrpc-simplify-two-level-sysctl-registration-for-tsvcrdma_parm_table/20230312-045320
        git checkout ab6eabde979082754f93fa8556379b39585a5d04
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash net/sunrpc/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303120646.T9Vnw7y8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/sunrpc/sysctl.c:135:35: error: 'proc_dodebug' undeclared here (not in a function); did you mean 'rpc_debug'?
     135 |                 .proc_handler   = proc_dodebug
         |                                   ^~~~~~~~~~~~
         |                                   rpc_debug
   net/sunrpc/sysctl.c:162:35: error: 'proc_do_xprt' undeclared here (not in a function)
     162 |                 .proc_handler   = proc_do_xprt,
         |                                   ^~~~~~~~~~~~
>> net/sunrpc/sysctl.c:178:1: warning: no previous prototype for 'rpc_register_sysctl' [-Wmissing-prototypes]
     178 | rpc_register_sysctl(void)
         | ^~~~~~~~~~~~~~~~~~~
>> net/sunrpc/sysctl.c:185:1: warning: no previous prototype for 'rpc_unregister_sysctl' [-Wmissing-prototypes]
     185 | rpc_unregister_sysctl(void)
         | ^~~~~~~~~~~~~~~~~~~~~


vim +/rpc_register_sysctl +178 net/sunrpc/sysctl.c

   128	
   129	static struct ctl_table debug_table[] = {
   130		{
   131			.procname	= "rpc_debug",
   132			.data		= &rpc_debug,
   133			.maxlen		= sizeof(int),
   134			.mode		= 0644,
 > 135			.proc_handler	= proc_dodebug
   136		},
   137		{
   138			.procname	= "nfs_debug",
   139			.data		= &nfs_debug,
   140			.maxlen		= sizeof(int),
   141			.mode		= 0644,
   142			.proc_handler	= proc_dodebug
   143		},
   144		{
   145			.procname	= "nfsd_debug",
   146			.data		= &nfsd_debug,
   147			.maxlen		= sizeof(int),
   148			.mode		= 0644,
   149			.proc_handler	= proc_dodebug
   150		},
   151		{
   152			.procname	= "nlm_debug",
   153			.data		= &nlm_debug,
   154			.maxlen		= sizeof(int),
   155			.mode		= 0644,
   156			.proc_handler	= proc_dodebug
   157		},
   158		{
   159			.procname	= "transports",
   160			.maxlen		= 256,
   161			.mode		= 0444,
 > 162			.proc_handler	= proc_do_xprt,
   163		},
   164		{ }
   165	};
   166	
   167	static struct ctl_table sunrpc_table[] = {
   168		{
   169			.procname	= "sunrpc",
   170			.mode		= 0555,
   171			.child		= debug_table
   172		},
   173		{ }
   174	};
   175	
   176	
   177	void
 > 178	rpc_register_sysctl(void)
   179	{
   180		if (!sunrpc_table_header)
   181			sunrpc_table_header = register_sysctl_table(sunrpc_table);
   182	}
   183	
   184	void
 > 185	rpc_unregister_sysctl(void)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v2 5/5] sunrpc: simplify one-level sysctl registration for debug_table

[Luis Chamberlain]

There is no need to declare an extra tables to just create directory,
this can be easily be done with a prefix path with register_sysctl().

Simplify this registration.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 net/sunrpc/sysctl.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index a54438d68d1b..d51e6d9a2467 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -164,21 +164,11 @@ static struct ctl_table debug_table[] = {
 	{ }
 };
 
-static struct ctl_table sunrpc_table[] = {
-	{
-		.procname	= "sunrpc",
-		.mode		= 0555,
-		.child		= debug_table
-	},
-	{ }
-};
-
-
 void
 rpc_register_sysctl(void)
 {
 	if (!sunrpc_table_header)
-		sunrpc_table_header = register_sysctl_table(sunrpc_table);
+		sunrpc_table_header = register_sysctl("sunrpc", debug_table);
 }
 
 void
-- 
2.39.1