On Wed, Apr 29, 2020 at 01:21:38PM -0400, Brian Foster wrote:
> We use the same buffer I/O failure code in a few different places.
> It's not much code, but it's not necessarily self-explanatory.
> Factor it into a helper and document it in one place.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> Reviewed-by: Allison Collins <allison.henderson@xxxxxxxxxx>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_buf.c | 21 +++++++++++++++++----
> fs/xfs/xfs_buf.h | 1 +
> fs/xfs/xfs_buf_item.c | 21 +++------------------
> fs/xfs/xfs_inode.c | 6 +-----
> 4 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 9ec3eaf1c618..d5d6a68bb1e6 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1248,6 +1248,22 @@ xfs_buf_ioerror_alert(
> -bp->b_error);
> }
>
> +/*
> + * To simulate an I/O failure, the buffer must be locked and held with at least
> + * three references. The LRU reference is dropped by the stale call. The buf
> + * item reference is dropped via ioend processing. The third reference is owned
> + * by the caller and is dropped on I/O completion if the buffer is XBF_ASYNC.
> + */
> +void
> +xfs_buf_ioend_fail(
> + struct xfs_buf *bp)
> +{
> + bp->b_flags &= ~XBF_DONE;
> + xfs_buf_stale(bp);
> + xfs_buf_ioerror(bp, -EIO);
> + xfs_buf_ioend(bp);
> +}
> +
> int
> xfs_bwrite(
> struct xfs_buf *bp)
> @@ -1480,10 +1496,7 @@ __xfs_buf_submit(
>
> /* on shutdown we stale and complete the buffer immediately */
> if (XFS_FORCED_SHUTDOWN(bp->b_mount)) {
> - xfs_buf_ioerror(bp, -EIO);
> - bp->b_flags &= ~XBF_DONE;
> - xfs_buf_stale(bp);
> - xfs_buf_ioend(bp);
> + xfs_buf_ioend_fail(bp);
> return -EIO;
> }
>
> diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
> index 9a04c53c2488..06ea3eef866e 100644
> --- a/fs/xfs/xfs_buf.h
> +++ b/fs/xfs/xfs_buf.h
> @@ -263,6 +263,7 @@ extern void __xfs_buf_ioerror(struct xfs_buf *bp, int error,
> xfs_failaddr_t failaddr);
> #define xfs_buf_ioerror(bp, err) __xfs_buf_ioerror((bp), (err), __this_address)
> extern void xfs_buf_ioerror_alert(struct xfs_buf *bp, xfs_failaddr_t fa);
> +void xfs_buf_ioend_fail(struct xfs_buf *);
>
> extern int __xfs_buf_submit(struct xfs_buf *bp, bool);
> static inline int xfs_buf_submit(struct xfs_buf *bp)
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 8796adde2d12..b452a399a441 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -471,28 +471,13 @@ 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 will drop the IO reference if the buffer
> - * is marked XBF_ASYNC.
> - *
> - * Hence we need to take an additional reference here so that IO
> - * completion processing doesn't free the buffer prematurely.
> + * The buffer must be locked and held by the caller to simulate
> + * an async I/O failure.
> */
> xfs_buf_lock(bp);
> xfs_buf_hold(bp);
> bp->b_flags |= XBF_ASYNC;
> - xfs_buf_ioerror(bp, -EIO);
> - bp->b_flags &= ~XBF_DONE;
> - xfs_buf_stale(bp);
> - xfs_buf_ioend(bp);
> + xfs_buf_ioend_fail(bp);
> }
> }
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index d1772786af29..909ca7c0bac4 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3630,11 +3630,7 @@ xfs_iflush_cluster(
> */
> ASSERT(bp->b_iodone);
> bp->b_flags |= XBF_ASYNC;
> - bp->b_flags &= ~XBF_DONE;
> - xfs_buf_stale(bp);
> - xfs_buf_ioerror(bp, -EIO);
> - xfs_buf_ioend(bp);
> -
> + xfs_buf_ioend_fail(bp);
> xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE);
>
> /* abort the corrupt inode, as it was not attached to the buffer */
> --
> 2.21.1
>
