diff mbox series

[1/5] nfsd: ensure new clients break delegations

Message ID 1618596018-9899-1-git-send-email-bfields@redhat.com (mailing list archive)
State New
Headers show
Series [1/5] nfsd: ensure new clients break delegations | expand

Commit Message

J. Bruce Fields April 16, 2021, 6 p.m. UTC
From: "J. Bruce Fields" <bfields@redhat.com>

If nfsd already has an open file that it plans to use for IO from
another, it may not need to do another vfs open, but it still may need
to break any delegations in case the existing opens are for another
client.

Symptoms are that we may incorrectly fail to break a delegation on a
write open from a different client, when the delegation-holding client
already has a write open.

Fixes: 28df3d1539de ("nfsd: clients don't need to break their own delegations")
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Comments

Chuck Lever III April 16, 2021, 7:11 p.m. UTC | #1
> On Apr 16, 2021, at 2:00 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> 
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> If nfsd already has an open file that it plans to use for IO from
> another,

from another... client?


> it may not need to do another vfs open, but it still may need
> to break any delegations in case the existing opens are for another
> client.
> 
> Symptoms are that we may incorrectly fail to break a delegation on a
> write open from a different client, when the delegation-holding client
> already has a write open.
> 
> Fixes: 28df3d1539de ("nfsd: clients don't need to break their own delegations")
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 97447a64bad0..886e50ed07c2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4869,6 +4869,11 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
> 	if (nf)
> 		nfsd_file_put(nf);
> 
> +	status = nfserrno(nfsd_open_break_lease(cur_fh->fh_dentry->d_inode,
> +								access));
> +	if (status)
> +		goto out_put_access;
> +
> 	status = nfsd4_truncate(rqstp, cur_fh, open);
> 	if (status)
> 		goto out_put_access;
> @@ -6849,11 +6854,20 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> static __be32 nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file_lock *lock)
> {
> 	struct nfsd_file *nf;
> -	__be32 err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
> -	if (!err) {
> -		err = nfserrno(vfs_test_lock(nf->nf_file, lock));
> -		nfsd_file_put(nf);
> -	}
> +	__be32 err;
> +
> +	err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
> +	if (err)
> +		return err;
> +	fh_lock(fhp); /* to block new leases till after test_lock: */
> +	err = nfserrno(nfsd_open_break_lease(fhp->fh_dentry->d_inode,
> +							NFSD_MAY_READ));
> +	if (err)
> +		goto out;
> +	err = nfserrno(vfs_test_lock(nf->nf_file, lock));
> +out:
> +	fh_unlock(fhp);
> +	nfsd_file_put(nf);
> 	return err;
> }
> 
> -- 
> 2.30.2
> 

--
Chuck Lever
Bruce Fields April 19, 2021, 7:55 p.m. UTC | #2
On Fri, Apr 16, 2021 at 07:11:36PM +0000, Chuck Lever III wrote:
> 
> 
> > On Apr 16, 2021, at 2:00 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > If nfsd already has an open file that it plans to use for IO from
> > another,
> 
> from another... client?

Yes, thanks.--b.

> 
> 
> > it may not need to do another vfs open, but it still may need
> > to break any delegations in case the existing opens are for another
> > client.
> > 
> > Symptoms are that we may incorrectly fail to break a delegation on a
> > write open from a different client, when the delegation-holding client
> > already has a write open.
> > 
> > Fixes: 28df3d1539de ("nfsd: clients don't need to break their own delegations")
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/nfsd/nfs4state.c | 24 +++++++++++++++++++-----
> > 1 file changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 97447a64bad0..886e50ed07c2 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4869,6 +4869,11 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
> > 	if (nf)
> > 		nfsd_file_put(nf);
> > 
> > +	status = nfserrno(nfsd_open_break_lease(cur_fh->fh_dentry->d_inode,
> > +								access));
> > +	if (status)
> > +		goto out_put_access;
> > +
> > 	status = nfsd4_truncate(rqstp, cur_fh, open);
> > 	if (status)
> > 		goto out_put_access;
> > @@ -6849,11 +6854,20 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > static __be32 nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file_lock *lock)
> > {
> > 	struct nfsd_file *nf;
> > -	__be32 err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
> > -	if (!err) {
> > -		err = nfserrno(vfs_test_lock(nf->nf_file, lock));
> > -		nfsd_file_put(nf);
> > -	}
> > +	__be32 err;
> > +
> > +	err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
> > +	if (err)
> > +		return err;
> > +	fh_lock(fhp); /* to block new leases till after test_lock: */
> > +	err = nfserrno(nfsd_open_break_lease(fhp->fh_dentry->d_inode,
> > +							NFSD_MAY_READ));
> > +	if (err)
> > +		goto out;
> > +	err = nfserrno(vfs_test_lock(nf->nf_file, lock));
> > +out:
> > +	fh_unlock(fhp);
> > +	nfsd_file_put(nf);
> > 	return err;
> > }
> > 
> > -- 
> > 2.30.2
> > 
> 
> --
> Chuck Lever
> 
>
diff mbox series

Patch

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 97447a64bad0..886e50ed07c2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4869,6 +4869,11 @@  static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp,
 	if (nf)
 		nfsd_file_put(nf);
 
+	status = nfserrno(nfsd_open_break_lease(cur_fh->fh_dentry->d_inode,
+								access));
+	if (status)
+		goto out_put_access;
+
 	status = nfsd4_truncate(rqstp, cur_fh, open);
 	if (status)
 		goto out_put_access;
@@ -6849,11 +6854,20 @@  nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 static __be32 nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file_lock *lock)
 {
 	struct nfsd_file *nf;
-	__be32 err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
-	if (!err) {
-		err = nfserrno(vfs_test_lock(nf->nf_file, lock));
-		nfsd_file_put(nf);
-	}
+	__be32 err;
+
+	err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf);
+	if (err)
+		return err;
+	fh_lock(fhp); /* to block new leases till after test_lock: */
+	err = nfserrno(nfsd_open_break_lease(fhp->fh_dentry->d_inode,
+							NFSD_MAY_READ));
+	if (err)
+		goto out;
+	err = nfserrno(vfs_test_lock(nf->nf_file, lock));
+out:
+	fh_unlock(fhp);
+	nfsd_file_put(nf);
 	return err;
 }