lkml.org 
[lkml]   [2009]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
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 http://marc.info/?l=linux-kernel&m=121438776626316&w=2
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
http://marc.info/?l=linux-kernel&m=122962334723834&w=2 does not turn off
lockdep!). Of course it's better if someone can just do this rework :)

Thanks,

Louis

--
Dr Louis Rilling Kerlabs
Skype: louis.rilling Batiment Germanium
Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
http://www.kerlabs.com/ 35700 Rennes[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2009-01-26 15:59    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site