On Wed, Dec 13 2017, NeilBrown wrote:
> There are 2 comments in the NFSv4 code which suggest that
> SIGLOST should possibly be sent to a process. In these
> cases a lock has been lost.
> The current practice is to set NFS_LOCK_LOST so that
> read/write returns EIO when a lock is lost.
> So change these comments to code when sets NFS_LOCK_LOST.
>
> One case is when lock recovery after apparent server restart
> fails with NFS4ERR_DENIED, NFS4ERR_RECLAIM_BAD, or
> NFS4ERRO_RECLAIM_CONFLICT. The other case is when a lock
> attempt as part of lease recovery fails with NFS4ERR_DENIED.
>
> In an ideal world, these should not happen. However I have
> a packet trace showing an NFSv4.1 session getting
> NFS4ERR_BADSESSION after an extended network parition. The
> NFSv4.1 client treats this like server reboot until/unless
> it get NFS4ERR_NO_GRACE, in which case it switches over to
> "nograce" recovery mode. In this network trace, the client
> attempts to recover a lock and the server (incorrectly)
> reports NFS4ERR_DENIED rather than NFS4ERR_NO_GRACE. This
> leads to the ineffective comment and the client then
> continues to write using the OPEN stateid.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxxx>
> ---
>
> Note that I have not tested this as I do not have direct access to a
> faulty NFS server. Once I get confirmation I will provide an update.
Hi,
I have now received confirmation that this change does fix the locking
behavior in this case where the server is returning the wrong error
code.
Thanks,
NeilBrown
>
> NeilBrown
>
> fs/nfs/nfs4proc.c | 12 ++++++++----
> fs/nfs/nfs4state.c | 5 ++++-
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 56fa5a16e097..083802f7a1e9 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2019,7 +2019,7 @@ static int nfs4_open_reclaim(struct nfs4_state_owner *sp, struct nfs4_state *sta
> return ret;
> }
>
> -static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct nfs4_state *state, const nfs4_stateid *stateid, int err)
> +static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct nfs4_state *state, const nfs4_stateid *stateid, struct file_lock *fl, int err)
> {
> switch (err) {
> default:
> @@ -2066,7 +2066,11 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
> return -EAGAIN;
> case -ENOMEM:
> case -NFS4ERR_DENIED:
> - /* kill_proc(fl->fl_pid, SIGLOST, 1); */
> + if (fl) {
> + struct nfs4_lock_state *lsp = fl->fl_u.nfs4_fl.owner;
> + if (lsp)
> + set_bit(NFS_LOCK_LOST, &lsp->ls_flags);
> + }
> return 0;
> }
> return err;
> @@ -2102,7 +2106,7 @@ int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
> err = nfs4_open_recover_helper(opendata, FMODE_READ);
> }
> nfs4_opendata_put(opendata);
> - return nfs4_handle_delegation_recall_error(server, state, stateid, err);
> + return nfs4_handle_delegation_recall_error(server, state, stateid, NULL, err);
> }
>
> static void nfs4_open_confirm_prepare(struct rpc_task *task, void *calldata)
> @@ -6739,7 +6743,7 @@ int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state,
> if (err != 0)
> return err;
> err = _nfs4_do_setlk(state, F_SETLK, fl, NFS_LOCK_NEW);
> - return nfs4_handle_delegation_recall_error(server, state, stateid, err);
> + return nfs4_handle_delegation_recall_error(server, state, stateid, fl, err);
> }
>
> struct nfs_release_lockowner_data {
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index e4f4a09ed9f4..91a4d4eeb235 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1482,6 +1482,7 @@ static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_
> struct inode *inode = state->inode;
> struct nfs_inode *nfsi = NFS_I(inode);
> struct file_lock *fl;
> + struct nfs4_lock_state *lsp;
> int status = 0;
> struct file_lock_context *flctx = inode->i_flctx;
> struct list_head *list;
> @@ -1522,7 +1523,9 @@ static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_
> case -NFS4ERR_DENIED:
> case -NFS4ERR_RECLAIM_BAD:
> case -NFS4ERR_RECLAIM_CONFLICT:
> - /* kill_proc(fl->fl_pid, SIGLOST, 1); */
> + lsp = fl->fl_u.nfs4_fl.owner;
> + if (lsp)
> + set_bit(NFS_LOCK_LOST, &lsp->ls_flags);
> status = 0;
> }
> spin_lock(&flctx->flc_lock);
> --
> 2.14.0.rc0.dirty
Attachment:
signature.asc
Description: PGP signature
