On Fri, 2022-06-17 at 16:23 -0400, Anna Schumaker wrote:
> From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
>
> Previously, we required this to value to be a power of 2 for UDP related
> reasons. This patch keeps the power of 2 rule for UDP but allows more
> flexibility for TCP and RDMA.
>
> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> ---
> fs/nfs/client.c | 13 +++++++------
> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 6 ++++--
> fs/nfs/internal.h | 18 ++++++++++++++++++
> fs/nfs/nfs4client.c | 4 ++--
> 4 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index e828504cc396..da8da5cdbbc1 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -708,9 +708,9 @@ static int nfs_init_server(struct nfs_server *server,
> }
>
> if (ctx->rsize)
> - server->rsize = nfs_block_size(ctx->rsize, NULL);
> + server->rsize = nfs_io_size(ctx->rsize, clp->cl_proto);
> if (ctx->wsize)
> - server->wsize = nfs_block_size(ctx->wsize, NULL);
> + server->wsize = nfs_io_size(ctx->wsize, clp->cl_proto);
>
> server->acregmin = ctx->acregmin * HZ;
> server->acregmax = ctx->acregmax * HZ;
> @@ -755,18 +755,19 @@ static int nfs_init_server(struct nfs_server *server,
> static void nfs_server_set_fsinfo(struct nfs_server *server,
> struct nfs_fsinfo *fsinfo)
> {
> + struct nfs_client *clp = server->nfs_client;
> unsigned long max_rpc_payload, raw_max_rpc_payload;
>
> /* Work out a lot of parameters */
> if (server->rsize == 0)
> - server->rsize = nfs_block_size(fsinfo->rtpref, NULL);
> + server->rsize = nfs_io_size(fsinfo->rtpref, clp->cl_proto);
> if (server->wsize == 0)
> - server->wsize = nfs_block_size(fsinfo->wtpref, NULL);
> + server->wsize = nfs_io_size(fsinfo->wtpref, clp->cl_proto);
>
> if (fsinfo->rtmax >= 512 && server->rsize > fsinfo->rtmax)
> - server->rsize = nfs_block_size(fsinfo->rtmax, NULL);
> + server->rsize = nfs_io_size(fsinfo->rtmax, clp->cl_proto);
> if (fsinfo->wtmax >= 512 && server->wsize > fsinfo->wtmax)
> - server->wsize = nfs_block_size(fsinfo->wtmax, NULL);
> + server->wsize = nfs_io_size(fsinfo->wtmax, clp->cl_proto);
>
> raw_max_rpc_payload = rpc_max_payload(server->client);
> max_rpc_payload = nfs_block_size(raw_max_rpc_payload, NULL);
> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> index bfa7202ca7be..e028f5a0ef5f 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> @@ -113,8 +113,10 @@ nfs4_ff_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
> goto out_err_drain_dsaddrs;
> ds_versions[i].version = be32_to_cpup(p++);
> ds_versions[i].minor_version = be32_to_cpup(p++);
> - ds_versions[i].rsize = nfs_block_size(be32_to_cpup(p++), NULL);
> - ds_versions[i].wsize = nfs_block_size(be32_to_cpup(p++), NULL);
> + ds_versions[i].rsize = nfs_io_size(be32_to_cpup(p++),
> + server->nfs_client->cl_proto);
> + ds_versions[i].wsize = nfs_io_size(be32_to_cpup(p++),
> + server->nfs_client->cl_proto);
> ds_versions[i].tightly_coupled = be32_to_cpup(p);
>
> if (ds_versions[i].rsize > NFS_MAX_FILE_IO_SIZE)
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 8f8cd6e2d4db..af6d261241ff 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -704,6 +704,24 @@ unsigned long nfs_block_size(unsigned long bsize, unsigned char *nrbitsp)
> return nfs_block_bits(bsize, nrbitsp);
> }
>
> +/*
> + * Compute and set NFS server rsize / wsize
> + */
> +static inline
> +unsigned long nfs_io_size(unsigned long iosize, enum xprt_transports proto)
> +{
> + if (iosize < NFS_MIN_FILE_IO_SIZE)
> + iosize = NFS_DEF_FILE_IO_SIZE;
> + else if (iosize >= NFS_MAX_FILE_IO_SIZE)
> + iosize = NFS_MAX_FILE_IO_SIZE;
> + else
> + iosize = iosize & PAGE_MASK;
> +
> + if (proto == XPRT_TRANSPORT_UDP)
> + return nfs_block_bits(iosize, NULL);
> + return iosize;
> +}
> +
> /*
> * Determine the maximum file size for a superblock
> */
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 47a6cf892c95..3c5678aec006 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -1161,9 +1161,9 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
> return error;
>
> if (ctx->rsize)
> - server->rsize = nfs_block_size(ctx->rsize, NULL);
> + server->rsize = nfs_io_size(ctx->rsize, server->nfs_client->cl_proto);
> if (ctx->wsize)
> - server->wsize = nfs_block_size(ctx->wsize, NULL);
> + server->wsize = nfs_io_size(ctx->wsize, server->nfs_client->cl_proto);
>
> server->acregmin = ctx->acregmin * HZ;
> server->acregmax = ctx->acregmax * HZ;
This patch seems to have caused a regression. With this patch in place,
I can't set an rsize/wsize value that is less than 4k:
# mount server:/export /mnt -o rsize=1024,wsize=1024
...now yields:
server:/export on /mnt type nfs4 (rw,relatime,vers=4.2,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.1.210,local_lock=none,addr=192.168.1.3)
...such that the requested sizes were ignored.
Was this an intended effect? If we really do want to deprecate the use
of small rsize/wsize with TCP/RDMA, then we probably ought to reject
these mount attempts with -EINVAL.
--
Jeff Layton <jlayton@xxxxxxxxxx>
