Messages in this thread | | | Date | Wed, 6 Oct 2010 09:30:22 +1100 | From | Dave Chinner <> | Subject | Re: [PATCH 06/17] fs: icache lock lru/writeback lists |
| |
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@suse.de> > > > > 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@fromorbit.com
| |