All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [git pull] vfs and fs fixes
Date: Wed, 18 Apr 2012 00:44:24 +0100	[thread overview]
Message-ID: <20120417234423.GY6589@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFwdh_QWG-R2FQ71kDXiNYZ04qPANBsY_PssVUwEBH4uSw@mail.gmail.com>

On Tue, Apr 17, 2012 at 03:08:26PM -0700, Linus Torvalds wrote:
> > Or I could increment that counter for all the conflicting operations and
> > rely on it instead of the i_mutex. ?I was trying to avoid adding
> > something like that (an inc, a dec, another error path) to every
> > operation. ?And hoping to avoid adding another field to struct inode.
> > Oh well.
> 
> We could just say that we can do a double inode lock, but then
> standardize on the order. And the only sane order is comparing inode
> pointers, not inode numbers like ext4 apparently does.
> 
> With a standard order, I don't think it would be at all wrong to just
> take the inode lock on rename.

In principle, yes, but have you tried to grep for i_mutex?  Note that
we have *another* place where multiple ->i_mutex might be held on
non-directories (and unless I'm missing something, ext4 move_extent.c
stuff doesn't play well with it): quota writes.  Which can, AFAICS,
happen while write(2) is holding ->i_mutex on a regular file.  So
it's not _that_ easy - we want something like "and quota file is goes
last", since there we don't get to change the locking order - the first
->i_mutex is taken too far outside.

I really don't like how messy i_mutex had become these days.  Right now
I'm staring at 700-odd lines all over the place where it's taken/released
and it's a wastebucket lock - used to protect random bits and scraps, with a
lot of filesystems, etc. using it for purposes of their own ;-/

  reply	other threads:[~2012-04-17 23:44 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-17  5:25 [git pull] vfs and fs fixes Al Viro
2012-04-17 15:01 ` Linus Torvalds
2012-04-17 16:22   ` J. Bruce Fields
2012-04-17 16:33     ` Linus Torvalds
2012-04-17 16:33       ` Linus Torvalds
2012-04-17 17:06       ` J. Bruce Fields
2012-04-17 17:06         ` J. Bruce Fields
2012-04-17 17:59       ` Al Viro
2012-04-17 18:01   ` Al Viro
2012-04-17 18:28     ` Al Viro
2012-04-17 21:14       ` J. Bruce Fields
2012-04-17 22:08         ` Linus Torvalds
2012-04-17 23:44           ` Al Viro [this message]
2012-04-18  0:49             ` J. Bruce Fields
2012-04-18  0:56             ` Linus Torvalds
2012-04-18 21:52             ` J. Bruce Fields
2012-04-25 15:20               ` J. Bruce Fields
2012-04-25 15:22               ` [PATCH 1/5] vfs: fix outdated i_mutex_lock_class documentation bfields
2012-04-25 15:22               ` [PATCH 2/5] vfs: pull ext4's double-i_mutex-locking into common code bfields
2012-04-25 15:22               ` [PATCH 3/5] vfs: don't use PARENT/CHILD lock classes for non-directories bfields
2012-04-25 15:22               ` [PATCH 4/5] vfs: take i_mutex on renamed file bfields
2012-04-25 15:22               ` [PATCH 5/5] vfs: change nondirectory i_mutex ordering to fix quota deadlock bfields
2012-04-25 15:28                 ` J. Bruce Fields
2012-04-25 19:53                   ` Jan Kara
2012-04-25 19:58                     ` J. Bruce Fields
2012-04-20 11:15             ` [git pull] vfs and fs fixes Jan Kara
2012-04-24 19:52               ` J. Bruce Fields
2012-04-24 22:23                 ` Jan Kara
2012-04-25 11:29                   ` J. Bruce Fields
2012-04-25 16:26                     ` Jan Kara
2012-04-25 16:47                       ` Steven Whitehouse
2012-04-25 17:11                       ` J. Bruce Fields
2012-04-18  0:47           ` J. Bruce Fields
2012-04-19  3:23 ` Benjamin Herrenschmidt
2012-04-19 14:50   ` Ted Ts'o
2012-04-24 17:40     ` Greg KH
2012-04-24 17:45       ` Al Viro
2012-04-24 17:59         ` Greg KH
2012-04-24 18:04           ` Al Viro
2012-04-24 20:37             ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2013-09-18 22:52 Al Viro

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=20120417234423.GY6589@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=bfields@fieldses.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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.