Re: [patch 29/52] fs: icache lock i_count

[Theodore Tso]

On Jul 6, 2010, at 12:34 AM, Nick Piggin wrote:
> On Tue, Jul 06, 2010 at 08:41:06AM +1000, Dave Chinner wrote:
>> 
>> 
>> I don't disagree with this approach - I object to the fact that you
>> repurpose an existing lock and change it's locking rules to "rule
>> the inode". We don't have any one lock that "rules the inode",
>> anyway, so adding a new "i_list_lock" for the new VFS level locking
>> strategies makes it a lot more self-contained. Fundamentally I'm
>> less concerned about the additional memory usage than I am about
>> having landmines planted around i_lock...
> 
> If some filesystem introduces a lock ordering problem from not
> reading the newly added documentation, lockdep should catch it pretty
> quick.

I assume you mean inline documentation in the source, because I
quickly scanned the source and couldn't find any significant changes
to any files in Documentation.

It would be nice if the new state of affairs is documented in a single file,
so that people who want to understand this new locking system don't
have to go crawling through the code, or searching mailing list archives
to figure out what's going on.

A lot of the text in this mail thread, including your discussion of the new
locking hierarchy, and why things are the way they are, would be good
fodder for a new documentation file.   And if you don't want to rename
i_lock, because no better name can be found, we should at least
document that starting as of 2.6.35/36 the meaning of i_lock changed.

-- Ted

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