> On Mar 28, 2016, at 5:19 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
> At Connectathon 2016, we found that recent upstream Linux clients
> would occasionally send a LOCK operation with a zero stateid. This
> appeared to happen in close proximity to another thread returning
> a delegation before unlinking the same file while it remained open.
>
> Earlier, the client received a write delegation on this file and
> returned the open stateid. Now, as it is getting ready to unlink the
> file, it returns the write delegation. But there is still an open
> file descriptor on that file, so the client must OPEN the file
> again before it returns the delegation.
>
> To eliminate this race, serialize delegation return with the
> acquisition of a file lock on the same file. Adopt the same approach
> as is used in the unlock path.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
I see two new behaviors when this patch is applied:
1. With NFSv4.0, the cthon04 lock test #7 hangs after
"unlock region so child will unblock". ^C then unmount
works as expected.
2. With NFSv4.0, xfstests generic/089 fails because one
of the three test processes is starved out of locking
the test file (t_mtab). The test fails, there are no
other hangs or error messages.
I'm starting to look into the first issue.
> fs/nfs/nfs4proc.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 01bef06..dd435f4 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6054,6 +6054,7 @@ static int nfs41_lock_expired(struct nfs4_state *state, struct file_lock *reques
> static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
> {
> struct nfs_inode *nfsi = NFS_I(state->inode);
> + struct nfs4_state_owner *sp = state->owner;
> unsigned char fl_flags = request->fl_flags;
> int status = -ENOLCK;
>
> @@ -6065,9 +6066,12 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
> if (status != 0)
> goto out;
> request->fl_flags |= FL_ACCESS;
> + mutex_lock(&sp->so_delegreturn_mutex);
> status = do_vfs_lock(state->inode, request);
> - if (status < 0)
> + if (status < 0) {
> + mutex_unlock(&sp->so_delegreturn_mutex);
> goto out;
> + }
> down_read(&nfsi->rwsem);
> if (test_bit(NFS_DELEGATED_STATE, &state->flags)) {
> /* Yes: cache locks! */
> @@ -6075,9 +6079,11 @@ static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
> request->fl_flags = fl_flags & ~FL_SLEEP;
> status = do_vfs_lock(state->inode, request);
> up_read(&nfsi->rwsem);
> + mutex_unlock(&sp->so_delegreturn_mutex);
> goto out;
> }
> up_read(&nfsi->rwsem);
> + mutex_unlock(&sp->so_delegreturn_mutex);
> status = _nfs4_do_setlk(state, cmd, request, NFS_LOCK_NEW);
> out:
> request->fl_flags = fl_flags;
>
--
Chuck Lever
--
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
