On Sat, Oct 16, 2010 at 05:02:19AM +1100, Nick Piggin wrote: > For those lookups where you are taking the i_lock anyway, they > will look the same, except the i_lock lock width reduction > loses the ability to lock all icache state of the inode (like > we can practically do today with inode_lock). > > This was a key consideration for maintainability for me. Maybe you've overlooked this point. It is, in fact, very important in my opinion. With my locking approach, everywhere where today we have: spin_lock(&inode_lock); do_something(inode); spin_unlock(&inode_lock); it can be replaced with spin_lock(&inode->i_lock); do_something(inode); spin_unlock(&inode->i_lock); Without worrying about the lock coverage. In fact, it is a tiny bit stronger because you also get to hold the refcount at the same time (doesn't really matter outside core icache though). Ditto for my dcache_lock appraoch (it's far more important there, being much more visible to filesystems IMO, but icache is still important). I never totally objected to reductions in i_lock lock width if they really are required for that last bit of performance, but I have always maintained that I want these kinds of locking irregularities merged on their own, on top of the base code. Especially with RCU inodes, I'm not sure if they'll be needed, however. Most of the slowpaths where that happens, the i_lock needs to be taken somewhere anyway, so you probably don't really save anything. -- 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
