lkml.org 
[lkml]   [2018]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 317/366] VFS: Impose ordering on accesses of d_inode and d_flags
    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)
    \
     
     \ /
      Last update: 2018-11-11 21:35    [W:2.615 / U:1.344 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site