On Fri, Oct 01, 2010 at 02:01:03AM -0400, Christoph Hellwig wrote: > On Wed, Sep 29, 2010 at 10:18:38PM +1000, Dave Chinner wrote: > > From: Nick Piggin <npiggin@xxxxxxx> > > > > The inode moves between different lists protected by the inode_lock. Introduce > > a new lock that protects all of the lists (dirty, unused, in use, etc) that the > > inode will move around as it changes state. As this is mostly a list for > > protecting the writeback lists, name it wb_inode_list_lock and nest all the > > list manipulations in this lock inside the current inode_lock scope. > > As a band-aid to get rid of the inode_lock this might be fine, but I > don't really like it. For one all the list are per-bdi_writeback, so > the lock should be as well. Second the lock is held over far too long > periods during writeback, which leads to a lot of whacky trylock > operations and unlock and sleep cycles inside it. In practice we only > need it in the places where we manipulate the lists. per-bdi writeback lock won't work with the patch set as it stands - it also protects the LRU which is a global list. I'll have to pull back another patch to split the LRU and IO lists to make this lock per-bdi. > Also it feels like it really should nest outside i_lock, not inside it, > but I need to look more deeply to figure why that might not easily be > possible. Yeah, I'm trying to rework the patch series to not nest anything inside i_lock. The more I look at all that trylock stuff, the more my eyes bleed.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- 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