2009-09-11 09:33:57

by James Pearson

[permalink] [raw]
Subject: Re: [PATCH] NFS: Revert default r/wsize behavior

Chuck Lever wrote:
> When the "rsize=" or "wsize=" mount options are not specified,
> text-based mounts have slightly different behavior than legacy binary
> mounts. Text-based mounts use the smaller of the server's maximum
> and the client's maximum, but binary mounts use the smaller of the
> server's _preferred_ size and the client's maximum.
>
> This difference is actually pretty subtle. Most servers advertise
> the same value as their largest and their preferred transfer size, so
> the end result is the same in most cases.
>
> The reason for this difference is that for text-based mounts, if
> r/wsize are not specified, they are set to the largest value supported
> by the client. For binary mounts, the values are set to zero if these
> options are not specified.
>
> nfs_server_set_fsinfo() can negotiate the defaults correctly in any
> case. There's no need to specify any particular value as default in
> the text-based option parsing logic.
>
> Thanks to James Pearson <[email protected]> for reporting and
> diagnosing the problem.
>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>
> James-
>
> Is this what you want?
>
> fs/nfs/super.c | 4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index bde444b..cf0d754 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1635,8 +1635,6 @@ static int nfs_validate_mount_data(void *options,
> goto out_no_data;
>
> args->flags = (NFS_MOUNT_VER3 | NFS_MOUNT_TCP);
> - args->rsize = NFS_MAX_FILE_IO_SIZE;
> - args->wsize = NFS_MAX_FILE_IO_SIZE;
> args->acregmin = NFS_DEF_ACREGMIN;
> args->acregmax = NFS_DEF_ACREGMAX;
> args->acdirmin = NFS_DEF_ACDIRMIN;
> @@ -2351,8 +2349,6 @@ static int nfs4_validate_mount_data(void *options,
> if (data == NULL)
> goto out_no_data;
>
> - args->rsize = NFS_MAX_FILE_IO_SIZE;
> - args->wsize = NFS_MAX_FILE_IO_SIZE;
> args->acregmin = NFS_DEF_ACREGMIN;
> args->acregmax = NFS_DEF_ACREGMAX;
> args->acdirmin = NFS_DEF_ACDIRMIN;

That is what I've already done myself for the NFSv3 case - personally I
would implicitly set args->[rw]size to zero (although args would have
been initialled with kzalloc()), just to make it clear that the defaults
are zero.

Thanks

James Pearson