Skip to content

Commit

Permalink
Revert "nfsd4: a client's own opens needn't prevent delegations"
Browse files Browse the repository at this point in the history
commit 6ee65a7 upstream.

This reverts commit 94415b0.

That commit claimed to allow a client to get a read delegation when it
was the only writer.  Actually it allowed a client to get a read
delegation when *any* client has a write open!

The main problem is that it's depending on nfs4_clnt_odstate structures
that are actually only maintained for pnfs exports.

This causes clients to miss writes performed by other clients, even when
there have been intervening closes and opens, violating close-to-open
cache consistency.

We can do this a different way, but first we should just revert this.

I've added pynfs 4.1 test DELEG19 to test for this, as I should have
done originally!

Cc: stable@vger.kernel.org
Reported-by: Timo Rothenpieler <timo@rothenpieler.org>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
J. Bruce Fields authored and gregkh committed Mar 20, 2021
1 parent c9c48d3 commit f8d4030
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 43 deletions.
3 changes: 0 additions & 3 deletions fs/locks.c
Original file line number Diff line number Diff line change
Expand Up @@ -1808,9 +1808,6 @@ check_conflicting_open(struct file *filp, const long arg, int flags)

if (flags & FL_LAYOUT)
return 0;
if (flags & FL_DELEG)
/* We leave these checks to the caller. */
return 0;

if (arg == F_RDLCK)
return inode_is_open_for_write(inode) ? -EAGAIN : 0;
Expand Down
54 changes: 14 additions & 40 deletions fs/nfsd/nfs4state.c
Original file line number Diff line number Diff line change
Expand Up @@ -4945,32 +4945,6 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
return fl;
}

static int nfsd4_check_conflicting_opens(struct nfs4_client *clp,
struct nfs4_file *fp)
{
struct nfs4_clnt_odstate *co;
struct file *f = fp->fi_deleg_file->nf_file;
struct inode *ino = locks_inode(f);
int writes = atomic_read(&ino->i_writecount);

if (fp->fi_fds[O_WRONLY])
writes--;
if (fp->fi_fds[O_RDWR])
writes--;
WARN_ON_ONCE(writes < 0);
if (writes > 0)
return -EAGAIN;
spin_lock(&fp->fi_lock);
list_for_each_entry(co, &fp->fi_clnt_odstate, co_perfile) {
if (co->co_client != clp) {
spin_unlock(&fp->fi_lock);
return -EAGAIN;
}
}
spin_unlock(&fp->fi_lock);
return 0;
}

static struct nfs4_delegation *
nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
struct nfs4_file *fp, struct nfs4_clnt_odstate *odstate)
Expand All @@ -4990,12 +4964,9 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,

nf = find_readable_file(fp);
if (!nf) {
/*
* We probably could attempt another open and get a read
* delegation, but for now, don't bother until the
* client actually sends us one.
*/
return ERR_PTR(-EAGAIN);
/* We should always have a readable file here */
WARN_ON_ONCE(1);
return ERR_PTR(-EBADF);
}
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
Expand Down Expand Up @@ -5025,19 +4996,11 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
if (!fl)
goto out_clnt_odstate;

status = nfsd4_check_conflicting_opens(clp, fp);
if (status) {
locks_free_lock(fl);
goto out_clnt_odstate;
}
status = vfs_setlease(fp->fi_deleg_file->nf_file, fl->fl_type, &fl, NULL);
if (fl)
locks_free_lock(fl);
if (status)
goto out_clnt_odstate;
status = nfsd4_check_conflicting_opens(clp, fp);
if (status)
goto out_clnt_odstate;

spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
Expand Down Expand Up @@ -5119,6 +5082,17 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open,
goto out_no_deleg;
if (!cb_up || !(oo->oo_flags & NFS4_OO_CONFIRMED))
goto out_no_deleg;
/*
* Also, if the file was opened for write or
* create, there's a good chance the client's
* about to write to it, resulting in an
* immediate recall (since we don't support
* write delegations):
*/
if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
goto out_no_deleg;
if (open->op_create == NFS4_OPEN_CREATE)
goto out_no_deleg;
break;
default:
goto out_no_deleg;
Expand Down

0 comments on commit f8d4030

Please sign in to comment.