All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Coddington <bcodding@redhat.com>
To: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH v2] NFS: Don't skip lookup when holding a delegation
Date: Thu, 19 Sep 2019 10:49:00 -0400	[thread overview]
Message-ID: <77be993185fa7f114f6856f74f2f7affb5bd411d.1568904510.git.bcodding@redhat.com> (raw)

If we skip lookup revalidation while holding a delegation, we might miss
that the file has changed directories on the server.  The directory's
change attribute should still be checked against the dentry's d_time to
perform a complete revalidation.

V2 - Add some commentary as suggested-by J. Bruce Fields.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/dir.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 0adfd8840110..8723e82f5c9d 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1197,12 +1197,20 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
 		goto out_bad;
 	}
 
-	if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
-		return nfs_lookup_revalidate_delegated(dir, dentry, inode);
-
 	/* Force a full look up iff the parent directory has changed */
 	if (!(flags & (LOOKUP_EXCL | LOOKUP_REVAL)) &&
 	    nfs_check_verifier(dir, dentry, flags & LOOKUP_RCU)) {
+
+		/*
+		 * Note that the file can't move while we hold a
+		 * delegation.  But this dentry could have been cached
+		 * before we got a delegation.  So it's only safe to
+		 * skip revalidation when the parent directory is
+		 * unchanged:
+		 */
+		if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
+			return nfs_lookup_revalidate_delegated(dir, dentry, inode);
+
 		error = nfs_lookup_verify_inode(inode, flags);
 		if (error) {
 			if (error == -ESTALE)
@@ -1635,9 +1643,6 @@ nfs4_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
 	if (inode == NULL)
 		goto full_reval;
 
-	if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
-		return nfs_lookup_revalidate_delegated(dir, dentry, inode);
-
 	/* NFS only supports OPEN on regular files */
 	if (!S_ISREG(inode->i_mode))
 		goto full_reval;
-- 
2.20.1


             reply	other threads:[~2019-09-19 14:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-19 14:49 Benjamin Coddington [this message]
2020-01-16 15:19 ` [PATCH v2] NFS: Don't skip lookup when holding a delegation Benjamin Coddington
2020-01-16 16:02   ` Trond Myklebust
2020-01-16 16:32     ` Benjamin Coddington
2020-01-16 16:44       ` Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=77be993185fa7f114f6856f74f2f7affb5bd411d.1568904510.git.bcodding@redhat.com \
    --to=bcodding@redhat.com \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.