All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, cluster-devel@redhat.com,
	swhiteho <swhiteho@redhat.com>
Subject: Re: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item()
Date: Tue, 27 Jan 2009 19:05:18 -0800	[thread overview]
Message-ID: <20090128030518.GB7244@mail.oracle.com> (raw)
In-Reply-To: <20090126145536.GG7532@hawkmoon.kerlabs.com>

Thank you both for keeping up on this.  I owe Louis some review that I'm
going to try to get to.

On Mon, Jan 26, 2009 at 03:55:36PM +0100, Louis Rilling wrote:
> 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?

	The "appearance" of the entire default group hierarchy should be
atomic.  When mkdir(2) returns, it is all there.  This does make our
lives a little difficult inside configfs, but it makes everyone else's
lives much easier.

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

	That is true - it works, and safely, and the lockdep conflict is
the only real known issue.  I'm not wary of changing it, I'm only wary
of breaking it.  That is, I want to understand what is being changed and
be sure that we're keeping the correctness we have so far.  I don't want
to change it and "hope" we got it right, you know?  This makes me
cautious, but don't think I'm against change.  As I stated, having
lockdep work for us is a good thing.

	More replies to the other mails coming.

Joel

-- 

You can use a screwdriver to screw in screws or to clean your ears,
however, the latter needs real skill, determination and a lack of fear
of injuring yourself.  It is much the same with JavaScript.
	- Chris Heilmann

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

WARNING: multiple messages have this Message-ID (diff)
From: Joel Becker <Joel.Becker@oracle.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] Re: [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item()
Date: Tue, 27 Jan 2009 19:05:18 -0800	[thread overview]
Message-ID: <20090128030518.GB7244@mail.oracle.com> (raw)
In-Reply-To: <20090126145536.GG7532@hawkmoon.kerlabs.com>

Thank you both for keeping up on this.  I owe Louis some review that I'm
going to try to get to.

On Mon, Jan 26, 2009 at 03:55:36PM +0100, Louis Rilling wrote:
> 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?

	The "appearance" of the entire default group hierarchy should be
atomic.  When mkdir(2) returns, it is all there.  This does make our
lives a little difficult inside configfs, but it makes everyone else's
lives much easier.

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

	That is true - it works, and safely, and the lockdep conflict is
the only real known issue.  I'm not wary of changing it, I'm only wary
of breaking it.  That is, I want to understand what is being changed and
be sure that we're keeping the correctness we have so far.  I don't want
to change it and "hope" we got it right, you know?  This makes me
cautious, but don't think I'm against change.  As I stated, having
lockdep work for us is a good thing.

	More replies to the other mails coming.

Joel

-- 

You can use a screwdriver to screw in screws or to clean your ears,
however, the latter needs real skill, determination and a lack of fear
of injuring yourself.  It is much the same with JavaScript.
	- Chris Heilmann

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127



  reply	other threads:[~2009-01-28  3:09 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-11 14:20 configfs, dlm_controld & lockdep Steven Whitehouse
2008-12-11 14:20 ` [Cluster-devel] " Steven Whitehouse
2008-12-11 14:44 ` Louis Rilling
2008-12-11 17:34   ` Joel Becker
2008-12-11 17:34     ` [Cluster-devel] " Joel Becker
2008-12-12 10:06     ` Louis Rilling
2008-12-12 15:29       ` [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item() Louis Rilling
2008-12-17 21:40         ` Andrew Morton
2008-12-17 21:40           ` [Cluster-devel] " Andrew Morton
2008-12-17 22:03           ` Joel Becker
2008-12-17 22:03             ` [Cluster-devel] " Joel Becker
2008-12-17 22:09             ` Andrew Morton
2008-12-17 22:09               ` [Cluster-devel] " Andrew Morton
2008-12-18  7:26           ` Peter Zijlstra
2008-12-18  9:27             ` Joel Becker
2008-12-18  9:27               ` [Cluster-devel] " Joel Becker
2008-12-18 11:15               ` Louis Rilling
2008-12-18 18:00                 ` Make lockdep happy with configfs Louis Rilling
2009-01-26 11:51                   ` Louis Rilling
2009-01-28  3:44                     ` Joel Becker
2009-01-28  3:44                       ` [Cluster-devel] " Joel Becker
2008-12-18 18:00                 ` [PATCH 1/2] configfs: Silence lockdep on mkdir() and rmdir() Louis Rilling
2009-01-28  3:55                   ` Joel Becker
2009-01-28  3:55                     ` [Cluster-devel] " Joel Becker
2009-01-28 10:38                     ` Louis Rilling
2008-12-18 18:00                 ` [PATCH 2/2] configfs: Rework configfs_depend_item() locking and make lockdep happy Louis Rilling
2009-01-28  4:13                   ` Joel Becker
2009-01-28  4:13                     ` [Cluster-devel] " Joel Becker
2009-01-28 10:32                     ` Louis Rilling
2008-12-18 11:26               ` [PATCH] configfs: Silence lockdep on mkdir(), rmdir() and configfs_depend_item() Steven Whitehouse
2008-12-18 11:26                 ` [Cluster-devel] " Steven Whitehouse
2008-12-18 11:48                 ` Louis Rilling
2008-12-18 11:56               ` Peter Zijlstra
2008-12-18 12:28                 ` Peter Zijlstra
2008-12-18 22:58                   ` Joel Becker
2008-12-18 22:58                     ` [Cluster-devel] " Joel Becker
2008-12-19 10:29                     ` Louis Rilling
2009-01-26 12:30                     ` Peter Zijlstra
2009-01-26 13:24                       ` Louis Rilling
2009-01-26 13:41                         ` Peter Zijlstra
2009-01-26 14:00                           ` Louis Rilling
2009-01-26 14:19                             ` Peter Zijlstra
2009-01-26 14:55                               ` Louis Rilling
2009-01-28  3:05                                 ` Joel Becker [this message]
2009-01-28  3:05                                   ` [Cluster-devel] " Joel Becker
2009-01-28  3:41                       ` Joel Becker
2009-01-28  3:41                         ` [Cluster-devel] " Joel Becker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090128030518.GB7244@mail.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=cluster-devel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=swhiteho@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.