lkml.org 
[lkml]   [2009]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [patch 09/33] fs: dcache scale dentry refcount
    From
    On Fri, Sep 4, 2009 at 2:51 AM, <npiggin@suse.de> wrote:
    > Make d_count non-atomic and protect it with d_lock. This allows us to
    > ensure a 0 refcount dentry remains 0 without dcache_lock. It is also
    > fairly natural when we start protecting many other dentry members with
    > d_lock.

    > +struct dentry *dget_parent(struct dentry *dentry)
    > +{
    > +       struct dentry *ret;
    > +
    > +repeat:
    > +       spin_lock(&dentry->d_lock);
    > +       ret = dentry->d_parent;
    > +       if (!ret)
    > +               goto out;
    > +       if (!spin_trylock(&ret->d_lock)) {
    > +               spin_unlock(&dentry->d_lock);
    > +               goto repeat;
    > +       }
    > +       BUG_ON(!ret->d_count);
    > +       ret->d_count++;
    > +       spin_unlock(&ret->d_lock);
    > +out:
    > +       spin_unlock(&dentry->d_lock);
    > +       return ret;
    > +}
    > +EXPORT_SYMBOL(dget_parent);


    > Index: linux-2.6/fs/notify/inotify/inotify.c
    > ===================================================================
    > --- linux-2.6.orig/fs/notify/inotify/inotify.c
    > +++ linux-2.6/fs/notify/inotify/inotify.c
    > @@ -339,18 +339,26 @@ void inotify_dentry_parent_queue_event(s
    >        if (!(dentry->d_flags & DCACHE_INOTIFY_PARENT_WATCHED))
    >                return;
    >
    > +again:
    >        spin_lock(&dentry->d_lock);
    >        parent = dentry->d_parent;
    > +       if (!spin_trylock(&parent->d_lock)) {
    > +               spin_unlock(&dentry->d_lock);
    > +               goto again;
    > +       }
    >        inode = parent->d_inode;
    >
    >        if (inotify_inode_watched(inode)) {
    > -               dget(parent);
    > +               dget_dlock(parent);
    >                spin_unlock(&dentry->d_lock);
    > +               spin_unlock(&parent->d_lock);
    >                inotify_inode_queue_event(inode, mask, cookie, name,
    >                                          dentry->d_inode);
    >                dput(parent);
    > -       } else
    > +       } else {
    >                spin_unlock(&dentry->d_lock);
    > +               spin_unlock(&parent->d_lock);
    > +       }

    I don't think I understand why in both of these cases you don't need
    to check for dentry->d_parent == dentry

    > Index: linux-2.6/fs/notify/fsnotify.c
    > ===================================================================
    > --- linux-2.6.orig/fs/notify/fsnotify.c
    > +++ linux-2.6/fs/notify/fsnotify.c
    > @@ -87,13 +87,18 @@ void __fsnotify_parent(struct dentry *de
    >        if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
    >                return;
    >
    > +again:
    >        spin_lock(&dentry->d_lock);
    >        parent = dentry->d_parent;
    > +       if (parent != dentry && !spin_trylock(&parent->d_lock)) {
    > +               spin_unlock(&dentry->d_lock);
    > +               goto again;
    > +       }
    >        p_inode = parent->d_inode;
    >
    >        if (fsnotify_inode_watches_children(p_inode)) {
    >                if (p_inode->i_fsnotify_mask & mask) {
    > -                       dget(parent);
    > +                       dget_dlock(parent);
    >                        send = true;
    >                }
    >        } else {

    And yet in this case we do check for dentry->d_parent == dentry. (my
    unknowing self thinks we'd want to check in all places)

    -Eric
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-09-06 20:03    [W:0.027 / U:125.404 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site