[RFC,v4,12/69] teach handle_mounts() to handle RCU mode

Commit Message

Al Viro March 13, 2020, 11:53 p.m. UTC

From: Al Viro <[e-mail]>

... and make the callers of __follow_mount_rcu() use handle_mounts().

Signed-off-by: Al Viro <[e-mail]>
---
 fs/namei.c | 46 +++++++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 29 deletions(-)

Comments

Linus Torvalds March 14, 2020, 12:28 a.m. UTC | #1

Oh, and here you accidentally fix the problem I pointed out about
patch 11, as you move the code:

On Fri, Mar 13, 2020 at 4:54 PM Al Viro <[e-mail]> wrote:
>
> +               if (unlikely(!*inode))
> +                       return -ENOENT;

Correct test added.

> -                       if (unlikely(!inode))
> -                               return -ENOENT;

Incorrect test removed.

And again, maybe I'm misreading the patch. But it does look like it's
wrong in the middle of the series, which would make bisection if
there's some related bug "interesting".

                Linus

Al Viro March 14, 2020, 1 a.m. UTC | #2

On Fri, Mar 13, 2020 at 05:28:12PM -0700, Linus Torvalds wrote:
> Oh, and here you accidentally fix the problem I pointed out about
> patch 11, as you move the code:
> 
> On Fri, Mar 13, 2020 at 4:54 PM Al Viro <[e-mail]> wrote:
> >
> > +               if (unlikely(!*inode))
> > +                       return -ENOENT;
> 
> Correct test added.
> 
> > -                       if (unlikely(!inode))
> > -                               return -ENOENT;
> 
> Incorrect test removed.
> 
> And again, maybe I'm misreading the patch. But it does look like it's
> wrong in the middle of the series, which would make bisection if
> there's some related bug "interesting".

Bisect hazard on botched reordering, actually.  Fixed (IOW, that should've
been if (!*inode) already in #11).

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 494f4959e790..37b2f82e6f2c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1393,6 +1393,18 @@  static inline int handle_mounts(struct nameidata *nd, struct dentry *dentry,
 
 	path->mnt = nd->path.mnt;
 	path->dentry = dentry;
+	if (nd->flags & LOOKUP_RCU) {
+		unsigned int seq = *seqp;
+		if (unlikely(!*inode))
+			return -ENOENT;
+		if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
+			return 1;
+		if (unlazy_child(nd, dentry, seq))
+			return -ECHILD;
+		// *path might've been clobbered by __follow_mount_rcu()
+		path->mnt = nd->path.mnt;
+		path->dentry = dentry;
+	}
 	ret = follow_managed(path, nd);
 	if (likely(ret >= 0)) {
 		*inode = d_backing_inode(path->dentry);
@@ -1620,7 +1632,6 @@  static int lookup_fast(struct nameidata *nd,
 		       struct path *path, struct inode **inode,
 		       unsigned *seqp)
 {
-	struct vfsmount *mnt = nd->path.mnt;
 	struct dentry *dentry, *parent = nd->path.dentry;
 	int status = 1;
 
@@ -1658,21 +1669,8 @@  static int lookup_fast(struct nameidata *nd,
 
 		*seqp = seq;
 		status = d_revalidate(dentry, nd->flags);
-		if (likely(status > 0)) {
-			/*
-			 * Note: do negative dentry check after revalidation in
-			 * case that drops it.
-			 */
-			if (unlikely(!inode))
-				return -ENOENT;
-			path->mnt = mnt;
-			path->dentry = dentry;
-			if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
-				return 1;
-			if (unlazy_child(nd, dentry, seq))
-				return -ECHILD;
+		if (likely(status > 0))
 			return handle_mounts(nd, dentry, path, inode, seqp);
-		}
 		if (unlazy_child(nd, dentry, seq))
 			return -ECHILD;
 		if (unlikely(status == -ECHILD))
@@ -2361,21 +2359,11 @@  static int handle_lookup_down(struct nameidata *nd)
 	unsigned seq = nd->seq;
 	int err;
 
-	if (nd->flags & LOOKUP_RCU) {
-		/*
-		 * don't bother with unlazy_walk on failure - we are
-		 * at the very beginning of walk, so we lose nothing
-		 * if we simply redo everything in non-RCU mode
-		 */
-		path = nd->path;
-		if (unlikely(!__follow_mount_rcu(nd, &path, &inode, &seq)))
-			return -ECHILD;
-	} else {
+	if (!(nd->flags & LOOKUP_RCU))
 		dget(nd->path.dentry);
-		err = handle_mounts(nd, nd->path.dentry, &path, &inode, &seq);
-		if (unlikely(err < 0))
-			return err;
-	}
+	err = handle_mounts(nd, nd->path.dentry, &path, &inode, &seq);
+	if (unlikely(err < 0))
+		return err;
 	path_to_nameidata(&path, nd);
 	nd->inode = inode;
 	nd->seq = seq;