2023-03-10 22:52:52

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH 0/5] sunrpc: simplfy sysctl registrations

We're trying to deprecate the APIs for sysctl that try to do
registration and but are exposed to recursion. Those paths are
only needed in complex cases and even those can be simplified
with time.

Sunrpc uses has simple requirements: just to have their parent
directory created. The new sysctl APIs can be used for this. If
you are curious about new requirements just review the new patch
for documentation I just posted:

https://lore.kernel.org/all/[email protected]/T/#u

So just simplify all this.

I haven't even build tested this, this is all being compile tested
now through sysctl-testing [0] along with the other rest of the
changes.

Posting this early in the development cycle so it gets proper testing
and review.

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.

For those curious -- yes most of this is based on Coccinelle grammar,
you can see the SmPL patch I first wrote years ago for some other use
cases here:

https://lore.kernel.org/all/[email protected]/

This is just an evolution of that with more complex cases, however
always writing the SmPL patch to each commit eats my time away and
I really cannot be bothered by that. Small modifications to the above
can be used however to do things which are similar.

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 above
sunrpc: simplify one-level sysctl registration for debug_table

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

--
2.39.1



2023-03-10 22:52:52

by Luis Chamberlain

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

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 <[email protected]>
---
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


2023-03-10 22:52:55

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH 5/5] sunrpc: simplify one-level sysctl registration for debug_table

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 <[email protected]>
---
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 4120797bf740..8000828b139f 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -80,21 +80,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


2023-03-10 22:52:58

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH 4/5] sunrpc: move sunrpc_table above

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 <[email protected]>
---
net/sunrpc/sysctl.c | 98 ++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 50 deletions(-)

diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index 3aad6ef18504..4120797bf740 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -41,7 +41,54 @@ 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[];
+
+static struct ctl_table debug_table[] = {
+ {
+ .procname = "rpc_debug",
+ .data = &rpc_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dodebug
+ },
+ {
+ .procname = "nfs_debug",
+ .data = &nfs_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dodebug
+ },
+ {
+ .procname = "nfsd_debug",
+ .data = &nfsd_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dodebug
+ },
+ {
+ .procname = "nlm_debug",
+ .data = &nlm_debug,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dodebug
+ },
+ {
+ .procname = "transports",
+ .maxlen = 256,
+ .mode = 0444,
+ .proc_handler = proc_do_xprt,
+ },
+ { }
+};
+
+static struct ctl_table sunrpc_table[] = {
+ {
+ .procname = "sunrpc",
+ .mode = 0555,
+ .child = debug_table
+ },
+ { }
+};
+

void
rpc_register_sysctl(void)
@@ -141,53 +188,4 @@ proc_dodebug(struct ctl_table *table, int write, void *buffer, size_t *lenp,
*ppos += *lenp;
return 0;
}
-
-
-static struct ctl_table debug_table[] = {
- {
- .procname = "rpc_debug",
- .data = &rpc_debug,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dodebug
- },
- {
- .procname = "nfs_debug",
- .data = &nfs_debug,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dodebug
- },
- {
- .procname = "nfsd_debug",
- .data = &nfsd_debug,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dodebug
- },
- {
- .procname = "nlm_debug",
- .data = &nlm_debug,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dodebug
- },
- {
- .procname = "transports",
- .maxlen = 256,
- .mode = 0444,
- .proc_handler = proc_do_xprt,
- },
- { }
-};
-
-static struct ctl_table sunrpc_table[] = {
- {
- .procname = "sunrpc",
- .mode = 0555,
- .child = debug_table
- },
- { }
-};
-
#endif
--
2.39.1


2023-03-10 22:53:00

by Luis Chamberlain

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

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 <[email protected]>
---
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


2023-03-10 22:53:01

by Luis Chamberlain

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

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 <[email protected]>
---
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