We've been hitting random -ENOENT's with stress testing btrfs, and it turns out
it's because we're clearing the DCACHE_NEED_LOOKUP flag too early. We clear it
before we actually have done the lookup, so there is a short period where a
process doing a lookup can come upon the stub dentry still on the hash list and
get it, and then have the flag cleared before it checks for it and will return a
dentry with no d_inode, thus resulting in a -ENOENT. So instead of allowing the
fs to clear DCACHE_NEED_LOOKUP, instead make it be cleared in __d_instantiate
when we set the d_inode. This way anybody doing the rcu lookup will have to be
refreshed because we do the rcu seq barrier on the dentry and it will get the
inode. With this patch the reproducer no longer reproduces the problem.
Thanks,
Signed-off-by: Josef Bacik <josef@xxxxxxxxxx>
---
fs/btrfs/inode.c | 1 -
fs/dcache.c | 29 ++++-------------------------
include/linux/dcache.h | 2 --
3 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index d13ad70..8128cba 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3959,7 +3959,6 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
memcpy(&location, dentry->d_fsdata, sizeof(struct btrfs_key));
kfree(dentry->d_fsdata);
dentry->d_fsdata = NULL;
- d_clear_need_lookup(dentry);
} else {
ret = btrfs_inode_by_name(dir, dentry, &location);
}
diff --git a/fs/dcache.c b/fs/dcache.c
index a88948b..d9101b6 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -355,24 +355,6 @@ void d_drop(struct dentry *dentry)
EXPORT_SYMBOL(d_drop);
/*
- * d_clear_need_lookup - drop a dentry from cache and clear the need lookup flag
- * @dentry: dentry to drop
- *
- * This is called when we do a lookup on a placeholder dentry that needed to be
- * looked up. The dentry should have been hashed in order for it to be found by
- * the lookup code, but now needs to be unhashed while we do the actual lookup
- * and clear the DCACHE_NEED_LOOKUP flag.
- */
-void d_clear_need_lookup(struct dentry *dentry)
-{
- spin_lock(&dentry->d_lock);
- __d_drop(dentry);
- dentry->d_flags &= ~DCACHE_NEED_LOOKUP;
- spin_unlock(&dentry->d_lock);
-}
-EXPORT_SYMBOL(d_clear_need_lookup);
-
-/*
* Finish off a dentry we've decided to kill.
* dentry->d_lock must be held, returns with it unlocked.
* If ref is non-zero, then decrement the refcount too.
@@ -1296,6 +1278,10 @@ static void __d_instantiate(struct dentry *dentry, struct inode *inode)
list_add(&dentry->d_alias, &inode->i_dentry);
}
dentry->d_inode = inode;
+ if (d_need_lookup(dentry)) {
+ __d_drop(dentry);
+ dentry->d_flags &= ~DCACHE_NEED_LOOKUP;
+ }
dentry_rcuwalk_barrier(dentry);
spin_unlock(&dentry->d_lock);
fsnotify_d_instantiate(dentry, inode);
@@ -1623,13 +1609,6 @@ struct dentry *d_add_ci(struct dentry *dentry, struct inode *inode,
}
/*
- * We are going to instantiate this dentry, unhash it and clear the
- * lookup flag so we can do that.
- */
- if (unlikely(d_need_lookup(found)))
- d_clear_need_lookup(found);
-
- /*
* Negative dentry: instantiate it unless the inode is a directory and
* already has a dentry.
*/
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 62157c0..6e91882 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -418,8 +418,6 @@ static inline bool d_need_lookup(struct dentry *dentry)
return dentry->d_flags & DCACHE_NEED_LOOKUP;
}
-extern void d_clear_need_lookup(struct dentry *dentry);
-
extern int sysctl_vfs_cache_pressure;
#endif /* __LINUX_DCACHE_H */
--
1.7.5.2
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
