On Tue, Oct 30, 2012 at 11:20:54AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> 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. 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.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_buf_item.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index a8d0ed9..b72fe88 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -526,7 +526,23 @@ xfs_buf_item_unpin(
> }
> xfs_buf_relse(bp);
> } else if (freed && remove) {
> + /*
> + * There are currently two references to the buffer - the active
> + * LRU reference and the buf log item. What we are about to do
> + * here - simulate a failed IO completion - requires 3
> + * references.
> + *
> + * The LRU reference is removed by the xfs_buf_stale() call. The
> + * buf item reference is removed by the xfs_buf_iodone()
> + * callback that is run by xfs_buf_do_callbacks() during ioend
> + * processing (via the bp->b_iodone callback), and then finally
> + * the ioend processing drops the IO reference.
> + *
> + * Hence we need to take an additional reference here so that IO
> + * completion processing doesn't free the buffer prematurely.
> + */
> xfs_buf_lock(bp);
> + xfs_buf_hold(bp);
> xfs_buf_ioerror(bp, EIO);
> XFS_BUF_UNDONE(bp);
> xfs_buf_stale(bp);
> --
> 1.7.10
>
Looks good
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
--
--Carlos
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
