nfsd: grant read delegations to clients holding writes

It's OK to grant a read delegation to a client that holds a write,
as long as it's the only client holding the write.

We originally tried to do this in commit 94415b06eb ("nfsd4: a
client's own opens needn't prevent delegations"), which had to be
reverted in commit 6ee65a7730 ("Revert "nfsd4: a client's own
opens needn't prevent delegations"").

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
This commit is contained in:
J. Bruce Fields 2021-04-16 14:00:18 -04:00 committed by Chuck Lever
parent ebd9d2c2f5
commit aba2072f45
2 changed files with 71 additions and 14 deletions

View File

@ -1808,6 +1808,9 @@ 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;

View File

@ -4988,6 +4988,65 @@ 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_ol_stateid *st;
struct file *f = fp->fi_deleg_file->nf_file;
struct inode *ino = locks_inode(f);
int writes;
writes = atomic_read(&ino->i_writecount);
if (!writes)
return 0;
/*
* There could be multiple filehandles (hence multiple
* nfs4_files) referencing this file, but that's not too
* common; let's just give up in that case rather than
* trying to go look up all the clients using that other
* nfs4_file as well:
*/
if (fp->fi_aliased)
return -EAGAIN;
/*
* If there's a close in progress, make sure that we see it
* clear any fi_fds[] entries before we see it decrement
* i_writecount:
*/
smp_mb__after_atomic();
if (fp->fi_fds[O_WRONLY])
writes--;
if (fp->fi_fds[O_RDWR])
writes--;
if (writes > 0)
return -EAGAIN; /* There may be non-NFSv4 writers */
/*
* It's possible there are non-NFSv4 write opens in progress,
* but if they haven't incremented i_writecount yet then they
* also haven't called break lease yet; so, they'll break this
* lease soon enough. So, all that's left to check for is NFSv4
* opens:
*/
spin_lock(&fp->fi_lock);
list_for_each_entry(st, &fp->fi_stateids, st_perfile) {
if (st->st_openstp == NULL /* it's an open */ &&
access_permit_write(st) &&
st->st_stid.sc_client != clp) {
spin_unlock(&fp->fi_lock);
return -EAGAIN;
}
}
spin_unlock(&fp->fi_lock);
/*
* There's a small chance that we could be racing with another
* NFSv4 open. However, any open that hasn't added itself to
* the fi_stateids list also hasn't called break_lease yet; so,
* they'll break this lease soon enough.
*/
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)
@ -5007,9 +5066,12 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
nf = find_readable_file(fp);
if (!nf) {
/* We should always have a readable file here */
WARN_ON_ONCE(1);
return ERR_PTR(-EBADF);
/*
* 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);
}
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
@ -5044,6 +5106,9 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
locks_free_lock(fl);
if (status)
goto out_clnt_odstate;
status = nfsd4_check_conflicting_opens(clp, fp);
if (status)
goto out_unlock;
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
@ -5125,17 +5190,6 @@ 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;