2021-03-22 05:31:28

by Eryu Guan

[permalink] [raw]
Subject: [PATCH 1/2] nfs: hornor timeo and retrans option when mounting NFSv3

Mounting NFSv3 uses default timeout parameters specified by underlying
sunrpc transport, and mount options like 'timeo' and 'retrans', unlike
NFSv4, are not honored.

But sometimes we want to set non-default timeout value when mounting
NFSv3, so pass 'timeo' and 'retrans' to nfs_mount() and fill the
'timeout' field of struct rpc_create_args before creating RPC
connection. This is also consistent with NFSv4 behavior.

Note that this only sets the timeout value of rpc connection to mountd,
but the timeout of rpcbind connection should be set as well. A later
patch will fix the rpcbind part.

Signed-off-by: Eryu Guan <[email protected]>
---
fs/nfs/internal.h | 2 +-
fs/nfs/mount_clnt.c | 12 +++++++-----
fs/nfs/super.c | 2 +-
3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 7b644d6c09e4..cf0d7db24d44 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -180,7 +180,7 @@ struct nfs_mount_request {
struct net *net;
};

-extern int nfs_mount(struct nfs_mount_request *info);
+extern int nfs_mount(struct nfs_mount_request *info, int timeo, int retrans);
extern void nfs_umount(const struct nfs_mount_request *info);

