Re: [PATCH 5/6 V2] xfs: fix buffer shudown reference count mismatch

Mark Tinguely
2012-11-02T13:15:45Z

On 11/01/12 22:23, Dave Chinner wrote:
> xfs: fix buffer shudown reference count mismatch
>
> From: Dave Chinner<dchinner@redhat.com>
>
> When we shut down the filesystem, we have to unpin and free all the
> buffers currently active in the CIL. To do this we unpin and remove
> them in one operation as a result of a failed iclogbuf write. For
> buffers, we do this removal via a simultated IO completion of after
> marking the buffer stale.
>
> At the time we do this, we have two references to the buffer - the
> active LRU reference and the buf log item. The LRU reference is
> removed by marking the buffer stale, and the active CIL reference is
> by the xfs_buf_iodone() callback that is run by
> xfs_buf_do_callbacks() during ioend processing (via the bp->b_iodone
> callback).
>
> However, ioend processing requires one more reference - that of the
> IO that it is completing. We don't have this reference, so we free
> the buffer prematurely and use it after it is freed. For buffers
> marked with XBF_ASYNC, this leads to assert failures in
> xfs_buf_rele() on debug kernels because the b_hold count is zero.
>
> Fix this by making sure we take the necessary IO reference before
> starting IO completion processing on the stale buffer, and set the
> XBF_ASYNC flag to ensure that IO completion processing removes all
> the active references from the buffer to ensure it is fully torn
> down.
>
> Cc:<stable@vger.kernel.org>
> Signed-off-by: Dave Chinner<dchinner@redhat.com>
> ---
> V2 - add XBF_ASYNC to buffers so last reference is always removed.
> Fixes shutdown assert failures due to references from buffers
> remaining the perag structures at unmount.

Yes and thank-you. This eliminates both the b_hold and pag_ref
asserts in xfstest 179.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>