Re: fs: break out inode operations from inode_lock V4

From: Dave Chinner
Date: Fri Oct 29 2010 - 05:34:46 EST


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@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/