ocfs2-devel.oss.oracle.com archive mirror
 help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Louis Rilling <Louis.Rilling@kerlabs.com>
Cc: ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org
Subject: [Ocfs2-devel] [BUG] deadlock between configfs_rmdir() and sys_rename() (WAS	Re: [RFC][PATCH 4/4] configfs: Make multiple default_group)	destructions lockdep friendly
Date: Wed, 11 Jun 2008 14:15:23 -0700	[thread overview]
Message-ID: <20080611211523.GB14754@mail.oracle.com> (raw)
In-Reply-To: <20080611173047.GR18153@localhost>

On Wed, Jun 11, 2008 at 07:30:47PM +0200, Louis Rilling wrote:
> I've spoken too fast. Doing things in d_revalidate() (or even in
> configfs_lookup()) is just too early: after they are called and before
> lock_rename(), nothing can prevent rmdir() from being called.

	Yeah, that's what I was saying.

> I'm afraid that we need a way to get rid of the whole tree locking in
> configfs_detach_prep(). Imagine that we protect all calls to configfs_rmdir()
> and configfs_mkdir() with a global mutex (say configfs_dir_mutex). Is it still
> needed to take default group's i_mutex in configfs_detach_prep() to check for
> the validity of the rmdir()?

	That's not a bad idea.  I was basically sure we need the tree
locking to prevent other VFS operations from happening, but the more I
think about it, the only reason we do the prep locking is to check and
protect the ENOTEMPTY case.  I've bounced it off Mark too, and he
agrees.

> 	After this check, we could set a REMOVING flag in configfs_dirent of all
> default groups (could actually be done by configfs_detach_prep(), making
> configfs_detach_rollback() resetting the flag), release configfs_dir_mutex, and
> then call configfs_detach_group(), with detach_groups() taking default group's
> i_mutex right before calling configfs_detach_group() recursively (this would
> lead to the same path locking as in populate_groups() or
> configfs_depend_prep()).

	We already set the flag - CONFIGFS_USET_DROPPING.  The only
change is that you don't take i_mutex anymore.  For the static mutex,
the name 'configfs_dir_mutex' is fine.

> 	On mkdir() side, once configfs_dir_mutex is acquired, we first check the
> REMOVING flag, and proceed if it is not set.

	And if it is set, we return -ENOENT.

> Just tell me if you think that it's feasible, and I'll send you a patch if it's

	Have at!  Thanks.

Joel

-- 

"Vote early and vote often." 
        - Al Capone

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

      reply	other threads:[~2008-06-11 21:15 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-22 11:40 [Ocfs2-devel] [RFC][PATCH 0/4] configfs: Make nested default groups lockdep-friendly v2 Louis Rilling
2008-05-22 11:40 ` [Ocfs2-devel] [RFC][PATCH 1/4] Prepare i_mutex lockdep subclasses for locking of variable path lengths Louis Rilling
2008-05-22 11:40 ` [Ocfs2-devel] [RFC][PATCH 2/4] Prepare vfs_rmdir() for further nested i_mutex locking Louis Rilling
2008-05-22 11:40 ` [Ocfs2-devel] [RFC][PATCH 3/4] configfs: Make nested default groups creations lockdep-friendly Louis Rilling
2008-05-22 11:40 ` [Ocfs2-devel] [RFC][PATCH 4/4] configfs: Make multiple default_group destructions lockdep friendly Louis Rilling
2008-05-23 16:44   ` Louis Rilling
2008-06-02 23:07     ` Joel Becker
2008-06-03 16:00       ` Louis Rilling
2008-06-06 23:01         ` Joel Becker
2008-06-09 11:03           ` Louis Rilling
2008-06-09 12:54             ` [Ocfs2-devel] [BUG] deadlock between configfs_rmdir() and sys_rename() (WAS Re: [RFC][PATCH 4/4] configfs: Make multiple default_group) " Louis Rilling
2008-06-10  1:58               ` Joel Becker
2008-06-10 10:14                 ` Louis Rilling
2008-06-10 17:36                   ` Joel Becker
2008-06-11 14:10                     ` Louis Rilling
2008-06-11 17:30                       ` Louis Rilling
2008-06-11 21:15                         ` Joel Becker [this message]

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=20080611211523.GB14754@mail.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=Louis.Rilling@kerlabs.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ocfs2-devel@oss.oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).