[PATCH 05/14] libxfs: make libxfs_buf_read_map return an error code

Darrick J. Wong
2020-02-20T01:45:08Z

Make libxfs_buf_read_map() and libxfs_readbuf() return an error code
instead of making callers guess what happened based on whether or not
they got a buffer back.

Add a new SALVAGE flag so that certain utilities (xfs_db and xfs_repair)
can attempt salvage operations even if the verifiers failed, which was
the behavior before this change.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
db/io.c | 4 +--
libxfs/libxfs_io.h | 25 ++++++++++++------
libxfs/rdwr.c | 72 ++++++++++++++++++++++++++++++++++++++++++----------
libxfs/trans.c | 24 ++++-------------
repair/da_util.c | 3 +-
5 files changed, 84 insertions(+), 44 deletions(-)

diff --git a/db/io.c b/db/io.c
index b81e9969..5c9d72bb 100644
--- a/db/io.c
+++ b/db/io.c
@@ -542,8 +542,8 @@ set_cur(
if (!iocur_top->bbmap)
return;
memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap));
- bp = libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b,
- bbmap->nmaps, 0, ops);
+ libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, bbmap->nmaps,
+ LIBXFS_READBUF_SALVAGE, &bp, ops);
} else {
bp = libxfs_buf_read(mp->m_ddev_targp, blknum, len, 0, ops);
iocur_top->bbmap = NULL;
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 189db8d8..06847ad5 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -126,6 +126,8 @@ extern struct cache_operations libxfs_bcache_operations;

/* Exit on buffer read error */
#define LIBXFS_READBUF_FAIL_EXIT (1 << 0)
+/* Return the buffer even if the IO or verifier fail. */
+#define LIBXFS_READBUF_SALVAGE (1 << 1)

/* Exit on buffer write error */
#define LIBXFS_WRITEBUF_FAIL_EXIT (1 << 0)
@@ -135,9 +137,9 @@ extern struct cache_operations libxfs_bcache_operations;
#define libxfs_buf_read(dev, daddr, len, flags, ops) \
libxfs_trace_readbuf(__FUNCTION__, __FILE__, __LINE__, \
(dev), (daddr), (len), (flags), (ops))
-#define libxfs_buf_read_map(dev, map, nmaps, flags, ops) \
+#define libxfs_buf_read_map(dev, map, nmaps, flags, bpp, ops) \
libxfs_trace_readbuf_map(__FUNCTION__, __FILE__, __LINE__, \
- (dev), (map), (nmaps), (flags), (ops))
+ (dev), (map), (nmaps), (flags), (bpp), (ops))
#define libxfs_buf_dirty(buf, flags) \
libxfs_trace_dirtybuf(__FUNCTION__, __FILE__, __LINE__, \
(buf), (flags))
@@ -153,9 +155,10 @@ extern struct cache_operations libxfs_bcache_operations;
struct xfs_buf *libxfs_trace_readbuf(const char *func, const char *file,
int line, struct xfs_buftarg *btp, xfs_daddr_t daddr,
size_t len, int flags, const struct xfs_buf_ops *ops);
-extern xfs_buf_t *libxfs_trace_readbuf_map(const char *, const char *, int,
- struct xfs_buftarg *, struct xfs_buf_map *, int, int,
- const struct xfs_buf_ops *);
+int libxfs_trace_readbuf_map(const char *func, const char *file, int line,
+ struct xfs_buftarg *btp, struct xfs_buf_map *maps,
+ int nmaps, int flags, struct xfs_buf **bpp,
+ const struct xfs_buf_ops *ops);
void libxfs_trace_dirtybuf(const char *func, const char *file, int line,
struct xfs_buf *bp, int flags);
struct xfs_buf *libxfs_trace_getbuf(const char *func, const char *file,
@@ -169,8 +172,9 @@ extern void libxfs_trace_putbuf (const char *, const char *, int,

#else

-extern xfs_buf_t *libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
- int, int, const struct xfs_buf_ops *);
+int libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
+ int nmaps, int flags, struct xfs_buf **bpp,
+ const struct xfs_buf_ops *ops);
void libxfs_buf_dirty(struct xfs_buf *bp, int flags);
int libxfs_buf_get_map(struct xfs_buftarg *btp, struct xfs_buf_map *maps,
int nmaps, int flags, struct xfs_buf **bpp);
@@ -200,9 +204,14 @@ libxfs_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 libxfs_buf_read_map(target, &map, 1, flags, ops);
+ error = libxfs_buf_read_map(target, &map, 1, flags, &bp, ops);
+ if (error)
+ return NULL;
+ return bp;
}

