Skip to content

Commit 6ee65a7

Browse files
J. Bruce Fieldschucklever
authored andcommitted
Revert "nfsd4: a client's own opens needn't prevent delegations"
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>
1 parent 4aa5e00 commit 6ee65a7

2 files changed

Lines changed: 14 additions & 43 deletions

File tree

fs/locks.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1808,9 +1808,6 @@ check_conflicting_open(struct file *filp, const long arg, int flags)
18081808

18091809
if (flags & FL_LAYOUT)
18101810
return 0;
1811-
if (flags & FL_DELEG)
1812-
/* We leave these checks to the caller. */
1813-
return 0;
18141811

18151812
if (arg == F_RDLCK)
18161813
return inode_is_open_for_write(inode) ? -EAGAIN : 0;

fs/nfsd/nfs4state.c

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4940,32 +4940,6 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
49404940
return fl;
49414941
}
49424942

4943-
static int nfsd4_check_conflicting_opens(struct nfs4_client *clp,
4944-
struct nfs4_file *fp)
4945-
{
4946-
struct nfs4_clnt_odstate *co;
4947-
struct file *f = fp->fi_deleg_file->nf_file;
4948-
struct inode *ino = locks_inode(f);
4949-
int writes = atomic_read(&ino->i_writecount);
4950-
4951-
if (fp->fi_fds[O_WRONLY])
4952-
writes--;
4953-
if (fp->fi_fds[O_RDWR])
4954-
writes--;
4955-
WARN_ON_ONCE(writes < 0);
4956-
if (writes > 0)
4957-
return -EAGAIN;
4958-
spin_lock(&fp->fi_lock);
4959-
list_for_each_entry(co, &fp->fi_clnt_odstate, co_perfile) {
4960-
if (co->co_client != clp) {
4961-
spin_unlock(&fp->fi_lock);
4962-
return -EAGAIN;
4963-
}
4964-
}
4965-
spin_unlock(&fp->fi_lock);
4966-
return 0;
4967-
}
4968-
49694943
static struct nfs4_delegation *
49704944
nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
49714945
struct nfs4_file *fp, struct nfs4_clnt_odstate *odstate)
@@ -4985,12 +4959,9 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
49854959

49864960
nf = find_readable_file(fp);
49874961
if (!nf) {
4988-
/*
4989-
* We probably could attempt another open and get a read
4990-
* delegation, but for now, don't bother until the
4991-
* client actually sends us one.
4992-
*/
4993-
return ERR_PTR(-EAGAIN);
4962+
/* We should always have a readable file here */
4963+
WARN_ON_ONCE(1);
4964+
return ERR_PTR(-EBADF);
49944965
}
49954966
spin_lock(&state_lock);
49964967
spin_lock(&fp->fi_lock);
@@ -5020,19 +4991,11 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
50204991
if (!fl)
50214992
goto out_clnt_odstate;
50224993

5023-
status = nfsd4_check_conflicting_opens(clp, fp);
5024-
if (status) {
5025-
locks_free_lock(fl);
5026-
goto out_clnt_odstate;
5027-
}
50284994
status = vfs_setlease(fp->fi_deleg_file->nf_file, fl->fl_type, &fl, NULL);
50294995
if (fl)
50304996
locks_free_lock(fl);
50314997
if (status)
50324998
goto out_clnt_odstate;
5033-
status = nfsd4_check_conflicting_opens(clp, fp);
5034-
if (status)
5035-
goto out_clnt_odstate;
50364999

50375000
spin_lock(&state_lock);
50385001
spin_lock(&fp->fi_lock);
@@ -5114,6 +5077,17 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open,
51145077
goto out_no_deleg;
51155078
if (!cb_up || !(oo->oo_flags & NFS4_OO_CONFIRMED))
51165079
goto out_no_deleg;
5080+
/*
5081+
* Also, if the file was opened for write or
5082+
* create, there's a good chance the client's
5083+
* about to write to it, resulting in an
5084+
* immediate recall (since we don't support
5085+
* write delegations):
5086+
*/
5087+
if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
5088+
goto out_no_deleg;
5089+
if (open->op_create == NFS4_OPEN_CREATE)
5090+
goto out_no_deleg;
51175091
break;
51185092
default:
51195093
goto out_no_deleg;

0 commit comments

Comments
 (0)