nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

xfs: lobotomise xfs_trans_read_buf_map()

Browse source 
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: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
This commit is contained in:
Dave Chinner 2014-12-04 09:43:13 +11:00 committed by Dave Chinner
parent cdc9cec7c0
commit 2d3d0c53df
1 changed files with 32 additions and 101 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

133
fs/xfs/xfs_trans_buf.c
View file

@ -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.
*/
bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
if (bp != NULL) {
if (tp)
bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
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;
ASSERT(bp->b_flags & XBF_DONE);
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;
}
}
/*
* 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;
if (!XFS_FORCED_SHUTDOWN(mp))
xfs_buf_ioerror_alert(bp, __func__);
bp->b_flags &= ~XBF_DONE;
xfs_buf_stale(bp);
XFS_BUF_DONE(bp);
xfs_buf_ioerror_alert(bp, __func__);
if (tp->t_flags & XFS_TRANS_DIRTY)
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);
xfs_buf_relse(bp);
xfs_debug(mp, "Returning trans error!");
return -EIO;
}
}
if (XFS_FORCED_SHUTDOWN(mp)) {
xfs_buf_relse(bp);
trace_xfs_trans_read_buf_shut(bp, _RET_IP_);
return -EIO;
}
#endif
if (XFS_FORCED_SHUTDOWN(mp))
goto shutdown_abort;
_xfs_trans_bjoin(tp, bp, 1);
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;
}
/*