[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()
    On 26/01/09 15:19 +0100, Peter Zijlstra wrote:
    > On Mon, 2009-01-26 at 15:00 +0100, Louis Rilling wrote:
    > > > Its not a locking correctness thing, but simply not being able to do it
    > > > from the vfs calls because those assume locks held?
    > > >
    > > > Can't you simply punt the work to a worklet once you've created/removed
    > > > the non-default group, which can be done from within the vfs callback ?
    > >
    > > I'm not sure to understand your suggestion. Is this:
    > > 1) for mkdir(), create the non-default group, but without its default groups,
    > > and defer their creation to a worker which won't have constraints on locks held
    > > by any caller;
    > > 2) for rmdir(), unlink the non-default group, but without unlinking its default
    > > groups, and defer the recursive work to a lock-free context?
    > >
    > > For mkdir(), this may work. Maybe a bit confusing for userspace, since mkdir(A)
    > > returns as soon as A is created, but A may be populated later and userspace may
    > > rely on A being populated as soon as it is created (current behavior). As a
    > > configfs user, this makes my life harder...
    > Right, so that is the whole crux of the matter?

    Probably not. I'm not the maintainer of configfs, but I guess that Joel is a bit
    reluctant to deeply rework parts of something that actually works (conflicts
    with lockdep excepted).

    > Initially I understood the whole recursive locking issue to be about
    > having to serialize mkdir vs rmdir so that you would know the default
    > groups to be empty etc.
    > You could create the subtree before you link it in. i_op->mkdir() only
    > has the parent i_mutex held, so you should be able to create your inode,
    > and all default groups (some of who will have the non-default group as
    > parent, but that's ok, as we don't have that locked yet).
    > Once you've constructed this, you could connect the non-default group to
    > its parent.
    > Also, you don't _need_ to have any i_mutex's locked here, because non of
    > these inodes are reachable.

    True. I already suggested this to Joel (while fixing a race condition), but this
    raises other issues (see
    for a previous discussion on this).

    > > For rmdir(), is this safe to unlink a non-empty directory, and to empty it
    > > afterwards? This looks like going back to the unmount problem.
    > Dunno :-), I think it should be safe. The only guarantee you need is
    > that there are no refs to inodes in the decoupled sub-tree (other than
    > your own of course.)
    > So you'd only need to punt the rmdir cleanup to eventd or something.

    May be. Anyway I can't investigate this right now, and that's why I'm asking
    Joel if he is going to accept one of the temporary solutions that I provided
    (Note that my second solution does not turn off
    lockdep!). Of course it's better if someone can just do this rework :)



    Dr Louis Rilling Kerlabs
    Skype: louis.rilling Batiment Germanium
    Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes 35700 Rennes
    [unhandled content-type:application/pgp-signature]
     \ /
      Last update: 2009-01-26 15:59    [W:0.043 / U:61.236 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site