xfs: make xfs_buf_read_map return an error code
authorDarrick J. Wong <darrick.wong@oracle.com>
Fri, 24 Jan 2020 01:01:16 +0000 (17:01 -0800)
committerDarrick J. Wong <darrick.wong@oracle.com>
Sun, 26 Jan 2020 22:32:26 +0000 (14:32 -0800)
Convert xfs_buf_read_map() to return numeric error codes like most
everywhere else in xfs.  This involves moving the open-coded logic that
reports metadata IO read / corruption errors and stales the buffer into
xfs_buf_read_map so that the logic is all in one place.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
fs/xfs/libxfs/xfs_alloc.c
fs/xfs/libxfs/xfs_attr_remote.c
fs/xfs/xfs_buf.c
fs/xfs/xfs_buf.h
fs/xfs/xfs_log_recover.c
fs/xfs/xfs_symlink.c
fs/xfs/xfs_trans_buf.c

index fc93fd88ec89f825ead506fe2787f90d742d1a62..4cc10aa43edfc5a6ba0bfb93587fd67ec2b9600d 100644 (file)
@@ -2956,14 +2956,17 @@ xfs_read_agf(
        trace_xfs_read_agf(mp, agno);
 
        ASSERT(agno != NULLAGNUMBER);
-       error = xfs_trans_read_buf(
-                       mp, tp, mp->m_ddev_targp,
+       error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
                        XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
                        XFS_FSS_TO_BB(mp, 1), flags, bpp, &xfs_agf_buf_ops);
+       /*
+        * Callers of xfs_read_agf() currently interpret a NULL bpp as EAGAIN
+        * and need to be converted to check for EAGAIN specifically.
+        */
+       if (error == -EAGAIN)
+               return 0;
        if (error)
                return error;
-       if (!*bpp)
-               return 0;
 
        ASSERT(!(*bpp)->b_error);
        xfs_buf_set_ref(*bpp, XFS_AGF_REF);
index a266d05df1469ef5a72b68b3e08e623ebf2a23fe..88e50e90443647ec0ea6384b1e04ecf483cb523a 100644 (file)
@@ -422,16 +422,6 @@ xfs_attr_rmtval_get(
                                        &xfs_attr3_rmt_buf_ops);
                        if (!bp)
                                return -ENOMEM;
-                       error = bp->b_error;
-                       if (error) {
-                               xfs_buf_ioerror_alert(bp, __func__);
-                               xfs_buf_relse(bp);
-
-                               /* bad CRC means corrupted metadata */
-                               if (error == -EFSBADCRC)
-                                       error = -EFSCORRUPTED;
-                               return error;
-                       }
 
                        error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
                                                        &offset, &valuelen,
index 5c07b4a7002695910c94843c2fd75bfd8aa978bd..871abaabff3d6607a53602a95e545c88010e17da 100644 (file)
@@ -796,47 +796,76 @@ xfs_buf_reverify(
        return bp->b_error;
 }
 
-xfs_buf_t *
+int
 xfs_buf_read_map(
        struct xfs_buftarg      *target,
        struct xfs_buf_map      *map,
        int                     nmaps,
        xfs_buf_flags_t         flags,
+       struct xfs_buf          **bpp,
        const struct xfs_buf_ops *ops)
 {
        struct xfs_buf          *bp;
        int                     error;
 
        flags |= XBF_READ;
+       *bpp = NULL;
 
        error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
        if (error)
-               return NULL;
+               return error;
 
        trace_xfs_buf_read(bp, flags, _RET_IP_);
 
        if (!(bp->b_flags & XBF_DONE)) {
+               /* Initiate the buffer read and wait. */
                XFS_STATS_INC(target->bt_mount, xb_get_read);
                bp->b_ops = ops;
-               _xfs_buf_read(bp, flags);
-               return bp;
+               error = _xfs_buf_read(bp, flags);
+
+               /* Readahead iodone already dropped the buffer, so exit. */
+               if (flags & XBF_ASYNC)
+                       return 0;
+       } else {
+               /* Buffer already read; all we need to do is check it. */
+               error = xfs_buf_reverify(bp, ops);
+
+               /* Readahead already finished; drop the buffer and exit. */
+               if (flags & XBF_ASYNC) {
+                       xfs_buf_relse(bp);
+                       return 0;
+               }
+
+               /* We do not want read in the flags */
+               bp->b_flags &= ~XBF_READ;
+               ASSERT(bp->b_ops != NULL || ops == NULL);
        }
 
-       xfs_buf_reverify(bp, ops);
+       /*
+        * 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 (error) {
+               if (!XFS_FORCED_SHUTDOWN(target->bt_mount))
+                       xfs_buf_ioerror_alert(bp, __func__);
 
-       if (flags & XBF_ASYNC) {
-               /*
-                * Read ahead call which is already satisfied,
-                * drop the buffer
-                */
+               bp->b_flags &= ~XBF_DONE;
+               xfs_buf_stale(bp);
                xfs_buf_relse(bp);
-               return NULL;
+
+               /* bad CRC means corrupted metadata */
+               if (error == -EFSBADCRC)
+                       error = -EFSCORRUPTED;
+               return error;
        }
 
-       /* We do not want read in the flags */
-       bp->b_flags &= ~XBF_READ;
-       ASSERT(bp->b_ops != NULL || ops == NULL);
-       return bp;
+       *bpp = bp;
+       return 0;
 }
 
 /*
@@ -850,11 +879,13 @@ xfs_buf_readahead_map(
        int                     nmaps,
        const struct xfs_buf_ops *ops)
 {
+       struct xfs_buf          *bp;
+
        if (bdi_read_congested(target->bt_bdev->bd_bdi))
                return;
 
        xfs_buf_read_map(target, map, nmaps,
-                    XBF_TRYLOCK|XBF_ASYNC|XBF_READ_AHEAD, ops);
+                    XBF_TRYLOCK | XBF_ASYNC | XBF_READ_AHEAD, &bp, ops);
 }
 
 /*
index 25dd2aa4322b1cde7faf1feeff076c95e14808ef..f58147354b0220d206b3415505f92dfbbd089321 100644 (file)
@@ -194,10 +194,9 @@ struct xfs_buf *xfs_buf_incore(struct xfs_buftarg *target,
 
 int xfs_buf_get_map(struct xfs_buftarg *target, struct xfs_buf_map *map,
                int nmaps, xfs_buf_flags_t flags, struct xfs_buf **bpp);
-struct xfs_buf *xfs_buf_read_map(struct xfs_buftarg *target,
-                              struct xfs_buf_map *map, int nmaps,
-                              xfs_buf_flags_t flags,
-                              const struct xfs_buf_ops *ops);
+int xfs_buf_read_map(struct xfs_buftarg *target, struct xfs_buf_map *map,
+               int nmaps, xfs_buf_flags_t flags, struct xfs_buf **bpp,
+               const struct xfs_buf_ops *ops);
 void xfs_buf_readahead_map(struct xfs_buftarg *target,
                               struct xfs_buf_map *map, int nmaps,
                               const struct xfs_buf_ops *ops);
@@ -226,8 +225,14 @@ xfs_buf_read(
        xfs_buf_flags_t         flags,
        const struct xfs_buf_ops *ops)
 {
+       struct xfs_buf          *bp;
+       int                     error;
        DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
-       return xfs_buf_read_map(target, &map, 1, flags, ops);
+
+       error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
+       if (error)
+               return NULL;
+       return bp;
 }
 
 static inline void
index 0d683fb963966443a3b626be0f1617f20766dba8..c805a02f0078b4b4a12396fd9a7d2c98337c17d9 100644 (file)
@@ -2749,11 +2749,6 @@ xlog_recover_buffer_pass2(
                          buf_flags, NULL);
        if (!bp)
                return -ENOMEM;
-       error = bp->b_error;
-       if (error) {
-               xfs_buf_ioerror_alert(bp, "xlog_recover_do..(read#1)");
-               goto out_release;
-       }
 
        /*
         * Recover the buffer only if we get an LSN from it and it's less than
@@ -2956,11 +2951,6 @@ xlog_recover_inode_pass2(
                error = -ENOMEM;
                goto error;
        }
-       error = bp->b_error;
-       if (error) {
-               xfs_buf_ioerror_alert(bp, "xlog_recover_do..(read#2)");
-               goto out_release;
-       }
        ASSERT(in_f->ilf_fields & XFS_ILOG_CORE);
        dip = xfs_buf_offset(bp, in_f->ilf_boffset);
 
index a25502bc2071c2b9d8b730259d81bac459bbb169..b255a393a73b49b6e60400c83bdb0fafa0eb8e77 100644 (file)
@@ -57,16 +57,6 @@ xfs_readlink_bmap_ilocked(
                                  &xfs_symlink_buf_ops);
                if (!bp)
                        return -ENOMEM;
-               error = bp->b_error;
-               if (error) {
-                       xfs_buf_ioerror_alert(bp, __func__);
-                       xfs_buf_relse(bp);
-
-                       /* bad CRC means corrupted metadata */
-                       if (error == -EFSBADCRC)
-                               error = -EFSCORRUPTED;
-                       goto out;
-               }
                byte_cnt = XFS_SYMLINK_BUF_SPACE(mp, byte_cnt);
                if (pathlen < byte_cnt)
                        byte_cnt = pathlen;
index 288333fef13adc5f3b4bcd4b5559427136af0b39..cdb66c661425c2385ac030a0160ba27926b0a9d2 100644 (file)
@@ -302,36 +302,16 @@ xfs_trans_read_buf_map(
                return 0;
        }
 
-       bp = xfs_buf_read_map(target, map, nmaps, flags, ops);
-       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);
-
+       error = xfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
+       switch (error) {
+       case 0:
+               break;
+       default:
                if (tp && (tp->t_flags & XFS_TRANS_DIRTY))
                        xfs_force_shutdown(tp->t_mountp, SHUTDOWN_META_IO_ERROR);
-               xfs_buf_relse(bp);
-
-               /* bad CRC means corrupted metadata */
-               if (error == -EFSBADCRC)
-                       error = -EFSCORRUPTED;
+               /* fall through */
+       case -ENOMEM:
+       case -EAGAIN:
                return error;
        }