Re: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item()

[Peter Zijlstra <peterz@infradead.org>]

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?

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.

> 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.