[Top] [All Lists]

Re: [PATCH] xfs: fix race while discarding buffers [V4]

To: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix race while discarding buffers [V4]
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 21 Aug 2012 09:21:58 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1344621711-8049-1-git-send-email-cmaiolino@xxxxxxxxxx>
References: <1344621711-8049-1-git-send-email-cmaiolino@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Aug 10, 2012 at 03:01:51PM -0300, Carlos Maiolino wrote:
> While xfs_buftarg_shrink() is freeing buffers from the dispose list (filled 
> with
> buffers from lru list), there is a possibility to have xfs_buf_stale() racing
> with it, and removing buffers from dispose list before xfs_buftarg_shrink() 
> does
> it.
> This happens because xfs_buftarg_shrink() handle the dispose list without
> locking and the test condition in xfs_buf_stale() checks for the buffer being 
> in
> *any* list:
> if (!list_empty(&bp->b_lru)
> If the buffer happens to be on dispose list, this causes the buffer counter of
> lru list (btp->bt_lru_nr) to be decremented twice (once in 
> xfs_buftarg_shrink()
> and another in xfs_buf_stale()) causing a wrong account usage of the lru list.
> This may cause xfs_buftarg_shrink() to return a wrong value to the memory
> shrinker shrink_slab(), and such account error may also cause an underflowed
> value to be returned; since the counter is lower than the current number of
> items in the lru list, a decrement may happen when the counter is 0, causing
> an underflow on the counter.
> The fix uses a new flag field (and a new buffer flag) to serialize buffer
> handling during the shrink process. The new flag field has been designed to 
> use
> btp->bt_lru_lock/unlock instead of xfs_buf_lock/unlock mechanism.
> dchinner, sandeen, aquini and aris also deserve credits for this.
> Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>

Looks good.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>