On Wed, 2017-11-08 at 14:30 +1100, NeilBrown wrote:
> What to people think of the following as an approach
> to Joshua's need?
>
> It isn't complete by itself: it needs a couple of changes to
> nfs-utils so that it doesn't stat the mountpoint on remount,
> and it might need another kernel change so that the "mount" system
> call performs the same sort of careful lookup for remount as the umount
> system call does, but those are relatively small details.
>
Yeah, that'd be good.
> This is the patch that you will either love of hate.
>
> With this patch, Joshua (or any other sysadmin) could:
>
> mount -o remount,retrans=0,timeo=1 /path
>
> and then new requests on any mountpoint from that server will timeout
> quickly.
> Then
> umount -f /path
> umount -f /path
>
> should kill off any existing requests that use the old timeout. (I just
> tested and I did need this twice to kill of an "ls -l".
> The first was an 'open' systemcall, then next as 'newfstat'. I wonder
> why the getattr for fstat didn't use the new timeout...)
>
> Thoughts?
> NeilBrown
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index c9d24bae3025..ced12fcec349 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2210,27 +2210,39 @@ static int nfs_validate_text_mount_data(void *options,
> ~(NFS_MOUNT_UNSHARED | NFS_MOUNT_NORESVPORT))
>
> static int
> -nfs_compare_remount_data(struct nfs_server *nfss,
> - struct nfs_parsed_mount_data *data)
> +nfs_compare_and_set_remount_data(struct nfs_server *nfss,
> + struct nfs_parsed_mount_data *data)
> {
> if ((data->flags ^ nfss->flags) & NFS_REMOUNT_CMP_FLAGMASK ||
> data->rsize != nfss->rsize ||
> data->wsize != nfss->wsize ||
> data->version != nfss->nfs_client->rpc_ops->version ||
> data->minorversion != nfss->nfs_client->cl_minorversion ||
> - data->retrans != nfss->client->cl_timeout->to_retries ||
> !nfs_auth_info_match(&data->auth_info, nfss->client->cl_auth->au_flavor) ||
> data->acregmin != nfss->acregmin / HZ ||
> data->acregmax != nfss->acregmax / HZ ||
> data->acdirmin != nfss->acdirmin / HZ ||
> data->acdirmax != nfss->acdirmax / HZ ||
> - data->timeo != (10U * nfss->client->cl_timeout->to_initval / HZ) ||
> data->nfs_server.port != nfss->port ||
> data->nfs_server.addrlen != nfss->nfs_client->cl_addrlen ||
> !rpc_cmp_addr((struct sockaddr *)&data->nfs_server.address,
> (struct sockaddr *)&nfss->nfs_client->cl_addr))
> return -EINVAL;
>
> + if (data->retrans != nfss->client->cl_timeout->to_retries ||
> + data->timeo != (10U * nfss->client->cl_timeout->to_initval / HZ)) {
> + /* Note that this will affect all mounts from the same server,
> + * that use the same protocol. The timeouts are always forced
> + * to be the same.
> + */
> + struct rpc_clnt *cl = nfss->client;
> + if (cl->cl_timeout != &cl->cl_timeout_default)
> + memcpy(&cl->cl_timeout_default, cl->cl_timeout,
> + sizeof(struct rpc_timeout));
> + cl->cl_timeout_default.to_retries = data->retrans;
> + cl->cl_timeout_default.to_initval = data->timeo * HZ / 10U;
> + }
> +
No objection to allowing more mount options to be changed on remount, we
really ought to allow a lot more options to be changed on remount if we
can reasonably pull it off.
> return 0;
> }
>
> @@ -2244,7 +2256,8 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
> struct nfs4_mount_data *options4 = (struct nfs4_mount_data *)raw_data;
> u32 nfsvers = nfss->nfs_client->rpc_ops->version;
>
> - sync_filesystem(sb);
> + if (sb->s_readonly_remount)
> + sync_filesystem(sb);
>
> /*
> * Userspace mount programs that send binary options generally send
> @@ -2295,7 +2308,7 @@ nfs_remount(struct super_block *sb, int *flags, char *raw_data)
> *flags |= MS_SYNCHRONOUS;
>
> /* compare new mount options with old ones */
> - error = nfs_compare_remount_data(nfss, data);
> + error = nfs_compare_and_set_remount_data(nfss, data);
> out:
> kfree(data);
> return error;
Looks like a reasonable approach overall to preventing new RPCs from
being dispatched once the "force" umount runs.
I do wonder if this ought to be more automatic when you specify -f on
the umount. Having to manually do a remount first doesn't seem very
admin-friendly.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
