linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	"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: Fri, 20 Apr 2012 13:15:17 +0200	[thread overview]
Message-ID: <20120420111517.GB8985@quack.suse.cz> (raw)
In-Reply-To: <20120417234423.GY6589@ZenIV.linux.org.uk>

On Wed 18-04-12 00:44:24, Al Viro wrote:
> 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.
  Hum, I think I could just do away with quota file i_mutex being special.
It's used for two purposes:
  1) When quota is being turned on/off, we want to set/clear inode immutable
flag, truncate page cache, etc. But we should be able push this locking
outside of quota locks.
  2) Inside filesystems when quota file is written to. Quota writes are
serialized by quota code anyway and noone else has any bussiness with quota
files (they are marked as immutable to avoid mistakes) so there i_mutex is
not really needed.

> 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 ;-/

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  parent reply	other threads:[~2012-04-20 11:15 UTC|newest]

Thread overview: 39+ 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 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
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             ` Jan Kara [this message]
2012-04-24 19:52               ` [git pull] vfs and fs fixes 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=20120420111517.GB8985@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=bfields@fieldses.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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).