linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] VFS: name lookup improvements.
@ 2017-11-09  7:20 NeilBrown
  2017-11-09  7:20 ` [PATCH 1/3] VFS/nfs/9p: revise meaning of d_weak_invalidate NeilBrown
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: NeilBrown @ 2017-11-09  7:20 UTC (permalink / raw)
  To: Ian Kent, Latchesar Ionkov, Jeff Layton, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

These three patches address two issues: d_weak_revalidate and
path_mountpoint lookups.

The former is poorly defined and doesn't actually do the one thing
that it would be useful for it to do.  So the nfs implemention
is improved, the 9p one discarded, and the documentation clarified.

Given this change and recent change to follow_automount() the
mountpoint path lookup functions are no longer needed.  The regular
path look functions are quite sufficient.
The second two patches remove this with detailed explanation of why
it is OK.

Thanks,
NeilBrown


---

NeilBrown (3):
      VFS/nfs/9p: revise meaning of d_weak_invalidate.
      VFS: remove user_path_mountpoint_at()
      VFS / autofs4: remove kern_path_mountpoint()


 Documentation/filesystems/porting |    5 +
 Documentation/filesystems/vfs.txt |   11 +--
 fs/9p/vfs_dentry.c                |    1 
 fs/autofs4/dev-ioctl.c            |    5 -
 fs/internal.h                     |    1 
 fs/namei.c                        |  150 -------------------------------------
 fs/namespace.c                    |    2 
 fs/nfs/dir.c                      |   60 ++-------------
 include/linux/namei.h             |    1 
 9 files changed, 24 insertions(+), 212 deletions(-)

--
Signature

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] VFS/nfs/9p: revise meaning of d_weak_invalidate.
  2017-11-09  7:20 [PATCH 0/3] VFS: name lookup improvements NeilBrown
@ 2017-11-09  7:20 ` NeilBrown
  2017-11-09  7:20 ` [PATCH 2/3] VFS: remove user_path_mountpoint_at() NeilBrown
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2017-11-09  7:20 UTC (permalink / raw)
  To: Ian Kent, Latchesar Ionkov, Jeff Layton, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

d_weak_invalidate() is called when a path lookup ends with
something other than a simple name.
This happen when it:
  - ends "." or "..",
  - ends at a mountpoint (including "/"), or
  - ends at a procfs symlink.

In these cases, revalidating the name of the dentry is inappropriate
as the name wasn't used.  The comments suggest it is necessary to
revalidate the inode, but this is not the case.  Whatever operation
is called on the final dentry has the opportunity to revalidate it,
and will do so - certainly both nfs_getattr and v9fs_vfs_getattr do.

The one case where d_weak_revalidate() *is* needed is for an open()
when d_revalidate() performs some handling of LOOKUP_OPEN.  The same
handling should be performed for d_weak_revalidate().  NFS is the only
filesystem which handles LOOKUP_OPEN, but it doesn't do the handling
in d_weak_revalidate().

A consequence of this is that we do not get proper close-to-open semantics
of paths that do not end with a simple name.
This can easily be confirmed by changing directory to a non-root NFS directory
and running "echo *" while watching network traffic.
CTO semantics requires the file attributes to be revalidated on each open,
but that doesn't happen.

d_weak_revalidate can use the same implementation as d_invalidate by
ensuring that implementation does nothing when LOOKUP_JUMPED is set
(implying d_weak_revalidate was called) and LOOKUP_OPEN is clear (implying
the inodes isn't being opened).

This patch:
  - removes d_weak_invalidate() from 9p as 9p doesn't handle LOOKUP_OPEN.
  - discards nfs_weak_revalidate,
  - uses nfs_revalidate for d_weak_revalidate, ensuring to avoid the
    unnecessary lookup when LOOKUP_JUMPED is set (but still handling
    LOOKUP_OPEN correctly),
  - defines d_weak_revalidate for nfsv4 as well (this omission first lead
    me to examine d_weak_revalidate more closely),
  - removes special revalidation of the root directory in nfs_opendir()
    as this is no longer needed,
  - updates some documentation.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 Documentation/filesystems/porting |    5 +++
 Documentation/filesystems/vfs.txt |   11 ++++---
 fs/9p/vfs_dentry.c                |    1 -
 fs/nfs/dir.c                      |   60 ++++++-------------------------------
 4 files changed, 21 insertions(+), 56 deletions(-)

diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index 93e0a2404532..f455757ff1c6 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -606,3 +606,8 @@ in your dentry operations instead.
 	dentry separately, and it now has request_mask and query_flags arguments
 	to specify the fields and sync type requested by statx.  Filesystems not
 	supporting any statx-specific features may ignore the new arguments.
+--
+[recommended]
+	->d_weak_revalidate should perform the same handling of LOOKUP_OPEN as
+	->d_revalidate.  If LOOKUP_OPEN is not set, d_weak_revalidate need not
+	do anything.
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 5fd325df59e2..c2025e226b29 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -1015,14 +1015,15 @@ struct dentry_operations {
 	doing a lookup in the parent directory. This includes "/", "." and "..",
 	as well as procfs-style symlinks and mountpoint traversal.
 
-	In this case, we are less concerned with whether the dentry is still
-	fully correct, but rather that the inode is still valid. As with
-	d_revalidate, most local filesystems will set this to NULL since their
-	dcache entries are always valid.
+	Filesystems only need this if they handle LOOKUP_OPEN in
+	d_revalidate, in which case the same handling should be applied
+	in d_weak_revalidate.  When LOOKUP_OPEN is not set,
+	d_weak_revalidate can safely be a no-op.
 
 	This function has the same return code semantics as d_revalidate.
 
-	d_weak_revalidate is only called after leaving rcu-walk mode.
+	d_weak_revalidate is only called after leaving rcu-walk mode,
+	so LOOKUP_RCU is never set.
 
   d_hash: called when the VFS adds a dentry to the hash table. The first
 	dentry passed to d_hash is the parent directory that the name is
diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
index bd456c668d39..99eaf3c6d44c 100644
--- a/fs/9p/vfs_dentry.c
+++ b/fs/9p/vfs_dentry.c
@@ -111,7 +111,6 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 
 const struct dentry_operations v9fs_cached_dentry_operations = {
 	.d_revalidate = v9fs_lookup_revalidate,
-	.d_weak_revalidate = v9fs_lookup_revalidate,
 	.d_delete = v9fs_cached_dentry_delete,
 	.d_release = v9fs_dentry_release,
 };
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5ceaeb1f6fb6..fc349577526f 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -118,13 +118,6 @@ nfs_opendir(struct inode *inode, struct file *filp)
 		goto out;
 	}
 	filp->private_data = ctx;
-	if (filp->f_path.dentry == filp->f_path.mnt->mnt_root) {
-		/* This is a mountpoint, so d_revalidate will never
-		 * have been called, so we need to refresh the
-		 * inode (for close-open consistency) ourselves.
-		 */
-		__nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	}
 out:
 	put_rpccred(cred);
 	return res;
@@ -972,17 +965,19 @@ EXPORT_SYMBOL_GPL(nfs_force_lookup_revalidate);
  * If rcu_walk prevents us from performing a full check, return 0.
  */
 static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
-			      int rcu_walk)
+			      unsigned int flags)
 {
 	if (IS_ROOT(dentry))
 		return 1;
+	if (flags & LOOKUP_JUMPED)
+		return 1;
 	if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONE)
 		return 0;
 	if (!nfs_verify_change_attribute(dir, dentry->d_time))
 		return 0;
 	/* Revalidate nfsi->cache_change_attribute before we declare a match */
 	if (nfs_mapping_need_revalidate_inode(dir)) {
-		if (rcu_walk)
+		if (flags & LOOKUP_RCU)
 			return 0;
 		if (__nfs_revalidate_inode(NFS_SERVER(dir), dir) < 0)
 			return 0;
@@ -1056,7 +1051,7 @@ int nfs_neg_need_reval(struct inode *dir, struct dentry *dentry,
 		return 0;
 	if (NFS_SERVER(dir)->flags & NFS_MOUNT_LOOKUP_CACHE_NONEG)
 		return 1;
-	return !nfs_check_verifier(dir, dentry, flags & LOOKUP_RCU);
+	return !nfs_check_verifier(dir, dentry, flags);
 }
 
 /*
@@ -1114,7 +1109,7 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 
 	/* Force a full look up iff the parent directory has changed */
 	if (!nfs_is_exclusive_create(dir, flags) &&
-	    nfs_check_verifier(dir, dentry, flags & LOOKUP_RCU)) {
+	    nfs_check_verifier(dir, dentry, flags)) {
 		error = nfs_lookup_verify_inode(inode, flags);
 		if (error) {
 			if (flags & LOOKUP_RCU)
@@ -1129,6 +1124,8 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
+	if (flags & LOOKUP_JUMPED)
+		return 1;
 
 	if (NFS_STALE(inode))
 		goto out_bad;
@@ -1210,44 +1207,6 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 	return error;
 }
 
-/*
- * A weaker form of d_revalidate for revalidating just the d_inode(dentry)
- * when we don't really care about the dentry name. This is called when a
- * pathwalk ends on a dentry that was not found via a normal lookup in the
- * parent dir (e.g.: ".", "..", procfs symlinks or mountpoint traversals).
- *
- * In this situation, we just want to verify that the inode itself is OK
- * since the dentry might have changed on the server.
- */
-static int nfs_weak_revalidate(struct dentry *dentry, unsigned int flags)
-{
-	struct inode *inode = d_inode(dentry);
-	int error = 0;
-
-	/*
-	 * I believe we can only get a negative dentry here in the case of a
-	 * procfs-style symlink. Just assume it's correct for now, but we may
-	 * eventually need to do something more here.
-	 */
-	if (!inode) {
-		dfprintk(LOOKUPCACHE, "%s: %pd2 has negative inode\n",
-				__func__, dentry);
-		return 1;
-	}
-
-	if (is_bad_inode(inode)) {
-		dfprintk(LOOKUPCACHE, "%s: %pd2 has dud inode\n",
-				__func__, dentry);
-		return 0;
-	}
-
-	if (nfs_mapping_need_revalidate_inode(inode))
-		error = __nfs_revalidate_inode(NFS_SERVER(inode), inode);
-	dfprintk(LOOKUPCACHE, "NFS: %s: inode %lu is %s\n",
-			__func__, inode->i_ino, error ? "invalid" : "valid");
-	return !error;
-}
-
 /*
  * This is called from dput() when d_count is going to 0.
  */
@@ -1314,7 +1273,7 @@ static void nfs_d_release(struct dentry *dentry)
 
 const struct dentry_operations nfs_dentry_operations = {
 	.d_revalidate	= nfs_lookup_revalidate,
-	.d_weak_revalidate	= nfs_weak_revalidate,
+	.d_weak_revalidate	= nfs_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,
@@ -1393,6 +1352,7 @@ static int nfs4_lookup_revalidate(struct dentry *, unsigned int);
 
 const struct dentry_operations nfs4_dentry_operations = {
 	.d_revalidate	= nfs4_lookup_revalidate,
+	.d_weak_revalidate	= nfs4_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] VFS: remove user_path_mountpoint_at()
  2017-11-09  7:20 [PATCH 0/3] VFS: name lookup improvements NeilBrown
  2017-11-09  7:20 ` [PATCH 1/3] VFS/nfs/9p: revise meaning of d_weak_invalidate NeilBrown
