iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

nfsd4: a client's own opens needn't prevent delegations

Browse Source 
We recently fixed lease breaking so that a client's actions won't break
its own delegations.

But we still have an unnecessary self-conflict when granting
delegations: a client's own write opens will prevent us from handing out
a read delegation even when no other client has the file open for write.

Fix that by turning off the checks for conflicting opens under
vfs_setlease, and instead performing those checks in the nfsd code.

We don't depend much on locks here: instead we acquire the delegation,
then check for conflicts, and drop the delegation again if we find any.

The check beforehand is an optimization of sorts, just to avoid
acquiring the delegation unnecessarily.  There's a race where the first
check could cause us to deny the delegation when we could have granted
it.  But, that's OK, delegation grants are optional (and probably not
even a good idea in that case).

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 2020-07-07 09:28:05 -04:00 committed by Chuck Lever
parent 0b7cd9d9ca
commit 94415b06eb
2 changed files with 43 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
fs/locks.c
View File

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

54
fs/nfsd/nfs4state.c
View File

@ -4922,6 +4922,32 @@ 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)
@ -4941,9 +4967,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);
@ -4973,11 +5002,19 @@ 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);
@ -5059,17 +5096,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;