RE: Trying to reduce NFSv4 timeouts to a few seconds on an established connection
From: Andrew Klaassen <hidden>
Date: 2023-02-06 17:19:02
Subsystem:
kernel nfsd, sunrpc, and lockd servers, networking [general], nfs, sunrpc, and lockd clients, the rest · Maintainers:
Chuck Lever, Jeff Layton, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Trond Myklebust, Anna Schumaker, Linus Torvalds
From: Andrew Klaassen <redacted> Sent: Monday, February 6, 2023 10:28 AM
[snipping for readability; hope that's okay] - I'm allocating memory. I assume that means I should free it somewhere. But where? In xprt_destroy(), which appears to do cleanup? Or in xprt_destroy_cb(), which is called from xprt_destroy() and which frees xprt-quoted
servername? Or somewhere else completely?- If I free the allocated memory, will that cause any problems in the cases where no timeout is passed in via the args and the static const struct xs_tcp_default_timeout is assigned to xprt->timeout? - If freeing the static const struct default will cause a problem, what should I do instead? Allocate and memcpy even when assigning the default? And would that mean doing the same thing for all the other transports that are setting timeouts (local, udp, tcp, and bc_tcp)?
Here's my best guess as the answer to my questions. Any advice/feedback appreciated.
diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index b9f59aabee53..4543ec07cc12 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h@@ -333,6 +333,7 @@ struct xprt_create { struct svc_xprt *bc_xprt; /* NFSv4.1 backchannel */ struct rpc_xprt_switch *bc_xps; unsigned int flags; + const struct rpc_timeout *timeout; /* timeout parms */ }; struct xprt_class {
@@ -373,6 +374,8 @@ void xprt_release_xprt_cong(struct rpc_xprt *xprt, struct rpc_task *task); void xprt_release(struct rpc_task *task); struct rpc_xprt * xprt_get(struct rpc_xprt *xprt); void xprt_put(struct rpc_xprt *xprt); +struct rpc_timeout * xprt_alloc_timeout(const struct rpc_timeout * timeo, + const struct rpc_timeout *default_timeo); struct rpc_xprt * xprt_alloc(struct net *net, size_t size, unsigned int num_prealloc, unsigned int max_req);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 0b0b9f1eed46..1350c1f489f7 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c@@ -532,6 +532,7 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args) .addrlen = args->addrsize, .servername = args->servername, .bc_xprt = args->bc_xprt, + .timeout = args->timeout, }; char servername[48]; struct rpc_clnt *clnt;
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index ab453ede54f0..1065b76ddff4 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c@@ -1801,6 +1801,26 @@ static void xprt_free_id(struct rpc_xprt *xprt) ida_free(&rpc_xprt_ids, xprt->id); } +struct rpc_timeout *xprt_alloc_timeout(const struct rpc_timeout *timeo, + const struct rpc_timeout *default_timeo) +{ + struct rpc_timeout *timeout; + timeout = kzalloc(sizeof(struct rpc_timeout), GFP_KERNEL); + if (timeout == NULL) + return ERR_PTR(-ENOMEM); + if (timeo) + memcpy(timeout, timeo, sizeof(struct rpc_timeout)); + else + memcpy(timeout, default_timeo, sizeof(struct rpc_timeout)); + return timeout; +} + +static void xprt_free_timeout(struct rpc_xprt *xprt) +{ + if (xprt->timeout != NULL) + kfree(xprt->timeout); +} + struct rpc_xprt *xprt_alloc(struct net *net, size_t size, unsigned int num_prealloc, unsigned int max_alloc)
@@ -1837,6 +1857,7 @@ EXPORT_SYMBOL_GPL(xprt_alloc); void xprt_free(struct rpc_xprt *xprt) { + xprt_free_timeout(xprt); put_net_track(xprt->xprt_net, &xprt->ns_tracker); xprt_free_all_slots(xprt); xprt_free_id(xprt);
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index aaa5b2741b79..ba05258509fa 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c@@ -3003,7 +3003,11 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args) xprt->idle_timeout = XS_IDLE_DISC_TO; xprt->ops = &xs_tcp_ops; - xprt->timeout = &xs_tcp_default_timeout; + + xprt->timeout = xprt_alloc_timeout(args->timeout, &xs_tcp_default_timeout); + + if (IS_ERR(xprt->timeout)) + goto out_err; xprt->max_reconnect_timeout = xprt->timeout->to_maxval; xprt->connect_timeout = xprt->timeout->to_initval *