lkml.org 
[lkml]   [2009]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item()
    From
    Date
    On Thu, 2008-12-18 at 14:58 -0800, Joel Becker wrote:
    > On Thu, Dec 18, 2008 at 01:28:28PM +0100, Peter Zijlstra wrote:
    > > In fact, both (configfs) mkdir and rmdir seem to synchronize on
    > > su_mutex..
    > >
    > > mkdir B/C/bar
    > >
    > > C.i_mutex
    > > su_mutex
    > >
    > > vs
    > >
    > > rmdir foo
    > >
    > > parent(foo).i_mutex
    > > foo.i_mutex
    > > su_mutex
    > >
    > >
    > > once holding the rmdir su_mutex you can check foo's user-content, since
    > > any mkdir will be blocked. All you have to do is then re-validate in
    > > mkdir's su_mutex that !IS_DEADDIR(C).
    >
    > We explicitly do not take any i_mutex locks after taking
    > su_mutex. That's an ABBA risk. su_mutex protects the hierarchy of
    > config_items. i_mutex protects the vfs view thereof.

    I don't think I was suggesting that. All you need is to serialize any
    mkdir/creat against the rmdir of the youngest non-default group, and you
    can do that by holding su_mutex.

    In rmdir, you already own all the i_mutex instances you need to uncouple
    the whole tree, all you need to do is validate that its indeed empty --
    you don't need i_mutex's for that, because you're holding su_mutex, and
    any concurrent mkdir/creat will be blocking on that.

    If you find it empty, just mark everybody DEAD, drop su_mutex and
    decouple. All concurrent mkdir/creat thingies that were blocking will
    now bail because their parent is found DEAD.

    > If you look in mkdir, we take su_mutex, get a new item from the
    > client subsystem, then drop su_mutex.

    All you need to do before dropping su_mutex again is checking
    IS_DEADDIR(), if so, you just fail the whole mkdir() no extra i_mutex's
    needed.

    > After that, we go about building
    > our filesystem structure, using i_mutex where appropriate.

    Sure, but its ok to grow the default groups non-atomically, right? mkdir
    will only need to check that everything is empty in as far as it has
    been linked, and ensure the not yet linked entries won't be.

    > More
    > importantly is rmdir(2), where we use i_mutex in
    > configfs_detach_group(), but are not holding su_sem. Only when
    > configfs_detach_group() has successfully returned and we have torn down
    > the filesystem structure do we take su_mutex and tear down the
    > config_item structure.

    The only thing that matters is that you can hold su_mutex inside
    i_mutex.


    configfs_rmdir( "foo" )
    {
    /* we hold i_mutex for foo and its parent */

    mutex_lock(&subsys->su_mutex);
    if (default_tree_empty())
    mark_default_tree_dead();
    else
    ret = -EBUSY;
    mutex_unlock(&subsys->su_mutex);

    if (ret)
    return ret;

    /* do actual unlink foo */
    }


    configfs_mkdir( "B/A/bar" )
    {
    /* we hold i_mutex for A */

    mutex_lock(&subsys->su_mutex);
    if (IS_DEADDIR(A))
    ret = -EINVAL; /* or whatever */

    /* increase A's use count, so default_tree_empty() will fail. *
    inc_A_or_subsys_use_count();
    mutex_unlock(&subsys->su_mutex);
    if (ret)
    return ret;

    /* do actual mkdir */
    }


    Surely something along these lines ought to work?



    \
     
     \ /
      Last update: 2009-01-26 13:33    [W:4.108 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site