3.16.61-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: David Howells dhowells@redhat.com
commit 4bf46a272647d89e780126b52eda04737defd9f4 upstream.
Impose ordering on accesses of d_inode and d_flags to avoid the need to do this:
if (!dentry->d_inode || d_is_negative(dentry)) {
when this:
if (d_is_negative(dentry)) {
should suffice.
This check is especially problematic if a dentry can have its type field set to something other than DENTRY_MISS_TYPE when d_inode is NULL (as in unionmount).
What we really need to do is stick a write barrier between setting d_inode and setting d_flags and a read barrier between reading d_flags and reading d_inode.
Signed-off-by: David Howells dhowells@redhat.com Signed-off-by: Al Viro viro@zeniv.linux.org.uk [bwh: Backported to 3.16: - Use ACCESS_ONCE() instead of {READ,WRITE}_ONCE() - There's no DCACHE_FALLTHRU flag] Signed-off-by: Ben Hutchings ben@decadent.org.uk --- fs/dcache.c | 47 +++++++++++++++++++++++++++++++++++------- include/linux/dcache.h | 21 +++---------------- 2 files changed, 42 insertions(+), 26 deletions(-)
--- a/fs/dcache.c +++ b/fs/dcache.c @@ -281,6 +281,41 @@ void release_dentry_name_snapshot(struct } EXPORT_SYMBOL(release_dentry_name_snapshot);
+/* + * Make sure other CPUs see the inode attached before the type is set. + */ +static inline void __d_set_inode_and_type(struct dentry *dentry, + struct inode *inode, + unsigned type_flags) +{ + unsigned flags; + + dentry->d_inode = inode; + smp_wmb(); + flags = ACCESS_ONCE(dentry->d_flags); + flags &= ~DCACHE_ENTRY_TYPE; + flags |= type_flags; + ACCESS_ONCE(dentry->d_flags) = flags; +} + +/* + * Ideally, we want to make sure that other CPUs see the flags cleared before + * the inode is detached, but this is really a violation of RCU principles + * since the ordering suggests we should always set inode before flags. + * + * We should instead replace or discard the entire dentry - but that sucks + * performancewise on mass deletion/rename. + */ +static inline void __d_clear_type_and_inode(struct dentry *dentry) +{ + unsigned flags = ACCESS_ONCE(dentry->d_flags); + + flags &= ~DCACHE_ENTRY_TYPE; + ACCESS_ONCE(dentry->d_flags) = flags; + smp_wmb(); + dentry->d_inode = NULL; +} + static void dentry_free(struct dentry *dentry) { WARN_ON(!hlist_unhashed(&dentry->d_u.d_alias)); @@ -317,7 +352,7 @@ static void dentry_iput(struct dentry * { struct inode *inode = dentry->d_inode; if (inode) { - dentry->d_inode = NULL; + __d_clear_type_and_inode(dentry); hlist_del_init(&dentry->d_u.d_alias); spin_unlock(&dentry->d_lock); spin_unlock(&inode->i_lock); @@ -341,8 +376,7 @@ static void dentry_unlink_inode(struct d __releases(dentry->d_inode->i_lock) { struct inode *inode = dentry->d_inode; - __d_clear_type(dentry); - dentry->d_inode = NULL; + __d_clear_type_and_inode(dentry); hlist_del_init(&dentry->d_u.d_alias); dentry_rcuwalk_barrier(dentry); spin_unlock(&dentry->d_lock); @@ -1644,10 +1678,9 @@ static void __d_instantiate(struct dentr unsigned add_flags = d_flags_for_inode(inode);
spin_lock(&dentry->d_lock); - __d_set_type(dentry, add_flags); if (inode) hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry); - dentry->d_inode = inode; + __d_set_inode_and_type(dentry, inode, add_flags); dentry_rcuwalk_barrier(dentry); spin_unlock(&dentry->d_lock); fsnotify_d_instantiate(dentry, inode); @@ -1904,8 +1937,7 @@ struct dentry *d_obtain_alias(struct ino add_flags = d_flags_for_inode(inode) | DCACHE_DISCONNECTED;
spin_lock(&tmp->d_lock); - tmp->d_inode = inode; - tmp->d_flags |= add_flags; + __d_set_inode_and_type(tmp, inode, add_flags); hlist_add_head(&tmp->d_u.d_alias, &inode->i_dentry); hlist_bl_lock(&tmp->d_sb->s_anon); hlist_bl_add_head(&tmp->d_hash, &tmp->d_sb->s_anon); --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -411,26 +411,11 @@ static inline bool d_mountpoint(const st /* * Directory cache entry type accessor functions. */ -static inline void __d_set_type(struct dentry *dentry, unsigned type) -{ - dentry->d_flags = (dentry->d_flags & ~DCACHE_ENTRY_TYPE) | type; -} - -static inline void __d_clear_type(struct dentry *dentry) -{ - __d_set_type(dentry, DCACHE_MISS_TYPE); -} - -static inline void d_set_type(struct dentry *dentry, unsigned type) -{ - spin_lock(&dentry->d_lock); - __d_set_type(dentry, type); - spin_unlock(&dentry->d_lock); -} - static inline unsigned __d_entry_type(const struct dentry *dentry) { - return dentry->d_flags & DCACHE_ENTRY_TYPE; + unsigned type = ACCESS_ONCE(dentry->d_flags); + smp_rmb(); + return type & DCACHE_ENTRY_TYPE; }
static inline bool d_can_lookup(const struct dentry *dentry)