From: Dave Chinner <david@fromorbit.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
Nick Piggin <npiggin@kernel.dk>
Subject: Re: fs: break out inode operations from inode_lock V4
Date: Fri, 29 Oct 2010 20:34:21 +1100 [thread overview]
Message-ID: <20101029093421.GJ2715@dastard> (raw)
In-Reply-To: <20101029092930.GR19804@ZenIV.linux.org.uk>
On Fri, Oct 29, 2010 at 10:29:30AM +0100, Al Viro wrote:
> On Fri, Oct 29, 2010 at 07:59:55PM +1100, Dave Chinner wrote:
> > Hi Al,
> >
> > Another update to the inode_lock splitting patch set. It's still
> > based on your merge-stem branch. I'm going to be out all weekend, so
> > any further changes will take a couple of days to turn around.
> >
> > Version 4:
> > - whitespace cleanup
> > - moved setting state on new inodes till after the hash search fails
> > in insert_inode_locked
> > - made hash insert operations atomic with state changes by holding
> > inode->i_lock while doing hash inserts
> > - made inode hash removals atomic with state changes by taking the
> > inode_lock (later inode_hash_lock) and inode->i_lock. Combined
> > with the insert changes, this means the inode_unhashed check in
> > ->drop_inode is safely protected by just holding the
> > inode->i_lock.
> > - protect inode_unhashed() checks in insert_inode_locked with
> > inode->i_lock
>
> The last one is not needed at all; look at what's getting done there - we
> drop that ->i_lock immediately after the check, so it doesn't buy us anything.
> The stuff before that *is* a race fix; namely, the race with BS iget()
> triggered by nfsd. This check is just verifying that it was a race and not
> a badly confused filesystem. IOW, no need to lock anything and no _point_
> locking anything. We are repeating the hash walk anyway; this is just making
> sure that we hadn't run into infinite retries.
Oh, I forgot to remove that from the changelog - I removed the
locking from the code, though....
> Other than that I'm OK with that set; could you add "lift ->i_lock from
> the beginning of writeback_single_inode()" to the series and post your
> current RCU-for-i_hash patch for review?
It'll be Monday before I get to this. It's way past beer o'clock
here....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2010-10-29 9:34 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-29 8:59 fs: break out inode operations from inode_lock V4 Dave Chinner
2010-10-29 8:59 ` [PATCH 1/8] fs: protect inode->i_state with inode->i_lock Dave Chinner
2010-10-29 8:59 ` [PATCH 2/8] fs: factor inode disposal Dave Chinner
2010-10-29 8:59 ` [PATCH 3/8] fs: Lock the inode LRU list separately Dave Chinner
2010-10-29 8:59 ` [PATCH 4/8] fs: remove inode_lock from iput_final and prune_icache Dave Chinner
2010-10-29 9:00 ` [PATCH 5/8] fs: move i_sb_list out from under inode_lock Dave Chinner
2010-10-29 9:00 ` [PATCH 6/8] fs: move i_wb_list " Dave Chinner
2010-10-29 9:43 ` Christoph Hellwig
2010-10-29 9:48 ` Al Viro
2010-10-29 9:00 ` [PATCH 7/8] fs: rename inode_lock to inode_hash_lock Dave Chinner
2010-10-29 9:00 ` [PATCH 8/8] fs: Clean up documentation references to inode_lock Dave Chinner
2010-10-29 9:29 ` fs: break out inode operations from inode_lock V4 Al Viro
2010-10-29 9:34 ` Dave Chinner [this message]
2010-11-09 3:43 ` Nick Piggin
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=20101029093421.GJ2715@dastard \
--to=david@fromorbit.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@kernel.dk \
--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