Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2d3d0c53 authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner
Browse files

xfs: lobotomise xfs_trans_read_buf_map() 



There's a case in that code where it checks for a buffer match in a
transaction where the buffer is not marked done. i.e. trying to
catch a buffer we have locked in the transaction but have not
completed IO on.

The only way we can find a buffer that has not had IO completed on
it is if it had readahead issued on it, but we never do readahead on
buffers that we have already joined into a transaction. Hence this
condition cannot occur, and buffers locked and joined into a
transaction should always be marked done and not under IO.

Remove this code and re-order xfs_trans_read_buf_map() to remove
duplicated IO dispatch and error handling code.

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent cdc9cec7

fs/xfs/xfs_trans_buf.c

+33 −102
Original line number Diff line number Diff line
@@ -229,13 +229,6 @@ xfs_trans_getsb(xfs_trans_t *tp, 
	return bp;

}



#ifdef DEBUG

xfs_buftarg_t *xfs_error_target;

int	xfs_do_error;

int	xfs_req_num;

int	xfs_error_mod = 33;

#endif



/*

 * Get and lock the buffer for the caller if it is not already

 * locked within the given transaction.  If it has not yet been
@@ -257,46 +250,11 @@ xfs_trans_read_buf_map( 
	struct xfs_buf		**bpp,

	const struct xfs_buf_ops *ops)

{

	xfs_buf_t		*bp;

	xfs_buf_log_item_t	*bip;

	struct xfs_buf		*bp = NULL;

	struct xfs_buf_log_item	*bip;

	int			error;



	*bpp = NULL;

	if (!tp) {

		bp = xfs_buf_read_map(target, map, nmaps, flags, ops);

		if (!bp)

			return (flags & XBF_TRYLOCK) ?

					-EAGAIN : -ENOMEM;



		if (bp->b_error) {

			error = bp->b_error;

			xfs_buf_ioerror_alert(bp, __func__);

			XFS_BUF_UNDONE(bp);

			xfs_buf_stale(bp);

			xfs_buf_relse(bp);



			/* bad CRC means corrupted metadata */

			if (error == -EFSBADCRC)

				error = -EFSCORRUPTED;

			return error;

		}

#ifdef DEBUG

		if (xfs_do_error) {

			if (xfs_error_target == target) {

				if (((xfs_req_num++) % xfs_error_mod) == 0) {

					xfs_buf_relse(bp);

					xfs_debug(mp, "Returning error!");

					return -EIO;

				}

			}

		}

#endif

		if (XFS_FORCED_SHUTDOWN(mp))

			goto shutdown_abort;

		*bpp = bp;

		return 0;

	}



	/*

	 * If we find the buffer in the cache with this transaction

	 * pointer in its b_fsprivate2 field, then we know we already
@@ -305,49 +263,24 @@ xfs_trans_read_buf_map( 
	 * If the buffer is not yet read in, then we read it in, increment

	 * the lock recursion count, and return it to the caller.

	 */

	if (tp)

		bp = xfs_trans_buf_item_match(tp, target, map, nmaps);

	if (bp != NULL) {

	if (bp) {

		ASSERT(xfs_buf_islocked(bp));

		ASSERT(bp->b_transp == tp);

		ASSERT(bp->b_fspriv != NULL);

		ASSERT(!bp->b_error);

		if (!(XFS_BUF_ISDONE(bp))) {

			trace_xfs_trans_read_buf_io(bp, _RET_IP_);

			ASSERT(!XFS_BUF_ISASYNC(bp));

			ASSERT(bp->b_iodone == NULL);

			XFS_BUF_READ(bp);

			bp->b_ops = ops;



			error = xfs_buf_submit_wait(bp);

			if (error) {

				if (!XFS_FORCED_SHUTDOWN(mp))

					xfs_buf_ioerror_alert(bp, __func__);

				xfs_buf_relse(bp);

				/*

				 * We can gracefully recover from most read

				 * errors. Ones we can't are those that happen

				 * after the transaction's already dirty.

				 */

				if (tp->t_flags & XFS_TRANS_DIRTY)

					xfs_force_shutdown(tp->t_mountp,

							SHUTDOWN_META_IO_ERROR);

				/* bad CRC means corrupted metadata */

				if (error == -EFSBADCRC)

					error = -EFSCORRUPTED;

				return error;

			}

		}

		ASSERT(bp->b_flags & XBF_DONE);



		/*

		 * We never locked this buf ourselves, so we shouldn't

		 * brelse it either. Just get out.

		 */

		if (XFS_FORCED_SHUTDOWN(mp)) {

			trace_xfs_trans_read_buf_shut(bp, _RET_IP_);

			*bpp = NULL;

			return -EIO;

		}





		bip = bp->b_fspriv;

		bip->bli_recur++;
@@ -358,17 +291,29 @@ xfs_trans_read_buf_map( 
	}



	bp = xfs_buf_read_map(target, map, nmaps, flags, ops);

	if (bp == NULL) {

		*bpp = NULL;

		return (flags & XBF_TRYLOCK) ?

					0 : -ENOMEM;

	if (!bp) {

		if (!(flags & XBF_TRYLOCK))

			return -ENOMEM;

		return tp ? 0 : -EAGAIN;

	}



	/*

	 * If we've had a read error, then the contents of the buffer are

	 * invalid and should not be used. To ensure that a followup read tries

	 * to pull the buffer from disk again, we clear the XBF_DONE flag and

	 * mark the buffer stale. This ensures that anyone who has a current

	 * reference to the buffer will interpret it's contents correctly and

	 * future cache lookups will also treat it as an empty, uninitialised

	 * buffer.

	 */

	if (bp->b_error) {

		error = bp->b_error;

		xfs_buf_stale(bp);

		XFS_BUF_DONE(bp);

		if (!XFS_FORCED_SHUTDOWN(mp))

			xfs_buf_ioerror_alert(bp, __func__);

		if (tp->t_flags & XFS_TRANS_DIRTY)

		bp->b_flags &= ~XBF_DONE;

		xfs_buf_stale(bp);



		if (tp && (tp->t_flags & XFS_TRANS_DIRTY))

			xfs_force_shutdown(tp->t_mountp, SHUTDOWN_META_IO_ERROR);

		xfs_buf_relse(bp);
@@ -377,33 +322,19 @@ xfs_trans_read_buf_map( 
			error = -EFSCORRUPTED;

		return error;

	}

#ifdef DEBUG

	if (xfs_do_error && !(tp->t_flags & XFS_TRANS_DIRTY)) {

		if (xfs_error_target == target) {

			if (((xfs_req_num++) % xfs_error_mod) == 0) {

				xfs_force_shutdown(tp->t_mountp,

						   SHUTDOWN_META_IO_ERROR);



	if (XFS_FORCED_SHUTDOWN(mp)) {

		xfs_buf_relse(bp);

				xfs_debug(mp, "Returning trans error!");

		trace_xfs_trans_read_buf_shut(bp, _RET_IP_);

		return -EIO;

	}

		}

	}

#endif

	if (XFS_FORCED_SHUTDOWN(mp))

		goto shutdown_abort;



	if (tp)

		_xfs_trans_bjoin(tp, bp, 1);

	trace_xfs_trans_read_buf(bp->b_fspriv);



	*bpp = bp;

	return 0;



shutdown_abort:

	trace_xfs_trans_read_buf_shut(bp, _RET_IP_);

	xfs_buf_relse(bp);

	*bpp = NULL;

	return -EIO;

}



/*