#endif /* XFS_BUF_TRACING */
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 6eaa3e69..222ef441 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -389,8 +389,10 @@ libxfs_log_header(
#undef libxfs_writebuf
#undef libxfs_buf_get_map

-xfs_buf_t *libxfs_buf_read_map(struct xfs_buftarg *, struct xfs_buf_map *,
- int, int, const struct xfs_buf_ops *);
+int libxfs_buf_read_map(struct xfs_buftarg *btp,
+ struct xfs_buf_map *maps, int nmaps, int flags,
+ struct xfs_buf **bpp,
+ const struct xfs_buf_ops *ops);
int libxfs_writebuf(xfs_buf_t *, int);
int libxfs_buf_get_map(struct xfs_buftarg *btp,
struct xfs_buf_map *maps, int nmaps, int flags,
@@ -420,11 +422,30 @@ libxfs_trace_readbuf(
struct xfs_buf *bp;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);

- bp = libxfs_buf_read_map(btp, &map, 1, flags, ops);
+ libxfs_buf_read_map(btp, &map, 1, flags, &bp, ops);
__add_trace(bp, func, file, line);
return bp;
}

+int
+libxfs_trace_readbuf_map(
+ const char *func,
+ const char *file,
+ int line,
+ struct xfs_buftarg *btp,
+ struct xfs_buf_map *map,
+ int nmaps,
+ int flags,
+ struct xfs_buf **bpp,
+ const struct xfs_buf_ops *ops)
+{
+ int error;
+
+ error = libxfs_buf_read_map(btp, map, nmaps, flags, bpp, ops);
+ __add_trace(*bpp, func, file, line);
+ return error;
+}
+
void
libxfs_trace_dirtybuf(
const char *func,
@@ -1004,20 +1025,27 @@ libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
return error;
}

-struct xfs_buf *
-libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
- int flags, const struct xfs_buf_ops *ops)
+int
+libxfs_buf_read_map(
+ struct xfs_buftarg *btp,
+ struct xfs_buf_map *map,
+ int nmaps,
+ int flags,
+ struct xfs_buf **bpp,
+ const struct xfs_buf_ops *ops)
{
- struct xfs_buf *bp;
- int error = 0;
+ struct xfs_buf *bp;
+ bool salvage = flags & LIBXFS_READBUF_SALVAGE;
+ int error = 0;

+ *bpp = NULL;
if (nmaps == 1)
error = libxfs_getbuf_flags(btp, map[0].bm_bn, map[0].bm_len,
0, &bp);
else
error = __libxfs_buf_get_map(btp, map, nmaps, 0, &bp);
if (error)
- return NULL;
+ return error;

/*
* if the buffer was prefetched, it is likely that it was not validated.
@@ -1030,12 +1058,17 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
* should not be dirtying unchecked buffers and therefore failing it
* here because it's dirty and unchecked indicates we've screwed up
* somewhere else.
+ *
+ * Note that if the caller passes in LIBXFS_READBUF_SALVAGE, that means
+ * they want the buffer even if it contains error status.
*/
bp->b_error = 0;
if (bp->b_flags & (LIBXFS_B_UPTODATE | LIBXFS_B_DIRTY)) {
if (bp->b_flags & LIBXFS_B_UNCHECKED)
- libxfs_readbuf_verify(bp, ops);
- return bp;
+ error = libxfs_readbuf_verify(bp, ops);
+ if (error && !salvage)
+ goto err;
+ goto ok;
}

/*
@@ -1050,15 +1083,26 @@ libxfs_buf_read_map(struct xfs_buftarg *btp, struct xfs_buf_map *map, int nmaps,
flags);
else
error = libxfs_readbufr_map(btp, bp, flags);
- if (!error)
- libxfs_readbuf_verify(bp, ops);
+ if (error == -EIO && salvage)
+ goto ok;
+ if (error)
+ goto err;
+
+ error = libxfs_readbuf_verify(bp, ops);
+ if (error && !salvage)
+ goto err;

+ok:
#ifdef IO_DEBUGX
printf("%lx: %s: read %lu bytes, error %d, blkno=%llu(%llu), %p\n",
pthread_self(), __FUNCTION__, buf - (char *)bp->b_addr, error,
(long long)LIBXFS_BBTOOFF64(bp->b_bn), (long long)bp->b_bn, bp);
#endif
- return bp;
+ *bpp = bp;
+ return 0;
+err:
+ libxfs_buf_relse(bp);
+ return error;
}

/* Allocate a raw uncached buffer. */
diff --git a/libxfs/trans.c b/libxfs/trans.c
index a7407dc3..b833e369 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -510,15 +510,8 @@ libxfs_trans_read_buf_map(

*bpp = NULL;

- if (tp == NULL) {
- bp = libxfs_buf_read_map(target, map, nmaps, flags, ops);
- if (!bp) {
- return (flags & XBF_TRYLOCK) ? -EAGAIN : -ENOMEM;
- }
- if (bp->b_error)
- goto out_relse;
- goto done;
- }
+ if (tp == NULL)
+ return libxfs_buf_read_map(target, map, nmaps, flags, bpp, ops);

bp = xfs_trans_buf_item_match(tp, target, map, nmaps);
if (bp) {
@@ -530,22 +523,15 @@ libxfs_trans_read_buf_map(
goto done;
}

- bp = libxfs_buf_read_map(target, map, nmaps, flags, ops);
- if (!bp) {
- return (flags & XBF_TRYLOCK) ? -EAGAIN : -ENOMEM;
- }
- if (bp->b_error)
- goto out_relse;
+ error = libxfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
+ if (error)
+ return error;

_libxfs_trans_bjoin(tp, bp, 1);
done:
trace_xfs_trans_read_buf(bp->b_log_item);
*bpp = bp;
return 0;
-out_relse:
- error = bp->b_error;
- xfs_buf_relse(bp);
- return error;
}

/*
diff --git a/repair/da_util.c b/repair/da_util.c
index ed2ec3ba..add93ad8 100644
--- a/repair/da_util.c
+++ b/repair/da_util.c
@@ -64,7 +64,8 @@ da_read_buf(
map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
}
- bp = libxfs_buf_read_map(mp->m_dev, map, nex, 0, ops);
+ libxfs_buf_read_map(mp->m_dev, map, nex, LIBXFS_READBUF_SALVAGE,
+ &bp, ops);
if (map != map_array)
free(map);
return bp;