/* client.c */
diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
index dda5c3e65d8d..248319254672 100644
--- a/fs/nfs/mount_clnt.c
+++ b/fs/nfs/mount_clnt.c
@@ -137,13 +137,13 @@ struct mnt_fhstatus {
* nfs_mount - Obtain an NFS file handle for the given host and path
* @info: pointer to mount request arguments
*
- * Uses default timeout parameters specified by underlying transport. On
- * successful return, the auth_flavs list and auth_flav_len will be populated
- * with the list from the server or a faked-up list if the server didn't
- * provide one.
+ * Uses timeout parameters specified by caller. On successful return, the
+ * auth_flavs list and auth_flav_len will be populated with the list from the
+ * server or a faked-up list if the server didn't provide one.
*/
-int nfs_mount(struct nfs_mount_request *info)
+int nfs_mount(struct nfs_mount_request *info, int timeo, int retrans)
{
+ static struct rpc_timeout mnt_timeout;
struct mountres result = {
.fh = info->fh,
.auth_count = info->auth_flav_len,
@@ -158,6 +158,7 @@ int nfs_mount(struct nfs_mount_request *info)
.protocol = info->protocol,
.address = info->sap,
.addrsize = info->salen,
+ .timeout = &mnt_timeout,
.servername = info->hostname,
.program = &mnt_program,
.version = info->version,
@@ -177,6 +178,7 @@ int nfs_mount(struct nfs_mount_request *info)
if (info->noresvport)
args.flags |= RPC_CLNT_CREATE_NONPRIVPORT;

+ nfs_init_timeout_values(&mnt_timeout, info->protocol, timeo, retrans);
mnt_clnt = rpc_create(&args);
if (IS_ERR(mnt_clnt))
goto out_clnt_err;
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 94885c6f8f54..13a650750f04 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -867,7 +867,7 @@ static int nfs_request_mount(struct fs_context *fc,
* Now ask the mount server to map our export path
* to a file handle.
*/
- status = nfs_mount(&request);
+ status = nfs_mount(&request, ctx->timeo, ctx->retrans);
if (status != 0) {
dfprintk(MOUNT, "NFS: unable to mount server %s, error %d\n",
request.hostname, status);
--
2.26.1.107.gefe3874


2021-03-22 05:31:49

by Eryu Guan

[permalink] [raw]
Subject: [PATCH 2/2] sunrpc: honor rpc_task's timeout value in rpcb_create()

Currently rpcbind client is created without setting rpc timeout (thus
using the default value). But if the rpc_task already has a customized
timeout in its tk_client field, it's also ignored.

Let's use the same timeout setting in rpc_task->tk_client->cl_timeout
for rpcbind connection.

Signed-off-by: Eryu Guan <[email protected]>
---
net/sunrpc/rpcb_clnt.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 38fe2ce8a5aa..647b323cc1d5 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -344,13 +344,15 @@ static struct rpc_clnt *rpcb_create(struct net *net, const char *nodename,
const char *hostname,
struct sockaddr *srvaddr, size_t salen,
int proto, u32 version,
- const struct cred *cred)
+ const struct cred *cred,
+ const struct rpc_timeout *timeo)
{
struct rpc_create_args args = {
.net = net,
.protocol = proto,
.address = srvaddr,
.addrsize = salen,
+ .timeout = timeo,
.servername = hostname,
.nodename = nodename,
.program = &rpcb_program,
@@ -705,7 +707,8 @@ void rpcb_getport_async(struct rpc_task *task)
clnt->cl_nodename,
xprt->servername, sap, salen,
xprt->prot, bind_version,
- clnt->cl_cred);
+ clnt->cl_cred,
+ task->tk_client->cl_timeout);
if (IS_ERR(rpcb_clnt)) {
status = PTR_ERR(rpcb_clnt);
goto bailout_nofree;
--
2.26.1.107.gefe3874

2021-03-22 15:54:19

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs: hornor timeo and retrans option when mounting NFSv3

On Mon, 2021-03-22 at 13:29 +0800, Eryu Guan wrote:
> Mounting NFSv3 uses default timeout parameters specified by underlying
> sunrpc transport, and mount options like 'timeo' and 'retrans', unlike
> NFSv4, are not honored.
>
> But sometimes we want to set non-default timeout value when mounting
> NFSv3, so pass 'timeo' and 'retrans' to nfs_mount() and fill the
> 'timeout' field of struct rpc_create_args before creating RPC
> connection. This is also consistent with NFSv4 behavior.
>
> Note that this only sets the timeout value of rpc connection to mountd,
> but the timeout of rpcbind connection should be set as well. A later
> patch will fix the rpcbind part.
>
> Signed-off-by: Eryu Guan <[email protected]>
> ---
>  fs/nfs/internal.h   |  2 +-
>  fs/nfs/mount_clnt.c | 12 +++++++-----
>  fs/nfs/super.c      |  2 +-
>  3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 7b644d6c09e4..cf0d7db24d44 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -180,7 +180,7 @@ struct nfs_mount_request {
>         struct net              *net;
>  };
>  
> -extern int nfs_mount(struct nfs_mount_request *info);
> +extern int nfs_mount(struct nfs_mount_request *info, int timeo, int
> retrans);
>  extern void nfs_umount(const struct nfs_mount_request *info);
>  
>  /* client.c */
> diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> index dda5c3e65d8d..248319254672 100644
> --- a/fs/nfs/mount_clnt.c
> +++ b/fs/nfs/mount_clnt.c
> @@ -137,13 +137,13 @@ struct mnt_fhstatus {
>   * nfs_mount - Obtain an NFS file handle for the given host and path
>   * @info: pointer to mount request arguments
>   *
> - * Uses default timeout parameters specified by underlying transport.
> On
> - * successful return, the auth_flavs list and auth_flav_len will be
> populated
> - * with the list from the server or a faked-up list if the server
> didn't
> - * provide one.
> + * Uses timeout parameters specified by caller. On successful return,
> the
> + * auth_flavs list and auth_flav_len will be populated with the list
> from the
> + * server or a faked-up list if the server didn't provide one.
>   */
> -int nfs_mount(struct nfs_mount_request *info)
> +int nfs_mount(struct nfs_mount_request *info, int timeo, int retrans)


fs/nfs/mount_clnt.c:145: warning: Function parameter or member 'timeo'
not described in 'nfs_mount'
fs/nfs/mount_clnt.c:145: warning: Function parameter or member
'retrans' not described in 'nfs_mount'


--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-03-23 02:59:10

by Eryu Guan

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs: hornor timeo and retrans option when mounting NFSv3

On Mon, Mar 22, 2021 at 03:53:01PM +0000, Trond Myklebust wrote:
> On Mon, 2021-03-22 at 13:29 +0800, Eryu Guan wrote:
> > Mounting NFSv3 uses default timeout parameters specified by underlying
> > sunrpc transport, and mount options like 'timeo' and 'retrans', unlike
> > NFSv4, are not honored.
> >
> > But sometimes we want to set non-default timeout value when mounting
> > NFSv3, so pass 'timeo' and 'retrans' to nfs_mount() and fill the
> > 'timeout' field of struct rpc_create_args before creating RPC
> > connection. This is also consistent with NFSv4 behavior.
> >
> > Note that this only sets the timeout value of rpc connection to mountd,
> > but the timeout of rpcbind connection should be set as well. A later
> > patch will fix the rpcbind part.
> >
> > Signed-off-by: Eryu Guan <[email protected]>
> > ---
> >  fs/nfs/internal.h   |  2 +-
> >  fs/nfs/mount_clnt.c | 12 +++++++-----
> >  fs/nfs/super.c      |  2 +-
> >  3 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> > index 7b644d6c09e4..cf0d7db24d44 100644
> > --- a/fs/nfs/internal.h
> > +++ b/fs/nfs/internal.h
> > @@ -180,7 +180,7 @@ struct nfs_mount_request {
> >         struct net              *net;
> >  };
> >  
> > -extern int nfs_mount(struct nfs_mount_request *info);
> > +extern int nfs_mount(struct nfs_mount_request *info, int timeo, int
> > retrans);
> >  extern void nfs_umount(const struct nfs_mount_request *info);
> >  
> >  /* client.c */
> > diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> > index dda5c3e65d8d..248319254672 100644
> > --- a/fs/nfs/mount_clnt.c
> > +++ b/fs/nfs/mount_clnt.c
> > @@ -137,13 +137,13 @@ struct mnt_fhstatus {
> >   * nfs_mount - Obtain an NFS file handle for the given host and path
> >   * @info: pointer to mount request arguments
> >   *
> > - * Uses default timeout parameters specified by underlying transport.
> > On
> > - * successful return, the auth_flavs list and auth_flav_len will be
> > populated
> > - * with the list from the server or a faked-up list if the server
> > didn't
> > - * provide one.
> > + * Uses timeout parameters specified by caller. On successful return,
> > the
> > + * auth_flavs list and auth_flav_len will be populated with the list
> > from the
> > + * server or a faked-up list if the server didn't provide one.
> >   */
> > -int nfs_mount(struct nfs_mount_request *info)
> > +int nfs_mount(struct nfs_mount_request *info, int timeo, int retrans)
>
>
> fs/nfs/mount_clnt.c:145: warning: Function parameter or member 'timeo'
> not described in 'nfs_mount'
> fs/nfs/mount_clnt.c:145: warning: Function parameter or member
> 'retrans' not described in 'nfs_mount'

Fixed in v2, thanks!

Eryu

>
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> [email protected]
>
>