Re: [PATCH 06/17] fs: icache lock lru/writeback lists

From: Christoph Hellwig
Date: Fri Oct 01 2010 - 02:01:09 EST


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.

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.

But maybe all that is better left as a separate patch on top of the
current queue.

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