On Tue, Dec 02, 2014 at 09:34:50AM +1100, Dave Chinner wrote:
> 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.
Should we add an ASSERT that would trigger when someone tries
to issue readahead on a buffer with b_transp set?
> 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;
Can you fix the inconsistent return for the trylock case in a follow on
patch? This difference doesn't look intentional to me, and I would
be surprised if it's correctly handled in the callers.
> }
> +
> 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);
The old non-tp case did a XFS_BUF_UNDONE, which you open code here,
while the with-tp case did a XFS_BUF_DONE. I think this change needs
a little explanation.
> #ifdef DEBUG
> - if (xfs_do_error && !(tp->t_flags & XFS_TRANS_DIRTY)) {
> + if (xfs_do_error && (!tp || !(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!");
> + xfs_debug(mp, "Returning error!");
> return -EIO;
> }
> }
I would suggest to kill this xfs_do_error error code, it's the last
use of the never initialized xfs_do_error and xfs_error_target
variables.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
