On Tue, Jul 06, 2010 at 08:41:06AM +1000, Dave Chinner wrote: > On Sat, Jul 03, 2010 at 03:06:52PM +1000, Nick Piggin wrote: > > So it makes a lot of sense to have a lock to rule the inode (as opposed > > to now we have a lock to rule *all* inodes). > > 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", "rule the inode" was in regard to the inode cache / dirty / writeout handling; ie. what inode_lock is currently for. The idea is that in these code paths, we tend to want to take an inode, lock it, and then manipulate it (including putting it on or taking it off data structures). > 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... (it's not just lists, its refcount and i_state too) I just don't see a problem. _No_ filesystem takes any of the locks that nest inside i_lock. They may call some inode.c functions, but shouldn't do so while holding i_lock if they are treating i_lock as an innermost lock. So they can keep using i_lock. I did go through and attempt to look at all filesystems. Within inode.c, lock ordering is 2 levels deep, same as it was when we had inode_lock and i_lock. If some filesystem introduces a lock ordering problem from not reading the newly added documentation, lockdep should catch it pretty quick. So I don't see a landmine they don't have (in much larger doses) with i_mutex, reentrant reclaim, page lock, buffer lock, mmap_sem etc already. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
