Re: [PATCH 13/18] fs: split locking of inode writeback and LRU lists

[Nick Piggin <npiggin@kernel.dk>]

On Sat, Oct 16, 2010 at 12:20:21PM -0400, Christoph Hellwig wrote:
> On Sat, Oct 16, 2010 at 06:57:13PM +1100, Nick Piggin wrote:
> > > That needs some documentation both in the changelog and in the code
> > > I think.
> > 
> > This is another instance where the irregular i_lock locking is making
> > these little subtleties to the locking. I think that is actually much
> > worse for maintainence/complexity than a few trylocks which can be
> > mostly removed with rcu anyway (which are obvious because of the well
> > documented lock order).
> 
> Care to explain why?

OK.


>  The I_FREEING and co checks are how we do things
> all over the icache for a long time.

That's missing my point. My point is that the semantics of icache
concurrency here are changed from the old inode_lock model. With
my design, holding i_lock on an inode is equivalent (stronger,
actually) to holding inode_lock which is an important part of
making small correct steps.


>  They are perfectly easy to
> understand concept.

The concept is easy to understand, but catching all the changes
are not necessarily so easy. You said it yourself

 "What does this have to do with the rest of the patch?"

So, I repeat, I never objected to narrowing i_lock widths, but I
would like to do it as very small patches that would move i_lock
out of some data structure manipulation and add the required additions
like this hunk. I want to see what we gain by making the locking model
less regular.