@ 2017-11-09  7:20 ` NeilBrown
  2017-11-09  7:20 ` [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint() NeilBrown
  2017-11-09 12:32 ` [PATCH 0/3] VFS: name lookup improvements Jeff Layton
  3 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2017-11-09  7:20 UTC (permalink / raw)
  To: Ian Kent, Latchesar Ionkov, Jeff Layton, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

Now that d_weak_revalidate doesn't revalidate the inode (unless
LOOKUP_OPEN is set), we don't need any extra care when umounting.
A simple user_path_at() will find the desired dentry without
performing any access on the mounted filesystems.
So we don't need user_path_mountpoint_at().

By switching to user_path_at(), there are other changes than just the
d_weak_revalidate() change.
- We no longer use LOOKUP_NO_REVAL on the last component, in the
  unlikely case that d_lookup() failed.  It is hard to see why
  this is relevant.  Most likely if d_lookup() failed we will
  have called i_op->lookup, which is at least as much of a problem
  as d_revalidate() might be.
- we now call follow_managed() on the final dentry.  This cannot
  trigger an automount, due to the flags used, but might call
  ->d_manage().  There is no reason to expect that this might
  cause problems.

So we can safely switch to user_path_at() and discard
user_path_mountpoint_at().

kern_path_mountpoint() is still in use by autofs, and so cannot go
just yet.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/internal.h  |    1 -
 fs/namei.c     |   21 ---------------------
 fs/namespace.c |    2 +-
 3 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 48cee21b4f14..d1228af28761 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -52,7 +52,6 @@ extern void __init chrdev_init(void);
 /*
  * namei.c
  */
-extern int user_path_mountpoint_at(int, const char __user *, unsigned int, struct path *);
 extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
 			   const char *, unsigned int, struct path *);
 
diff --git a/fs/namei.c b/fs/namei.c
index ed8b9488a890..6639203d7eba 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2722,27 +2722,6 @@ filename_mountpoint(int dfd, struct filename *name, struct path *path,
 	return error;
 }
 
-/**
- * user_path_mountpoint_at - lookup a path from userland in order to umount it
- * @dfd:	directory file descriptor
- * @name:	pathname from userland
- * @flags:	lookup flags
- * @path:	pointer to container to hold result
- *
- * A umount is a special case for path walking. We're not actually interested
- * in the inode in this situation, and ESTALE errors can be a problem. We
- * simply want track down the dentry and vfsmount attached at the mountpoint
- * and avoid revalidating the last component.
- *
- * Returns 0 and populates "path" on success.
- */
-int
-user_path_mountpoint_at(int dfd, const char __user *name, unsigned int flags,
-			struct path *path)
-{
-	return filename_mountpoint(dfd, getname(name), path, flags);
-}
-
 int
 kern_path_mountpoint(int dfd, const char *name, struct path *path,
 			unsigned int flags)
diff --git a/fs/namespace.c b/fs/namespace.c
index 23cdf6c62895..6de22f658359 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1696,7 +1696,7 @@ SYSCALL_DEFINE2(umount, char __user *, name, int, flags)
 	if (!(flags & UMOUNT_NOFOLLOW))
 		lookup_flags |= LOOKUP_FOLLOW;
 
-	retval = user_path_mountpoint_at(AT_FDCWD, name, lookup_flags, &path);
+	retval = user_path_at(AT_FDCWD, name, lookup_flags, &path);
 	if (retval)
 		goto out;
 	mnt = real_mount(path.mnt);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint()
  2017-11-09  7:20 [PATCH 0/3] VFS: name lookup improvements NeilBrown
  2017-11-09  7:20 ` [PATCH 1/3] VFS/nfs/9p: revise meaning of d_weak_invalidate NeilBrown
  2017-11-09  7:20 ` [PATCH 2/3] VFS: remove user_path_mountpoint_at() NeilBrown
@ 2017-11-09  7:20 ` NeilBrown
  2017-11-09 20:06   ` Linus Torvalds
  2017-11-09 12:32 ` [PATCH 0/3] VFS: name lookup improvements Jeff Layton
  3 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2017-11-09  7:20 UTC (permalink / raw)
  To: Ian Kent, Latchesar Ionkov, Jeff Layton, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

kern_path_mountpoint() is only called from autofs4 to perform
lookups which need to identify autofs4 mount points.

Many of the differences between kern_path() and kern_path_mountpoint()
are related to the fact that we will never use O_CREAT with the
latter, and don't need to "open" the target.
The main differences that could be relevant to autofs4 are:
- kern_path_mountpoint() does not call complete_walk() in
  mountpoint_last(), contrasting with do_last() which does call it.
  This means ->d_weak_revalidate() is not called from autofs4.

- follow_managed() is not call from mountpoint_last().

- LOOKUP_NO_REVAL is used for lookup_slow on the last component,
  if it isn't in cache.

As ->d_weak_revalidate() is now a no-op when LOOKUP_OPEN isn't
present, the first difference is no longer important.

The use of LOOKUP_NO_REVAL shouldn't cause autofs4 any problems
as no autofs4 dentry has ->d_revalidate().

follow_managed() might:
 a/ call ->d_manage()
 b/ might cross a mountpoint
 c/ might call follow_automount()

'b' cannot be relevant as path_mountpoint calls follow_mount() after
mountpoint_last() is called.

'a' might only be interesting when ->d_manage is autofs4_d_manage(),
but autofs4 only calls kern_path_mountpoint from ioctls issued by the
automount daemon, and autofs4_d_manage() will exit quickly in that
case.  So there is no risk of autofs4_d_manage() waiting for the
automount daemon (which it would be blocking) and causing a deadlock.

'c' could have been a problem before commit 42f461482178 ("autofs: fix
AT_NO_AUTOMOUNT not being honored").  Prior to that commit a lookup
for a negative autofs4 dentry could trigger an automount, even though
'flags' is 0.  Since that commit and error is returned instead.

So follow_managed() is no longer a problem.

So there is no reason that autofs4 needs to use kern_path_mountpoint()
any more.  It cannot deadlock.
So the whole 'path mountpoint' infrastructure can be discarded.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/autofs4/dev-ioctl.c |    5 +-
 fs/namei.c             |  129 ------------------------------------------------
 include/linux/namei.h  |    1 
 3 files changed, 2 insertions(+), 133 deletions(-)

diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index b7c816f39404..716c44593117 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -209,7 +209,7 @@ static int find_autofs_mount(const char *pathname,
 	struct path path;
 	int err;
 
-	err = kern_path_mountpoint(AT_FDCWD, pathname, &path, 0);
+	err = kern_path(pathname,0 , &path);
 	if (err)
 		return err;
 	err = -ENOENT;
@@ -547,8 +547,7 @@ static int autofs_dev_ioctl_ismountpoint(struct file *fp,
 
 	if (!fp || param->ioctlfd == -1) {
 		if (autofs_type_any(type))
-			err = kern_path_mountpoint(AT_FDCWD,
-						   name, &path, LOOKUP_FOLLOW);
+			err = kern_path(name, LOOKUP_FOLLOW, &path);
 		else
 			err = find_autofs_mount(name, &path,
 						test_by_type, &type);
diff --git a/fs/namei.c b/fs/namei.c
index 6639203d7eba..e90680a3f6f1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2601,135 +2601,6 @@ int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
 }
 EXPORT_SYMBOL(user_path_at_empty);
 
-/**
- * mountpoint_last - look up last component for umount
- * @nd:   pathwalk nameidata - currently pointing at parent directory of "last"
- *
- * This is a special lookup_last function just for umount. In this case, we
- * need to resolve the path without doing any revalidation.
- *
- * The nameidata should be the result of doing a LOOKUP_PARENT pathwalk. Since
- * mountpoints are always pinned in the dcache, their ancestors are too. Thus,
- * in almost all cases, this lookup will be served out of the dcache. The only
- * cases where it won't are if nd->last refers to a symlink or the path is
- * bogus and it doesn't exist.
- *
- * Returns:
- * -error: if there was an error during lookup. This includes -ENOENT if the
- *         lookup found a negative dentry.
- *
- * 0:      if we successfully resolved nd->last and found it to not to be a
- *         symlink that needs to be followed.
- *
- * 1:      if we successfully resolved nd->last and found it to be a symlink
- *         that needs to be followed.
- */
-static int
-mountpoint_last(struct nameidata *nd)
-{
-	int error = 0;
-	struct dentry *dir = nd->path.dentry;
-	struct path path;
-
-	/* If we're in rcuwalk, drop out of it to handle last component */
-	if (nd->flags & LOOKUP_RCU) {
-		if (unlazy_walk(nd))
-			return -ECHILD;
-	}
-
-	nd->flags &= ~LOOKUP_PARENT;
-
-	if (unlikely(nd->last_type != LAST_NORM)) {
-		error = handle_dots(nd, nd->last_type);
-		if (error)
-			return error;
-		path.dentry = dget(nd->path.dentry);
-	} else {
-		path.dentry = d_lookup(dir, &nd->last);
-		if (!path.dentry) {
-			/*
-			 * No cached dentry. Mounted dentries are pinned in the
-			 * cache, so that means that this dentry is probably
-			 * a symlink or the path doesn't actually point
-			 * to a mounted dentry.
-			 */
-			path.dentry = lookup_slow(&nd->last, dir,
-					     nd->flags | LOOKUP_NO_REVAL);
-			if (IS_ERR(path.dentry))
-				return PTR_ERR(path.dentry);
-		}
-	}
-	if (d_is_negative(path.dentry)) {
-		dput(path.dentry);
-		return -ENOENT;
-	}
-	path.mnt = nd->path.mnt;
-	return step_into(nd, &path, 0, d_backing_inode(path.dentry), 0);
-}
-
-/**
- * path_mountpoint - look up a path to be umounted
- * @nd:		lookup context
- * @flags:	lookup flags
- * @path:	pointer to container for result
- *
- * Look up the given name, but don't attempt to revalidate the last component.
- * Returns 0 and "path" will be valid on success; Returns error otherwise.
- */
-static int
-path_mountpoint(struct nameidata *nd, unsigned flags, struct path *path)
-{
-	const char *s = path_init(nd, flags);
-	int err;
-	if (IS_ERR(s))
-		return PTR_ERR(s);
-	while (!(err = link_path_walk(s, nd)) &&
-		(err = mountpoint_last(nd)) > 0) {
-		s = trailing_symlink(nd);
-		if (IS_ERR(s)) {
-			err = PTR_ERR(s);
-			break;
-		}
-	}
-	if (!err) {
-		*path = nd->path;
-		nd->path.mnt = NULL;
-		nd->path.dentry = NULL;
-		follow_mount(path);
-	}
-	terminate_walk(nd);
-	return err;
-}
-
-static int
-filename_mountpoint(int dfd, struct filename *name, struct path *path,
-			unsigned int flags)
-{
-	struct nameidata nd;
-	int error;
-	if (IS_ERR(name))
-		return PTR_ERR(name);
-	set_nameidata(&nd, dfd, name);
-	error = path_mountpoint(&nd, flags | LOOKUP_RCU, path);
-	if (unlikely(error == -ECHILD))
-		error = path_mountpoint(&nd, flags, path);
-	if (unlikely(error == -ESTALE))
-		error = path_mountpoint(&nd, flags | LOOKUP_REVAL, path);
-	if (likely(!error))
-		audit_inode(name, path->dentry, 0);
-	restore_nameidata();
-	putname(name);
-	return error;
-}
-
-int
-kern_path_mountpoint(int dfd, const char *name, struct path *path,
-			unsigned int flags)
-{
-	return filename_mountpoint(dfd, getname_kernel(name), path, flags);
-}
-EXPORT_SYMBOL(kern_path_mountpoint);
-
 int __check_sticky(struct inode *dir, struct inode *inode)
 {
 	kuid_t fsuid = current_fsuid();
diff --git a/include/linux/namei.h b/include/linux/namei.h
index a982bb7cd480..e576bf40d761 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -79,7 +79,6 @@ extern struct dentry *kern_path_create(int, const char *, struct path *, unsigne
 extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int);
 extern void done_path_create(struct path *, struct dentry *);
 extern struct dentry *kern_path_locked(const char *, struct path *);
-extern int kern_path_mountpoint(int, const char *, struct path *, unsigned int);
 
 extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
 extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] VFS: name lookup improvements.
  2017-11-09  7:20 [PATCH 0/3] VFS: name lookup improvements NeilBrown
                   ` (2 preceding siblings ...)
  2017-11-09  7:20 ` [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint() NeilBrown
@ 2017-11-09 12:32 ` Jeff Layton
  2017-11-09 21:06   ` NeilBrown
  3 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2017-11-09 12:32 UTC (permalink / raw)
  To: NeilBrown, Ian Kent, Latchesar Ionkov, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

On Thu, 2017-11-09 at 18:20 +1100, NeilBrown wrote:
> These three patches address two issues: d_weak_revalidate and
> path_mountpoint lookups.
> 
> The former is poorly defined and doesn't actually do the one thing
> that it would be useful for it to do.  So the nfs implemention
> is improved, the 9p one discarded, and the documentation clarified.
> 
> Given this change and recent change to follow_automount() the
> mountpoint path lookup functions are no longer needed.  The regular
> path look functions are quite sufficient.
> The second two patches remove this with detailed explanation of why
> it is OK.
> 
> Thanks,
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (3):
>       VFS/nfs/9p: revise meaning of d_weak_invalidate.
>       VFS: remove user_path_mountpoint_at()
>       VFS / autofs4: remove kern_path_mountpoint()
> 
> 
>  Documentation/filesystems/porting |    5 +
>  Documentation/filesystems/vfs.txt |   11 +--
>  fs/9p/vfs_dentry.c                |    1 
>  fs/autofs4/dev-ioctl.c            |    5 -
>  fs/internal.h                     |    1 
>  fs/namei.c                        |  150 -------------------------------------
>  fs/namespace.c                    |    2 
>  fs/nfs/dir.c                      |   60 ++-------------
>  include/linux/namei.h             |    1 
>  9 files changed, 24 insertions(+), 212 deletions(-)
> 
> --
> Signature
> 

I love that diffstat and I think the patches and the logic behind them
look reasonable. I'm the one that added d_weak_revalidate and while it
did fix a problem at the time, it has always seemed a bit of an odd
d_op.

Your patch does make me wonder if we should consider merging
d_weak_revalidate and d_revalidate back together, and simply require all
the d_revalidate ops vet the flags more thoroughly.

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint()
  2017-11-09  7:20 ` [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint() NeilBrown
@ 2017-11-09 20:06   ` Linus Torvalds
  0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2017-11-09 20:06 UTC (permalink / raw)
  To: NeilBrown
  Cc: Ian Kent, Latchesar Ionkov, Jeff Layton, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust, Linux NFS Mailing List,
	autofs mailing list, Linux Kernel Mailing List, linux-fsdevel,
	V9FS Developers

On Wed, Nov 8, 2017 at 11:20 PM, NeilBrown <neilb@suse.com> wrote:
> ---
>  fs/autofs4/dev-ioctl.c |    5 +-
>  fs/namei.c             |  129 ------------------------------------------------
>  include/linux/namei.h  |    1
>  3 files changed, 2 insertions(+), 133 deletions(-)

This one certainly looks lovely, and the rationale all looks solid.

Al, mind taking a look?

             Linus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] VFS: name lookup improvements.
  2017-11-09 12:32 ` [PATCH 0/3] VFS: name lookup improvements Jeff Layton
@ 2017-11-09 21:06   ` NeilBrown
  2017-11-10  0:21     ` NeilBrown
  0 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2017-11-09 21:06 UTC (permalink / raw)
  To: Jeff Layton, Ian Kent, Latchesar Ionkov, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

[-- Attachment #1: Type: text/plain, Size: 2219 bytes --]

On Thu, Nov 09 2017, Jeff Layton wrote:

> On Thu, 2017-11-09 at 18:20 +1100, NeilBrown wrote:
>> These three patches address two issues: d_weak_revalidate and
>> path_mountpoint lookups.
>> 
>> The former is poorly defined and doesn't actually do the one thing
>> that it would be useful for it to do.  So the nfs implemention
>> is improved, the 9p one discarded, and the documentation clarified.
>> 
>> Given this change and recent change to follow_automount() the
>> mountpoint path lookup functions are no longer needed.  The regular
>> path look functions are quite sufficient.
>> The second two patches remove this with detailed explanation of why
>> it is OK.
>> 
>> Thanks,
>> NeilBrown
>> 
>> 
>> ---
>> 
>> NeilBrown (3):
>>       VFS/nfs/9p: revise meaning of d_weak_invalidate.
>>       VFS: remove user_path_mountpoint_at()
>>       VFS / autofs4: remove kern_path_mountpoint()
>> 
>> 
>>  Documentation/filesystems/porting |    5 +
>>  Documentation/filesystems/vfs.txt |   11 +--
>>  fs/9p/vfs_dentry.c                |    1 
>>  fs/autofs4/dev-ioctl.c            |    5 -
>>  fs/internal.h                     |    1 
>>  fs/namei.c                        |  150 -------------------------------------
>>  fs/namespace.c                    |    2 
>>  fs/nfs/dir.c                      |   60 ++-------------
>>  include/linux/namei.h             |    1 
>>  9 files changed, 24 insertions(+), 212 deletions(-)
>> 
>> --
>> Signature
>> 
>
> I love that diffstat and I think the patches and the logic behind them
> look reasonable. I'm the one that added d_weak_revalidate and while it
> did fix a problem at the time, it has always seemed a bit of an odd
> d_op.
>
> Your patch does make me wonder if we should consider merging
> d_weak_revalidate and d_revalidate back together, and simply require all
> the d_revalidate ops vet the flags more thoroughly.

This boils down to adding

 if (flags & LOOKUP_JUMPED)
 	return 1;

to the front of almost every d_revalidate function.
I'm probably in favour of that, but it isn't an obvious win.
I can certainly offer it as a follow-on patch so we can see
exactly the impact.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] VFS: name lookup improvements.
  2017-11-09 21:06   ` NeilBrown
@ 2017-11-10  0:21     ` NeilBrown
  0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2017-11-10  0:21 UTC (permalink / raw)
  To: Jeff Layton, Ian Kent, Latchesar Ionkov, Eric Van Hensbergen,
	Al Viro, Ron Minnich, Trond Myklebust
  Cc: linux-nfs, autofs, linux-kernel, linux-fsdevel, v9fs-developer,
	Linus Torvalds

[-- Attachment #1: Type: text/plain, Size: 18281 bytes --]

On Fri, Nov 10 2017, NeilBrown wrote:
>>
>> Your patch does make me wonder if we should consider merging
>> d_weak_revalidate and d_revalidate back together, and simply require all
>> the d_revalidate ops vet the flags more thoroughly.
>
> This boils down to adding
>
>  if (flags & LOOKUP_JUMPED)
>  	return 1;
>
> to the front of almost every d_revalidate function.
> I'm probably in favour of that, but it isn't an obvious win.
> I can certainly offer it as a follow-on patch so we can see
> exactly the impact.


See below.
NeilBrown

From: NeilBrown <neilb@suse.com>
Date: Wed, 16 Aug 2017 09:03:36 +1000
Subject: [PATCH] VFS: remove d_weak_revalidate inode operation

d_weak_revalidate is now very similar to d_revalidate.
The difference is that the former always has LOOKUP_JUMPED
set and the later doesn't.  When LOOKUP_JUMPED is set
the name does not need to be revalidated, but the inode might.

This distinction can be managed entirely in the filesystem
without needing two similar entry points.

So discard d_weak_revalidate and call d_revalidate instead.
Make sure all d_revalidate functions avoid checking the validity
of the name if LOOKUP_JUMPED is set.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 Documentation/filesystems/Locking            |  2 --
 Documentation/filesystems/porting            |  6 ++++++
 Documentation/filesystems/vfs.txt            | 20 +++++---------------
 drivers/staging/lustre/lustre/llite/dcache.c |  2 +-
 fs/9p/vfs_dentry.c                           |  3 +++
 fs/afs/dir.c                                 |  3 +++
 fs/ceph/dir.c                                |  3 +++
 fs/cifs/dir.c                                |  3 +++
 fs/coda/dir.c                                |  3 +++
 fs/crypto/crypto.c                           |  3 +++
 fs/dcache.c                                  |  3 ---
 fs/ecryptfs/dentry.c                         |  3 +++
 fs/fat/namei_vfat.c                          |  4 ++++
 fs/fuse/dir.c                                |  3 +++
 fs/gfs2/dentry.c                             |  3 +++
 fs/kernfs/dir.c                              |  3 +++
 fs/namei.c                                   |  4 ++--
 fs/ncpfs/dir.c                               |  3 +++
 fs/nfs/dir.c                                 |  2 --
 fs/ocfs2/dcache.c                            |  3 +++
 fs/orangefs/dcache.c                         |  3 +++
 fs/overlayfs/super.c                         | 19 -------------------
 fs/overlayfs/util.c                          |  3 +--
 fs/proc/base.c                               |  3 +++
 fs/proc/fd.c                                 |  3 +++
 include/linux/dcache.h                       |  3 ---
 26 files changed, 64 insertions(+), 49 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 75d2d57e2c44..a824a305cf5d 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -10,7 +10,6 @@ be able to use diff(1).
 --------------------------- dentry_operations --------------------------
 prototypes:
 	int (*d_revalidate)(struct dentry *, unsigned int);
-	int (*d_weak_revalidate)(struct dentry *, unsigned int);
 	int (*d_hash)(const struct dentry *, struct qstr *);
 	int (*d_compare)(const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
@@ -27,7 +26,6 @@ prototypes:
 locking rules:
 		rename_lock	->d_lock	may block	rcu-walk
 d_revalidate:	no		no		yes (ref-walk)	maybe
-d_weak_revalidate:no		no		yes	 	no
 d_hash		no		no		no		maybe
 d_compare:	yes		no		no		maybe
 d_delete:	no		yes		no		no
diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index f455757ff1c6..56809d6938b5 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -611,3 +611,9 @@ in your dentry operations instead.
 	->d_weak_revalidate should perform the same handling of LOOKUP_OPEN as
 	->d_revalidate.  If LOOKUP_OPEN is not set, d_weak_revalidate need not
 	do anything.
+--
+[mandatory]
+	->d_weak_revalidate() is gone.  ->d_revalidate must check for
+	LOOKUP_JUMPED and skip revalidate of the name in that case.
+	If LOOKUP_JUMPED is set and LOOKUP_OPEN is not set, ->d_revalidate
+	need not do anything.
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index c2025e226b29..d4bd4f016115 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -976,7 +976,6 @@ defined:
 
 struct dentry_operations {
 	int (*d_revalidate)(struct dentry *, unsigned int);
-	int (*d_weak_revalidate)(struct dentry *, unsigned int);
 	int (*d_hash)(const struct dentry *, struct qstr *);
 	int (*d_compare)(const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
@@ -1010,20 +1009,11 @@ struct dentry_operations {
 	If a situation is encountered that rcu-walk cannot handle, return
 	-ECHILD and it will be called again in ref-walk mode.
 
- d_weak_revalidate: called when the VFS needs to revalidate a "jumped" dentry.
-	This is called when a path-walk ends at dentry that was not acquired by
-	doing a lookup in the parent directory. This includes "/", "." and "..",
-	as well as procfs-style symlinks and mountpoint traversal.
-
-	Filesystems only need this if they handle LOOKUP_OPEN in
-	d_revalidate, in which case the same handling should be applied
-	in d_weak_revalidate.  When LOOKUP_OPEN is not set,
-	d_weak_revalidate can safely be a no-op.
-
-	This function has the same return code semantics as d_revalidate.
-
-	d_weak_revalidate is only called after leaving rcu-walk mode,
-	so LOOKUP_RCU is never set.
+	If LOOKUP_JUMPED is set in flags, the name does not need to be
+	revalidated and in many cases d_revalidate can safely do
+	nothing.  If the filesystem handles LOOKUP_OPEN in
+	d_revalidate, that same handling should still be performed
+	when LOOKUP_JUMPED is set.
 
   d_hash: called when the VFS adds a dentry to the hash table. The first
 	dentry passed to d_hash is the parent directory that the name is
diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
index 3670fcaf373f..b6c54edc45cd 100644
--- a/drivers/staging/lustre/lustre/llite/dcache.c
+++ b/drivers/staging/lustre/lustre/llite/dcache.c
@@ -240,7 +240,7 @@ static int ll_revalidate_dentry(struct dentry *dentry,
 	 * to this dentry, then its lock has not been revoked and the
 	 * path component is valid.
 	 */
-	if (lookup_flags & LOOKUP_PARENT)
+	if (lookup_flags & (LOOKUP_PARENT | LOOKUP_JUMPED))
 		return 1;
 
 	/* Symlink - always valid as long as the dentry was found */
diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
index 99eaf3c6d44c..0995aaa4e48e 100644
--- a/fs/9p/vfs_dentry.c
+++ b/fs/9p/vfs_dentry.c
@@ -83,6 +83,9 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode(dentry);
 	if (!inode)
 		goto out_valid;
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 613a77058263..71b46202c15a 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -588,6 +588,9 @@ static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	vnode = AFS_FS_I(d_inode(dentry));
 
 	if (d_really_is_positive(dentry))
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 8a5266699b67..718547b8c348 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1217,6 +1217,9 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
 	struct dentry *parent;
 	struct inode *dir;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	if (flags & LOOKUP_RCU) {
 		parent = READ_ONCE(dentry->d_parent);
 		dir = d_inode_rcu(parent);
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 81ba6e0d88d8..5714a036c0a6 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -843,6 +843,9 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	if (d_really_is_positive(direntry)) {
 		if (cifs_revalidate_dentry(direntry))
 			return 0;
diff --git a/fs/coda/dir.c b/fs/coda/dir.c
index 00876ddadb43..41a44465de2d 100644
--- a/fs/coda/dir.c
+++ b/fs/coda/dir.c
@@ -461,6 +461,9 @@ static int coda_dentry_revalidate(struct dentry *de, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode(de);
 	if (!inode || is_root_inode(inode))
 		goto out;
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index c7835df7e7b8..fd14eb2cd6cc 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -339,6 +339,9 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	dir = dget_parent(dentry);
 	if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) {
 		dput(dir);
diff --git a/fs/dcache.c b/fs/dcache.c
index 3130d62f29c9..0dd4e53fbc43 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1755,7 +1755,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
 	WARN_ON_ONCE(dentry->d_flags & (DCACHE_OP_HASH	|
 				DCACHE_OP_COMPARE	|
 				DCACHE_OP_REVALIDATE	|
-				DCACHE_OP_WEAK_REVALIDATE	|
 				DCACHE_OP_DELETE	|
 				DCACHE_OP_REAL));
 	dentry->d_op = op;
@@ -1767,8 +1766,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
 		dentry->d_flags |= DCACHE_OP_COMPARE;
 	if (op->d_revalidate)
 		dentry->d_flags |= DCACHE_OP_REVALIDATE;
-	if (op->d_weak_revalidate)
-		dentry->d_flags |= DCACHE_OP_WEAK_REVALIDATE;
 	if (op->d_delete)
 		dentry->d_flags |= DCACHE_OP_DELETE;
 	if (op->d_prune)
diff --git a/fs/ecryptfs/dentry.c b/fs/ecryptfs/dentry.c
index 63cd2c147221..bf908d964aab 100644
--- a/fs/ecryptfs/dentry.c
+++ b/fs/ecryptfs/dentry.c
@@ -50,6 +50,9 @@ static int ecryptfs_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	if (lower_dentry->d_flags & DCACHE_OP_REVALIDATE)
 		rc = lower_dentry->d_op->d_revalidate(lower_dentry, flags);
 
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 02c066663a3a..a5bfb7fde9c5 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -45,6 +45,7 @@ static inline void vfat_d_version_set(struct dentry *dentry,
 static int vfat_revalidate_shortname(struct dentry *dentry)
 {
 	int ret = 1;
+
 	spin_lock(&dentry->d_lock);
 	if (vfat_d_version(dentry) != d_inode(dentry->d_parent)->i_version)
 		ret = 0;
@@ -68,6 +69,9 @@ static int vfat_revalidate_ci(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	/*
 	 * This is not negative dentry. Always valid.
 	 *
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 24967382a7b1..883215127bcd 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -186,6 +186,9 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
 	struct fuse_inode *fi;
 	int ret;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode_rcu(entry);
 	if (inode && is_bad_inode(inode))
 		goto invalid;
diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index 5173b98ca036..8cd906abf424 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -47,6 +47,9 @@ static int gfs2_drevalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	parent = dget_parent(dentry);
 	sdp = GFS2_SB(d_inode(parent));
 	dip = GFS2_I(d_inode(parent));
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 89d1dc19340b..8f0aad50c3b3 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -562,6 +562,9 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	/* Always perform fresh lookup for negatives */
 	if (d_really_is_negative(dentry))
 		goto out_bad_unlocked;
diff --git a/fs/namei.c b/fs/namei.c
index e90680a3f6f1..cc1d472749b1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -809,10 +809,10 @@ static int complete_walk(struct nameidata *nd)
 	if (likely(!(nd->flags & LOOKUP_JUMPED)))
 		return 0;
 
-	if (likely(!(dentry->d_flags & DCACHE_OP_WEAK_REVALIDATE)))
+	if (likely(!(dentry->d_flags & DCACHE_OP_REVALIDATE)))
 		return 0;
 
-	status = dentry->d_op->d_weak_revalidate(dentry, nd->flags);
+	status = dentry->d_op->d_revalidate(dentry, nd->flags);
 	if (status > 0)
 		return 0;
 
diff --git a/fs/ncpfs/dir.c b/fs/ncpfs/dir.c
index b5ec1d980dc9..fb78808b45e2 100644
--- a/fs/ncpfs/dir.c
+++ b/fs/ncpfs/dir.c
@@ -323,6 +323,9 @@ ncp_lookup_validate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	parent = dget_parent(dentry);
 	dir = d_inode(parent);
 
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index fc349577526f..ffe76746cce8 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1273,7 +1273,6 @@ static void nfs_d_release(struct dentry *dentry)
 
 const struct dentry_operations nfs_dentry_operations = {
 	.d_revalidate	= nfs_lookup_revalidate,
-	.d_weak_revalidate	= nfs_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,
@@ -1352,7 +1351,6 @@ static int nfs4_lookup_revalidate(struct dentry *, unsigned int);
 
 const struct dentry_operations nfs4_dentry_operations = {
 	.d_revalidate	= nfs4_lookup_revalidate,
-	.d_weak_revalidate	= nfs4_lookup_revalidate,
 	.d_delete	= nfs_dentry_delete,
 	.d_iput		= nfs_dentry_iput,
 	.d_automount	= nfs_d_automount,
diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c
index 290373024d9d..2ca92974f0b3 100644
--- a/fs/ocfs2/dcache.c
+++ b/fs/ocfs2/dcache.c
@@ -57,6 +57,9 @@ static int ocfs2_dentry_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode(dentry);
 	osb = OCFS2_SB(dentry->d_sb);
 
diff --git a/fs/orangefs/dcache.c b/fs/orangefs/dcache.c
index ae782df5c063..64d5f462ffc2 100644
--- a/fs/orangefs/dcache.c
+++ b/fs/orangefs/dcache.c
@@ -103,6 +103,9 @@ static int orangefs_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	gossip_debug(GOSSIP_DCACHE_DEBUG, "%s: called on dentry %p.\n",
 		     __func__, dentry);
 
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f5738e96a052..6824ac56927f 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -138,24 +138,6 @@ static int ovl_dentry_revalidate(struct dentry *dentry, unsigned int flags)
 	return 1;
 }
 
-static int ovl_dentry_weak_revalidate(struct dentry *dentry, unsigned int flags)
-{
-	struct ovl_entry *oe = dentry->d_fsdata;
-	unsigned int i;
-	int ret = 1;
-
-	for (i = 0; i < oe->numlower; i++) {
-		struct dentry *d = oe->lowerstack[i].dentry;
-
-		if (d->d_flags & DCACHE_OP_WEAK_REVALIDATE) {
-			ret = d->d_op->d_weak_revalidate(d, flags);
-			if (ret <= 0)
-				break;
-		}
-	}
-	return ret;
-}
-
 static const struct dentry_operations ovl_dentry_operations = {
 	.d_release = ovl_dentry_release,
 	.d_real = ovl_d_real,
@@ -165,7 +147,6 @@ static const struct dentry_operations ovl_reval_dentry_operations = {
 	.d_release = ovl_dentry_release,
 	.d_real = ovl_d_real,
 	.d_revalidate = ovl_dentry_revalidate,
-	.d_weak_revalidate = ovl_dentry_weak_revalidate,
 };
 
 static struct kmem_cache *ovl_inode_cachep;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index b9b239fa5cfd..131501b5a35b 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -78,8 +78,7 @@ struct ovl_entry *ovl_alloc_entry(unsigned int numlower)
 bool ovl_dentry_remote(struct dentry *dentry)
 {
 	return dentry->d_flags &
-		(DCACHE_OP_REVALIDATE | DCACHE_OP_WEAK_REVALIDATE |
-		 DCACHE_OP_REAL);
+		(DCACHE_OP_REVALIDATE | DCACHE_OP_REAL);
 }
 
 bool ovl_dentry_weird(struct dentry *dentry)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 9d357b2ea6cb..10709224efca 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1926,6 +1926,9 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode(dentry);
 	task = get_proc_task(inode);
 	if (!task)
diff --git a/fs/proc/fd.c b/fs/proc/fd.c
index 96fc70225e54..68ec359c8161 100644
--- a/fs/proc/fd.c
+++ b/fs/proc/fd.c
@@ -91,6 +91,9 @@ static int tid_fd_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	if (flags & LOOKUP_JUMPED)
+		return 1;
+
 	inode = d_inode(dentry);
 	task = get_proc_task(inode);
 	fd = proc_fd(inode);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f05a659cdf34..5da4c2ed5f3b 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -135,7 +135,6 @@ enum dentry_d_lock_class
 
 struct dentry_operations {
 	int (*d_revalidate)(struct dentry *, unsigned int);
-	int (*d_weak_revalidate)(struct dentry *, unsigned int);
 	int (*d_hash)(const struct dentry *, struct qstr *);
 	int (*d_compare)(const struct dentry *,
 			unsigned int, const char *, const struct qstr *);
@@ -184,8 +183,6 @@ struct dentry_operations {
 #define DCACHE_GENOCIDE			0x00000200
 #define DCACHE_SHRINK_LIST		0x00000400
 
-#define DCACHE_OP_WEAK_REVALIDATE	0x00000800
-
 #define DCACHE_NFSFS_RENAMED		0x00001000
      /* this dentry has been "silly renamed" and has to be deleted on the last
       * dput() */
-- 
2.14.0.rc0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-11-10  0:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09  7:20 [PATCH 0/3] VFS: name lookup improvements NeilBrown
2017-11-09  7:20 ` [PATCH 1/3] VFS/nfs/9p: revise meaning of d_weak_invalidate NeilBrown
2017-11-09  7:20 ` [PATCH 2/3] VFS: remove user_path_mountpoint_at() NeilBrown
2017-11-09  7:20 ` [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint() NeilBrown
2017-11-09 20:06   ` Linus Torvalds
2017-11-09 12:32 ` [PATCH 0/3] VFS: name lookup improvements Jeff Layton
2017-11-09 21:06   ` NeilBrown
2017-11-10  0:21     ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).