[git,pull] vfs fixes

Commit Message

Al Viro April 3, 2017, 12:43 a.m. UTC

On Mon, Apr 03, 2017 at 01:30:45AM +0100, Al Viro wrote:

> Currently true and almost certainly will remain so.  Point taken, what you
> are suggesting is better.  Actually, the invariant to watch for is
> "no d_can_lookup() withtout DCACHE_RCUACCESS" and that we can trivially
> enforce by one-liner change in d_flags_for_inode() -
> s/DCACHE_DIRECTORY_TYPE/& | DCACHE_RCUACCESS/
> 
> OK...

Do you have any objections against the following (still untested) variant?
I don't see any point in checking for flags & LOOKUP_RCU in case of !*s -
flags is in register at that point, so...

Comments

Linus Torvalds April 3, 2017, 12:58 a.m. UTC | #1

On Sun, Apr 2, 2017 at 5:43 PM, Al Viro <[e-mail]> wrote:
>
> Do you have any objections against the following (still untested) variant?
> I don't see any point in checking for flags & LOOKUP_RCU in case of !*s -
> flags is in register at that point, so...

Looks sane to me.

I had to go and double-check that "DCACHE_DIRECTORY_TYPE" is what
d_can_lookup() actually checks, so _that_ part is perhaps a bit
subtle, and might be worth noting in that comment that you edited.

So the real "rule" ends up being that we only ever look up things from
dentries of type DCACHE_DIRECTORY_TYPE set, and those had better have
DCACHE_RCUACCESS bit set.

And the only reason path_init() only checks it for that case is that
nd->root and nd->pwd both have to be of type d_can_lookup().

Do we check that when we set it? I hope/assume we do.

                      Linus

Patch

diff --git a/fs/dcache.c b/fs/dcache.c
index 95d71eda8142..05550139a8a6 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1757,7 +1757,13 @@  static unsigned d_flags_for_inode(struct inode *inode)
 		return DCACHE_MISS_TYPE;
 
 	if (S_ISDIR(inode->i_mode)) {
-		add_flags = DCACHE_DIRECTORY_TYPE;
+		/*
+		 * Any potential starting point of lookup should have
+		 * DCACHE_RCUACCESS; currently directory dentries
+		 * come from d_alloc() anyway, but it costs us nothing
+		 * to enforce it here.
+		 */
+		add_flags = DCACHE_DIRECTORY_TYPE | DCACHE_RCUACCESS;
 		if (unlikely(!(inode->i_opflags & IOP_LOOKUP))) {
 			if (unlikely(!inode->i_op->lookup))
 				add_flags = DCACHE_AUTODIR_TYPE;
diff --git a/fs/namei.c b/fs/namei.c
index d41fab78798b..19dcf62133cc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2145,6 +2145,9 @@  static const char *path_init(struct nameidata *nd, unsigned flags)
 	int retval = 0;
 	const char *s = nd->name->name;
 
+	if (!*s)
+		flags &= ~LOOKUP_RCU;
+
 	nd->last_type = LAST_ROOT; /* if there are only slashes... */
 	nd->flags = flags | LOOKUP_JUMPED | LOOKUP_PARENT;
 	nd->depth = 0;