* [PATCH v5 00/12] xfs: make buffer functions return error codes
@ 2020-01-24 5:18 Darrick J. Wong
2020-01-24 5:19 ` [PATCH 01/12] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
` (11 more replies)
0 siblings, 12 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:18 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
Hi all,
Let's fix all the xfs read/get buffer functions to return the usual
integer error codes and pass the buffer pointer as a out-argument. This
makes it so that we can return useful error output instead of making
callers infer ENOMEM or EAGAIN or whatever other reality they crave from
the NULL pointer that they get when things don't go perfectly.
FWIW, all XBF_TRYLOCK callers must now trigger retries if they receive
EAGAIN. This may lead to a slight behavioral change in that TRYLOCK
callers will no longer retry for things like ENOMEM, though I didn't see
any obvious changes in user-visible behavior when running fstests.
After finishing the error code conversion, we straighten out the TRYLOCK
callers to remove all this null pointer checks in favor of explicit
EAGAIN checks; and then we change the buffer IO/corruption error
reporting so that we report whoever called the buffer code even when
reading a buffer in transaction context.
In v5 of this series I rearranged some of the patches so that the return
type conversions go from the lower levels of the buffer code upwards.
This eases some of the awkward warts of previous submissions.
This has been lightly tested with fstests. Enjoy!
Comments and questions are, as always, welcome.
--D
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 01/12] xfs: make xfs_buf_alloc return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:19 ` [PATCH 02/12] xfs: make xfs_buf_get_map " Darrick J. Wong
` (10 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert _xfs_buf_alloc() to return numeric error codes like most
everywhere else in xfs.
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/xfs_buf.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index a0229c368e78..f9a6cf71f4ab 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -198,20 +198,22 @@ xfs_buf_free_maps(
}
}
-static struct xfs_buf *
+static int
_xfs_buf_alloc(
struct xfs_buftarg *target,
struct xfs_buf_map *map,
int nmaps,
- xfs_buf_flags_t flags)
+ xfs_buf_flags_t flags,
+ struct xfs_buf **bpp)
{
struct xfs_buf *bp;
int error;
int i;
+ *bpp = NULL;
bp = kmem_zone_zalloc(xfs_buf_zone, KM_NOFS);
if (unlikely(!bp))
- return NULL;
+ return -ENOMEM;
/*
* We don't want certain flags to appear in b_flags unless they are
@@ -239,7 +241,7 @@ _xfs_buf_alloc(
error = xfs_buf_get_maps(bp, nmaps);
if (error) {
kmem_cache_free(xfs_buf_zone, bp);
- return NULL;
+ return error;
}
bp->b_bn = map[0].bm_bn;
@@ -256,7 +258,8 @@ _xfs_buf_alloc(
XFS_STATS_INC(bp->b_mount, xb_create);
trace_xfs_buf_init(bp, _RET_IP_);
- return bp;
+ *bpp = bp;
+ return 0;
}
/*
@@ -715,8 +718,8 @@ xfs_buf_get_map(
return NULL;
}
- new_bp = _xfs_buf_alloc(target, map, nmaps, flags);
- if (unlikely(!new_bp))
+ error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
+ if (error)
return NULL;
error = xfs_buf_allocate_memory(new_bp, flags);
@@ -917,8 +920,8 @@ xfs_buf_get_uncached(
DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);
/* flags might contain irrelevant bits, pass only what we care about */
- bp = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT);
- if (unlikely(bp == NULL))
+ error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);
+ if (error)
goto fail;
page_count = PAGE_ALIGN(numblks << BBSHIFT) >> PAGE_SHIFT;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 02/12] xfs: make xfs_buf_get_map return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-24 5:19 ` [PATCH 01/12] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 22:02 ` Dave Chinner
2020-01-24 22:59 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 03/12] xfs: make xfs_buf_read_map " Darrick J. Wong
` (9 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_buf_get_map() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_buf.c | 44 ++++++++++++++++----------------------------
fs/xfs/xfs_buf.h | 13 +++++++++----
fs/xfs/xfs_trans_buf.c | 14 +++++++++-----
3 files changed, 34 insertions(+), 37 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index f9a6cf71f4ab..5c07b4a70026 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -685,53 +685,39 @@ xfs_buf_incore(
* cache hits, as metadata intensive workloads will see 3 orders of magnitude
* more hits than misses.
*/
-struct xfs_buf *
+int
xfs_buf_get_map(
struct xfs_buftarg *target,
struct xfs_buf_map *map,
int nmaps,
- xfs_buf_flags_t flags)
+ xfs_buf_flags_t flags,
+ struct xfs_buf **bpp)
{
struct xfs_buf *bp;
struct xfs_buf *new_bp;
int error = 0;
+ *bpp = NULL;
error = xfs_buf_find(target, map, nmaps, flags, NULL, &bp);
-
- switch (error) {
- case 0:
- /* cache hit */
+ if (!error)
goto found;
- case -EAGAIN:
- /* cache hit, trylock failure, caller handles failure */
- ASSERT(flags & XBF_TRYLOCK);
- return NULL;
- case -ENOENT:
- /* cache miss, go for insert */
- break;
- case -EFSCORRUPTED:
- default:
- /*
- * None of the higher layers understand failure types
- * yet, so return NULL to signal a fatal lookup error.
- */
- return NULL;
- }
+ if (error != -ENOENT)
+ return error;
error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
if (error)
- return NULL;
+ return error;
error = xfs_buf_allocate_memory(new_bp, flags);
if (error) {
xfs_buf_free(new_bp);
- return NULL;
+ return error;
}
error = xfs_buf_find(target, map, nmaps, flags, new_bp, &bp);
if (error) {
xfs_buf_free(new_bp);
- return NULL;
+ return error;
}
if (bp != new_bp)
@@ -744,7 +730,7 @@ xfs_buf_get_map(
xfs_warn(target->bt_mount,
"%s: failed to map pagesn", __func__);
xfs_buf_relse(bp);
- return NULL;
+ return error;
}
}
@@ -757,7 +743,8 @@ xfs_buf_get_map(
XFS_STATS_INC(target->bt_mount, xb_get);
trace_xfs_buf_get(bp, flags, _RET_IP_);
- return bp;
+ *bpp = bp;
+ return 0;
}
STATIC int
@@ -818,11 +805,12 @@ xfs_buf_read_map(
const struct xfs_buf_ops *ops)
{
struct xfs_buf *bp;
+ int error;
flags |= XBF_READ;
- bp = xfs_buf_get_map(target, map, nmaps, flags);
- if (!bp)
+ error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
+ if (error)
return NULL;
trace_xfs_buf_read(bp, flags, _RET_IP_);
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 56e081dd1d96..25dd2aa4322b 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -192,9 +192,8 @@ struct xfs_buf *xfs_buf_incore(struct xfs_buftarg *target,
xfs_daddr_t blkno, size_t numblks,
xfs_buf_flags_t flags);
-struct xfs_buf *xfs_buf_get_map(struct xfs_buftarg *target,
- struct xfs_buf_map *map, int nmaps,
- xfs_buf_flags_t flags);
+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,
@@ -209,8 +208,14 @@ xfs_buf_get(
xfs_daddr_t blkno,
size_t numblks)
{
+ struct xfs_buf *bp;
+ int error;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_get_map(target, &map, 1, 0);
+
+ error = xfs_buf_get_map(target, &map, 1, 0, &bp);
+ if (error)
+ return NULL;
+ return bp;
}
static inline struct xfs_buf *
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index b5b3a78ef31c..288333fef13a 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -122,9 +122,14 @@ xfs_trans_get_buf_map(
{
xfs_buf_t *bp;
struct xfs_buf_log_item *bip;
+ int error;
- if (!tp)
- return xfs_buf_get_map(target, map, nmaps, flags);
+ if (!tp) {
+ error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
+ if (error)
+ return NULL;
+ return bp;
+ }
/*
* If we find the buffer in the cache with this transaction
@@ -149,10 +154,9 @@ xfs_trans_get_buf_map(
return bp;
}
- bp = xfs_buf_get_map(target, map, nmaps, flags);
- if (bp == NULL) {
+ error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
+ if (error)
return NULL;
- }
ASSERT(!bp->b_error);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/12] xfs: make xfs_buf_read_map return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-24 5:19 ` [PATCH 01/12] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
2020-01-24 5:19 ` [PATCH 02/12] xfs: make xfs_buf_get_map " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:07 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 04/12] xfs: make xfs_buf_get " Darrick J. Wong
` (8 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
From: Darrick J. Wong <darrick.wong@oracle.com>
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>
---
fs/xfs/libxfs/xfs_alloc.c | 11 ++++---
fs/xfs/libxfs/xfs_attr_remote.c | 10 ------
fs/xfs/xfs_buf.c | 63 +++++++++++++++++++++++++++++----------
fs/xfs/xfs_buf.h | 15 ++++++---
fs/xfs/xfs_log_recover.c | 10 ------
fs/xfs/xfs_symlink.c | 10 ------
fs/xfs/xfs_trans_buf.c | 36 +++++-----------------
7 files changed, 72 insertions(+), 83 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index fc93fd88ec89..4cc10aa43edf 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -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);
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index a266d05df146..88e50e904436 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -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,
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 5c07b4a70026..871abaabff3d 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -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);
}
/*
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 25dd2aa4322b..f58147354b02 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -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
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 0d683fb96396..c805a02f0078 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -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);
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index a25502bc2071..b255a393a73b 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -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;
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 288333fef13a..cdb66c661425 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/12] xfs: make xfs_buf_get return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (2 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 03/12] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:19 ` [PATCH 05/12] xfs: make xfs_buf_get_uncached " Darrick J. Wong
` (7 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_buf_get() to return numeric error codes like most
everywhere else in xfs.
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_attr_remote.c | 6 +++---
fs/xfs/libxfs/xfs_sb.c | 8 ++++----
fs/xfs/xfs_buf.h | 12 ++++--------
3 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 88e50e904436..7266e280b3e8 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -545,9 +545,9 @@ xfs_attr_rmtval_set(
dblkno = XFS_FSB_TO_DADDR(mp, map.br_startblock),
dblkcnt = XFS_FSB_TO_BB(mp, map.br_blockcount);
- bp = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_get(mp->m_ddev_targp, dblkno, dblkcnt, &bp);
+ if (error)
+ return error;
bp->b_ops = &xfs_attr3_rmt_buf_ops;
xfs_attr_rmtval_copyin(mp, bp, args->dp->i_ino, &offset,
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 0ac69751fe85..6fdd007f81ab 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -985,9 +985,9 @@ xfs_update_secondary_sbs(
for (agno = 1; agno < mp->m_sb.sb_agcount; agno++) {
struct xfs_buf *bp;
- bp = xfs_buf_get(mp->m_ddev_targp,
+ error = xfs_buf_get(mp->m_ddev_targp,
XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
- XFS_FSS_TO_BB(mp, 1));
+ XFS_FSS_TO_BB(mp, 1), &bp);
/*
* If we get an error reading or writing alternate superblocks,
* continue. xfs_repair chooses the "best" superblock based
@@ -995,12 +995,12 @@ xfs_update_secondary_sbs(
* superblocks un-updated than updated, and xfs_repair may
* pick them over the properly-updated primary.
*/
- if (!bp) {
+ if (error) {
xfs_warn(mp,
"error allocating secondary superblock for ag %d",
agno);
if (!saved_error)
- saved_error = -ENOMEM;
+ saved_error = error;
continue;
}
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index f58147354b02..c3aa4e322243 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -201,20 +201,16 @@ void xfs_buf_readahead_map(struct xfs_buftarg *target,
struct xfs_buf_map *map, int nmaps,
const struct xfs_buf_ops *ops);
-static inline struct xfs_buf *
+static inline int
xfs_buf_get(
struct xfs_buftarg *target,
xfs_daddr_t blkno,
- size_t numblks)
+ size_t numblks,
+ struct xfs_buf **bpp)
{
- struct xfs_buf *bp;
- int error;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- error = xfs_buf_get_map(target, &map, 1, 0, &bp);
- if (error)
- return NULL;
- return bp;
+ return xfs_buf_get_map(target, &map, 1, 0, bpp);
}
static inline struct xfs_buf *
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/12] xfs: make xfs_buf_get_uncached return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (3 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 04/12] xfs: make xfs_buf_get " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:19 ` [PATCH 06/12] xfs: make xfs_buf_read " Darrick J. Wong
` (6 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_buf_get_uncached() to return numeric error codes like most
everywhere else in xfs.
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_ag.c | 21 ++++++++++++---------
fs/xfs/xfs_buf.c | 25 ++++++++++++++++---------
fs/xfs/xfs_buf.h | 4 ++--
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 14fbdf22b7e7..08d6beb54f8c 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -23,25 +23,28 @@
#include "xfs_ag_resv.h"
#include "xfs_health.h"
-static struct xfs_buf *
+static int
xfs_get_aghdr_buf(
struct xfs_mount *mp,
xfs_daddr_t blkno,
size_t numblks,
+ struct xfs_buf **bpp,
const struct xfs_buf_ops *ops)
{
struct xfs_buf *bp;
+ int error;
- bp = xfs_buf_get_uncached(mp->m_ddev_targp, numblks, 0);
- if (!bp)
- return NULL;
+ error = xfs_buf_get_uncached(mp->m_ddev_targp, numblks, 0, &bp);
+ if (error)
+ return error;
xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
bp->b_bn = blkno;
bp->b_maps[0].bm_bn = blkno;
bp->b_ops = ops;
- return bp;
+ *bpp = bp;
+ return 0;
}
static inline bool is_log_ag(struct xfs_mount *mp, struct aghdr_init_data *id)
@@ -340,13 +343,13 @@ xfs_ag_init_hdr(
struct aghdr_init_data *id,
aghdr_init_work_f work,
const struct xfs_buf_ops *ops)
-
{
struct xfs_buf *bp;
+ int error;
- bp = xfs_get_aghdr_buf(mp, id->daddr, id->numblks, ops);
- if (!bp)
- return -ENOMEM;
+ error = xfs_get_aghdr_buf(mp, id->daddr, id->numblks, &bp, ops);
+ if (error)
+ return error;
(*work)(mp, bp, id);
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 871abaabff3d..b420e865b32e 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -902,12 +902,13 @@ xfs_buf_read_uncached(
const struct xfs_buf_ops *ops)
{
struct xfs_buf *bp;
+ int error;
*bpp = NULL;
- bp = xfs_buf_get_uncached(target, numblks, flags);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_get_uncached(target, numblks, flags, &bp);
+ if (error)
+ return error;
/* set up the buffer for a read IO */
ASSERT(bp->b_map_count == 1);
@@ -918,7 +919,7 @@ xfs_buf_read_uncached(
xfs_buf_submit(bp);
if (bp->b_error) {
- int error = bp->b_error;
+ error = bp->b_error;
xfs_buf_relse(bp);
return error;
}
@@ -927,17 +928,20 @@ xfs_buf_read_uncached(
return 0;
}
-xfs_buf_t *
+int
xfs_buf_get_uncached(
struct xfs_buftarg *target,
size_t numblks,
- int flags)
+ int flags,
+ struct xfs_buf **bpp)
{
unsigned long page_count;
int error, i;
struct xfs_buf *bp;
DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);
+ *bpp = NULL;
+
/* flags might contain irrelevant bits, pass only what we care about */
error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);
if (error)
@@ -950,8 +954,10 @@ xfs_buf_get_uncached(
for (i = 0; i < page_count; i++) {
bp->b_pages[i] = alloc_page(xb_to_gfp(flags));
- if (!bp->b_pages[i])
+ if (!bp->b_pages[i]) {
+ error = -ENOMEM;
goto fail_free_mem;
+ }
}
bp->b_flags |= _XBF_PAGES;
@@ -963,7 +969,8 @@ xfs_buf_get_uncached(
}
trace_xfs_buf_get_uncached(bp, _RET_IP_);
- return bp;
+ *bpp = bp;
+ return 0;
fail_free_mem:
while (--i >= 0)
@@ -973,7 +980,7 @@ xfs_buf_get_uncached(
xfs_buf_free_maps(bp);
kmem_cache_free(xfs_buf_zone, bp);
fail:
- return NULL;
+ return error;
}
/*
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index c3aa4e322243..7f7bd1edd99e 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -242,8 +242,8 @@ xfs_buf_readahead(
return xfs_buf_readahead_map(target, &map, 1, ops);
}
-struct xfs_buf *xfs_buf_get_uncached(struct xfs_buftarg *target, size_t numblks,
- int flags);
+int xfs_buf_get_uncached(struct xfs_buftarg *target, size_t numblks, int flags,
+ struct xfs_buf **bpp);
int xfs_buf_read_uncached(struct xfs_buftarg *target, xfs_daddr_t daddr,
size_t numblks, int flags, struct xfs_buf **bpp,
const struct xfs_buf_ops *ops);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/12] xfs: make xfs_buf_read return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (4 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 05/12] xfs: make xfs_buf_get_uncached " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:08 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 07/12] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
` (5 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_buf_read() to return numeric error codes like most
everywhere else in xfs.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr_remote.c | 8 ++++----
fs/xfs/xfs_buf.h | 10 +++-------
fs/xfs/xfs_log_recover.c | 16 +++++++---------
fs/xfs/xfs_symlink.c | 8 ++++----
4 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 7266e280b3e8..8b7f74b3bea2 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -418,10 +418,10 @@ xfs_attr_rmtval_get(
(map[i].br_startblock != HOLESTARTBLOCK));
dblkno = XFS_FSB_TO_DADDR(mp, map[i].br_startblock);
dblkcnt = XFS_FSB_TO_BB(mp, map[i].br_blockcount);
- bp = xfs_buf_read(mp->m_ddev_targp, dblkno, dblkcnt, 0,
- &xfs_attr3_rmt_buf_ops);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_read(mp->m_ddev_targp, dblkno, dblkcnt,
+ 0, &bp, &xfs_attr3_rmt_buf_ops);
+ if (error)
+ return error;
error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
&offset, &valuelen,
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 7f7bd1edd99e..aa145ad25e9a 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -213,22 +213,18 @@ xfs_buf_get(
return xfs_buf_get_map(target, &map, 1, 0, bpp);
}
-static inline struct xfs_buf *
+static inline int
xfs_buf_read(
struct xfs_buftarg *target,
xfs_daddr_t blkno,
size_t numblks,
xfs_buf_flags_t flags,
+ struct xfs_buf **bpp,
const struct xfs_buf_ops *ops)
{
- struct xfs_buf *bp;
- int error;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
- if (error)
- return NULL;
- return bp;
+ return xfs_buf_read_map(target, &map, 1, flags, bpp, ops);
}
static inline void
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index c805a02f0078..ac79537d3275 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2745,10 +2745,10 @@ xlog_recover_buffer_pass2(
if (buf_f->blf_flags & XFS_BLF_INODE_BUF)
buf_flags |= XBF_UNMAPPED;
- bp = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len,
- buf_flags, NULL);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len,
+ buf_flags, &bp, NULL);
+ if (error)
+ return error;
/*
* Recover the buffer only if we get an LSN from it and it's less than
@@ -2945,12 +2945,10 @@ xlog_recover_inode_pass2(
}
trace_xfs_log_recover_inode_recover(log, in_f);
- bp = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len, 0,
- &xfs_inode_buf_ops);
- if (!bp) {
- error = -ENOMEM;
+ error = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len,
+ 0, &bp, &xfs_inode_buf_ops);
+ if (error)
goto error;
- }
ASSERT(in_f->ilf_fields & XFS_ILOG_CORE);
dip = xfs_buf_offset(bp, in_f->ilf_boffset);
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index b255a393a73b..b94d7b9b55d0 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -53,10 +53,10 @@ xfs_readlink_bmap_ilocked(
d = XFS_FSB_TO_DADDR(mp, mval[n].br_startblock);
byte_cnt = XFS_FSB_TO_B(mp, mval[n].br_blockcount);
- bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt), 0,
- &xfs_symlink_buf_ops);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt), 0,
+ &bp, &xfs_symlink_buf_ops);
+ if (error)
+ return error;
byte_cnt = XFS_SYMLINK_BUF_SPACE(mp, byte_cnt);
if (pathlen < byte_cnt)
byte_cnt = pathlen;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/12] xfs: make xfs_trans_get_buf_map return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (5 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 06/12] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:19 ` [PATCH 08/12] xfs: make xfs_trans_get_buf " Darrick J. Wong
` (4 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_trans_get_buf_map() to return numeric error codes like most
everywhere else in xfs.
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_da_btree.c | 8 ++------
fs/xfs/xfs_trans.h | 15 ++++++++++-----
fs/xfs/xfs_trans_buf.c | 22 +++++++++++-----------
3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 8c3eafe280ed..875e04f82541 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2591,13 +2591,9 @@ xfs_da_get_buf(
if (error || nmap == 0)
goto out_free;
- bp = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0);
- error = bp ? bp->b_error : -EIO;
- if (error) {
- if (bp)
- xfs_trans_brelse(tp, bp);
+ error = xfs_trans_get_buf_map(tp, mp->m_ddev_targp, mapp, nmap, 0, &bp);
+ if (error)
goto out_free;
- }
*bpp = bp;
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 64d7f171ebd3..a0be934ec811 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -169,10 +169,9 @@ int xfs_trans_alloc_empty(struct xfs_mount *mp,
struct xfs_trans **tpp);
void xfs_trans_mod_sb(xfs_trans_t *, uint, int64_t);
-struct xfs_buf *xfs_trans_get_buf_map(struct xfs_trans *tp,
- struct xfs_buftarg *target,
- struct xfs_buf_map *map, int nmaps,
- uint flags);
+int xfs_trans_get_buf_map(struct xfs_trans *tp, struct xfs_buftarg *target,
+ struct xfs_buf_map *map, int nmaps, xfs_buf_flags_t flags,
+ struct xfs_buf **bpp);
static inline struct xfs_buf *
xfs_trans_get_buf(
@@ -182,8 +181,14 @@ xfs_trans_get_buf(
int numblks,
uint flags)
{
+ struct xfs_buf *bp;
+ int error;
+
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_trans_get_buf_map(tp, target, &map, 1, flags);
+ error = xfs_trans_get_buf_map(tp, target, &map, 1, flags, &bp);
+ if (error)
+ return NULL;
+ return bp;
}
int xfs_trans_read_buf_map(struct xfs_mount *mp,
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index cdb66c661425..83470998f87b 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -112,24 +112,22 @@ xfs_trans_bjoin(
* If the transaction pointer is NULL, make this just a normal
* get_buf() call.
*/
-struct xfs_buf *
+int
xfs_trans_get_buf_map(
struct xfs_trans *tp,
struct xfs_buftarg *target,
struct xfs_buf_map *map,
int nmaps,
- xfs_buf_flags_t flags)
+ xfs_buf_flags_t flags,
+ struct xfs_buf **bpp)
{
xfs_buf_t *bp;
struct xfs_buf_log_item *bip;
int error;
- if (!tp) {
- error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
- if (error)
- return NULL;
- return bp;
- }
+ *bpp = NULL;
+ if (!tp)
+ return xfs_buf_get_map(target, map, nmaps, flags, bpp);
/*
* If we find the buffer in the cache with this transaction
@@ -151,18 +149,20 @@ xfs_trans_get_buf_map(
ASSERT(atomic_read(&bip->bli_refcount) > 0);
bip->bli_recur++;
trace_xfs_trans_get_buf_recur(bip);
- return bp;
+ *bpp = bp;
+ return 0;
}
error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
if (error)
- return NULL;
+ return error;
ASSERT(!bp->b_error);
_xfs_trans_bjoin(tp, bp, 1);
trace_xfs_trans_get_buf(bp->b_log_item);
- return bp;
+ *bpp = bp;
+ return 0;
}
/*
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/12] xfs: make xfs_trans_get_buf return an error code
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (6 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 07/12] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:19 ` [PATCH 09/12] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert xfs_trans_get_buf() to return numeric error codes like most
everywhere else in xfs.
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_btree.c | 23 ++++++++++++++++-------
fs/xfs/libxfs/xfs_ialloc.c | 12 ++++++------
fs/xfs/libxfs/xfs_sb.c | 9 +++++----
fs/xfs/scrub/repair.c | 8 ++++++--
fs/xfs/xfs_attr_inactive.c | 17 +++++++++--------
fs/xfs/xfs_dquot.c | 8 ++++----
fs/xfs/xfs_inode.c | 12 ++++++------
fs/xfs/xfs_rtalloc.c | 8 +++-----
fs/xfs/xfs_symlink.c | 19 ++++++++-----------
fs/xfs/xfs_trans.h | 13 ++++---------
10 files changed, 67 insertions(+), 62 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index b22c7e928eb1..2d53e5fdff70 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -688,11 +688,16 @@ xfs_btree_get_bufl(
xfs_trans_t *tp, /* transaction pointer */
xfs_fsblock_t fsbno) /* file system block number */
{
+ struct xfs_buf *bp;
xfs_daddr_t d; /* real disk block address */
+ int error;
ASSERT(fsbno != NULLFSBLOCK);
d = XFS_FSB_TO_DADDR(mp, fsbno);
- return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0);
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
+ if (error)
+ return NULL;
+ return bp;
}
/*
@@ -706,12 +711,17 @@ xfs_btree_get_bufs(
xfs_agnumber_t agno, /* allocation group number */
xfs_agblock_t agbno) /* allocation group block number */
{
+ struct xfs_buf *bp;
xfs_daddr_t d; /* real disk block address */
+ int error;
ASSERT(agno != NULLAGNUMBER);
ASSERT(agbno != NULLAGBLOCK);
d = XFS_AGB_TO_DADDR(mp, agno, agbno);
- return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0);
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
+ if (error)
+ return NULL;
+ return bp;
}
/*
@@ -1270,11 +1280,10 @@ xfs_btree_get_buf_block(
error = xfs_btree_ptr_to_daddr(cur, ptr, &d);
if (error)
return error;
- *bpp = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d,
- mp->m_bsize, 0);
-
- if (!*bpp)
- return -ENOMEM;
+ error = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d, mp->m_bsize,
+ 0, bpp);
+ if (error)
+ return error;
(*bpp)->b_ops = cur->bc_ops->buf_ops;
*block = XFS_BUF_TO_BLOCK(*bpp);
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 5b759af4d165..bf161e930f1d 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -276,6 +276,7 @@ xfs_ialloc_inode_init(
int i, j;
xfs_daddr_t d;
xfs_ino_t ino = 0;
+ int error;
/*
* Loop over the new block(s), filling in the inodes. For small block
@@ -327,12 +328,11 @@ xfs_ialloc_inode_init(
*/
d = XFS_AGB_TO_DADDR(mp, agno, agbno +
(j * M_IGEO(mp)->blocks_per_cluster));
- fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
- mp->m_bsize *
- M_IGEO(mp)->blocks_per_cluster,
- XBF_UNMAPPED);
- if (!fbuf)
- return -ENOMEM;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
+ mp->m_bsize * M_IGEO(mp)->blocks_per_cluster,
+ XBF_UNMAPPED, &fbuf);
+ if (error)
+ return error;
/* Initialize the inode buffers and log them appropriately. */
fbuf->b_ops = &xfs_inode_buf_ops;
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 6fdd007f81ab..2f60fc3c99a0 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1185,13 +1185,14 @@ xfs_sb_get_secondary(
struct xfs_buf **bpp)
{
struct xfs_buf *bp;
+ int error;
ASSERT(agno != 0 && agno != NULLAGNUMBER);
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
XFS_AG_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
- XFS_FSS_TO_BB(mp, 1), 0);
- if (!bp)
- return -ENOMEM;
+ XFS_FSS_TO_BB(mp, 1), 0, &bp);
+ if (error)
+ return error;
bp->b_ops = &xfs_sb_buf_ops;
xfs_buf_oneshot(bp);
*bpp = bp;
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index b70a88bc975e..3df49d487940 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -341,13 +341,17 @@ xrep_init_btblock(
struct xfs_trans *tp = sc->tp;
struct xfs_mount *mp = sc->mp;
struct xfs_buf *bp;
+ int error;
trace_xrep_init_btblock(mp, XFS_FSB_TO_AGNO(mp, fsb),
XFS_FSB_TO_AGBNO(mp, fsb), btnum);
ASSERT(XFS_FSB_TO_AGNO(mp, fsb) == sc->sa.agno);
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, fsb),
- XFS_FSB_TO_BB(mp, 1), 0);
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+ XFS_FSB_TO_DADDR(mp, fsb), XFS_FSB_TO_BB(mp, 1), 0,
+ &bp);
+ if (error)
+ return error;
xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
xfs_btree_init_block(mp, bp, btnum, 0, 0, sc->sa.agno);
xfs_trans_buf_set_type(tp, bp, XFS_BLFT_BTREE_BUF);
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index 8fbb841cd6fe..bbfa6ba84dcd 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -205,11 +205,12 @@ xfs_attr3_node_inactive(
/*
* Remove the subsidiary block from the cache and from the log.
*/
- child_bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp,
+ error = xfs_trans_get_buf(*trans, mp->m_ddev_targp,
child_blkno,
- XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
- if (!child_bp)
- return -EIO;
+ XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0,
+ &child_bp);
+ if (error)
+ return error;
error = bp->b_error;
if (error) {
xfs_trans_brelse(*trans, child_bp);
@@ -298,10 +299,10 @@ xfs_attr3_root_inactive(
/*
* Invalidate the incore copy of the root block.
*/
- bp = xfs_trans_get_buf(*trans, mp->m_ddev_targp, blkno,
- XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0);
- if (!bp)
- return -EIO;
+ error = xfs_trans_get_buf(*trans, mp->m_ddev_targp, blkno,
+ XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0, &bp);
+ if (error)
+ return error;
error = bp->b_error;
if (error) {
xfs_trans_brelse(*trans, bp);
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 9cfd3209f52b..d223e1ae90a6 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -320,10 +320,10 @@ xfs_dquot_disk_alloc(
dqp->q_blkno = XFS_FSB_TO_DADDR(mp, map.br_startblock);
/* now we can just get the buffer (there's nothing to read yet) */
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
- mp->m_quotainfo->qi_dqchunklen, 0);
- if (!bp)
- return -ENOMEM;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, dqp->q_blkno,
+ mp->m_quotainfo->qi_dqchunklen, 0, &bp);
+ if (error)
+ return error;
bp->b_ops = &xfs_dquot_buf_ops;
/*
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1979a0055763..c5077e6326c7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2546,6 +2546,7 @@ xfs_ifree_cluster(
struct xfs_perag *pag;
struct xfs_ino_geometry *igeo = M_IGEO(mp);
xfs_ino_t inum;
+ int error;
inum = xic->first_ino;
pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
@@ -2574,12 +2575,11 @@ xfs_ifree_cluster(
* complete before we get a lock on it, and hence we may fail
* to mark all the active inodes on the buffer stale.
*/
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, blkno,
- mp->m_bsize * igeo->blocks_per_cluster,
- XBF_UNMAPPED);
-
- if (!bp)
- return -ENOMEM;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, blkno,
+ mp->m_bsize * igeo->blocks_per_cluster,
+ XBF_UNMAPPED, &bp);
+ if (error)
+ return error;
/*
* This buffer may not have been correctly initialised as we
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index d42b5a2047e0..6209e7b6b895 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -826,12 +826,10 @@ xfs_growfs_rt_alloc(
* Get a buffer for the block.
*/
d = XFS_FSB_TO_DADDR(mp, fsbno);
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
- mp->m_bsize, 0);
- if (bp == NULL) {
- error = -EIO;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
+ mp->m_bsize, 0, &bp);
+ if (error)
goto out_trans_cancel;
- }
memset(bp->b_addr, 0, mp->m_sb.sb_blocksize);
xfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
/*
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index b94d7b9b55d0..d762d42ed0ff 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -280,12 +280,10 @@ xfs_symlink(
d = XFS_FSB_TO_DADDR(mp, mval[n].br_startblock);
byte_cnt = XFS_FSB_TO_B(mp, mval[n].br_blockcount);
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
- BTOBB(byte_cnt), 0);
- if (!bp) {
- error = -ENOMEM;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
+ BTOBB(byte_cnt), 0, &bp);
+ if (error)
goto out_trans_cancel;
- }
bp->b_ops = &xfs_symlink_buf_ops;
byte_cnt = XFS_SYMLINK_BUF_SPACE(mp, byte_cnt);
@@ -423,13 +421,12 @@ xfs_inactive_symlink_rmt(
* Invalidate the block(s). No validation is done.
*/
for (i = 0; i < nmaps; i++) {
- bp = xfs_trans_get_buf(tp, mp->m_ddev_targp,
- XFS_FSB_TO_DADDR(mp, mval[i].br_startblock),
- XFS_FSB_TO_BB(mp, mval[i].br_blockcount), 0);
- if (!bp) {
- error = -ENOMEM;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+ XFS_FSB_TO_DADDR(mp, mval[i].br_startblock),
+ XFS_FSB_TO_BB(mp, mval[i].br_blockcount), 0,
+ &bp);
+ if (error)
goto error_trans_cancel;
- }
xfs_trans_binval(tp, bp);
}
/*
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index a0be934ec811..752c7fef9de7 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -173,22 +173,17 @@ int xfs_trans_get_buf_map(struct xfs_trans *tp, struct xfs_buftarg *target,
struct xfs_buf_map *map, int nmaps, xfs_buf_flags_t flags,
struct xfs_buf **bpp);
-static inline struct xfs_buf *
+static inline int
xfs_trans_get_buf(
struct xfs_trans *tp,
struct xfs_buftarg *target,
xfs_daddr_t blkno,
int numblks,
- uint flags)
+ uint flags,
+ struct xfs_buf **bpp)
{
- struct xfs_buf *bp;
- int error;
-
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- error = xfs_trans_get_buf_map(tp, target, &map, 1, flags, &bp);
- if (error)
- return NULL;
- return bp;
+ return xfs_trans_get_buf_map(tp, target, &map, 1, flags, bpp);
}
int xfs_trans_read_buf_map(struct xfs_mount *mp,
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/12] xfs: remove the xfs_btree_get_buf[ls] functions
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (7 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 08/12] xfs: make xfs_trans_get_buf " Darrick J. Wong
@ 2020-01-24 5:19 ` Darrick J. Wong
2020-01-24 5:20 ` [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:19 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Remove the xfs_btree_get_bufs and xfs_btree_get_bufl functions, since
they're pretty trivial oneliners.
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 | 16 +++++++++-------
fs/xfs/libxfs/xfs_bmap.c | 14 +++++++++-----
fs/xfs/libxfs/xfs_btree.c | 46 ---------------------------------------------
fs/xfs/libxfs/xfs_btree.h | 21 ---------------------
4 files changed, 18 insertions(+), 79 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 4cc10aa43edf..34b65635ee34 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1070,11 +1070,11 @@ xfs_alloc_ag_vextent_small(
if (args->datatype & XFS_ALLOC_USERDATA) {
struct xfs_buf *bp;
- bp = xfs_btree_get_bufs(args->mp, args->tp, args->agno, fbno);
- if (XFS_IS_CORRUPT(args->mp, !bp)) {
- error = -EFSCORRUPTED;
+ error = xfs_trans_get_buf(args->tp, args->mp->m_ddev_targp,
+ XFS_AGB_TO_DADDR(args->mp, args->agno, fbno),
+ args->mp->m_bsize, 0, &bp);
+ if (error)
goto error;
- }
xfs_trans_binval(args->tp, bp);
}
*fbnop = args->agbno = fbno;
@@ -2347,9 +2347,11 @@ xfs_free_agfl_block(
if (error)
return error;
- bp = xfs_btree_get_bufs(tp->t_mountp, tp, agno, agbno);
- if (XFS_IS_CORRUPT(tp->t_mountp, !bp))
- return -EFSCORRUPTED;
+ error = xfs_trans_get_buf(tp, tp->t_mountp->m_ddev_targp,
+ XFS_AGB_TO_DADDR(tp->t_mountp, agno, agbno),
+ tp->t_mountp->m_bsize, 0, &bp);
+ if (error)
+ return error;
xfs_trans_binval(tp, bp);
return 0;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4c2e046fbfad..cfcef076c72f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -730,11 +730,11 @@ xfs_bmap_extents_to_btree(
cur->bc_private.b.allocated++;
ip->i_d.di_nblocks++;
xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, 1L);
- abp = xfs_btree_get_bufl(mp, tp, args.fsbno);
- if (XFS_IS_CORRUPT(mp, !abp)) {
- error = -EFSCORRUPTED;
+ error = xfs_trans_get_buf(tp, mp->m_ddev_targp,
+ XFS_FSB_TO_DADDR(mp, args.fsbno),
+ mp->m_bsize, 0, &abp);
+ if (error)
goto out_unreserve_dquot;
- }
/*
* Fill in the child block.
@@ -878,7 +878,11 @@ xfs_bmap_local_to_extents(
ASSERT(args.fsbno != NULLFSBLOCK);
ASSERT(args.len == 1);
tp->t_firstblock = args.fsbno;
- bp = xfs_btree_get_bufl(args.mp, tp, args.fsbno);
+ error = xfs_trans_get_buf(tp, args.mp->m_ddev_targp,
+ XFS_FSB_TO_DADDR(args.mp, args.fsbno),
+ args.mp->m_bsize, 0, &bp);
+ if (error)
+ goto done;
/*
* Initialize the block, copy the data and log the remote buffer.
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 2d53e5fdff70..fd300dc93ca4 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -678,52 +678,6 @@ xfs_btree_get_block(
return XFS_BUF_TO_BLOCK(*bpp);
}
-/*
- * Get a buffer for the block, return it with no data read.
- * Long-form addressing.
- */
-xfs_buf_t * /* buffer for fsbno */
-xfs_btree_get_bufl(
- xfs_mount_t *mp, /* file system mount point */
- xfs_trans_t *tp, /* transaction pointer */
- xfs_fsblock_t fsbno) /* file system block number */
-{
- struct xfs_buf *bp;
- xfs_daddr_t d; /* real disk block address */
- int error;
-
- ASSERT(fsbno != NULLFSBLOCK);
- d = XFS_FSB_TO_DADDR(mp, fsbno);
- error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
- if (error)
- return NULL;
- return bp;
-}
-
-/*
- * Get a buffer for the block, return it with no data read.
- * Short-form addressing.
- */
-xfs_buf_t * /* buffer for agno/agbno */
-xfs_btree_get_bufs(
- xfs_mount_t *mp, /* file system mount point */
- xfs_trans_t *tp, /* transaction pointer */
- xfs_agnumber_t agno, /* allocation group number */
- xfs_agblock_t agbno) /* allocation group block number */
-{
- struct xfs_buf *bp;
- xfs_daddr_t d; /* real disk block address */
- int error;
-
- ASSERT(agno != NULLAGNUMBER);
- ASSERT(agbno != NULLAGBLOCK);
- d = XFS_AGB_TO_DADDR(mp, agno, agbno);
- error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
- if (error)
- return NULL;
- return bp;
-}
-
/*
* Change the cursor to point to the first record at the given level.
* Other levels are unaffected.
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index fb9b2121c628..3eff7c321d43 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -296,27 +296,6 @@ xfs_btree_dup_cursor(
xfs_btree_cur_t *cur, /* input cursor */
xfs_btree_cur_t **ncur);/* output cursor */
-/*
- * Get a buffer for the block, return it with no data read.
- * Long-form addressing.
- */
-struct xfs_buf * /* buffer for fsbno */
-xfs_btree_get_bufl(
- struct xfs_mount *mp, /* file system mount point */
- struct xfs_trans *tp, /* transaction pointer */
- xfs_fsblock_t fsbno); /* file system block number */
-
-/*
- * Get a buffer for the block, return it with no data read.
- * Short-form addressing.
- */
-struct xfs_buf * /* buffer for agno/agbno */
-xfs_btree_get_bufs(
- struct xfs_mount *mp, /* file system mount point */
- struct xfs_trans *tp, /* transaction pointer */
- xfs_agnumber_t agno, /* allocation group number */
- xfs_agblock_t agbno); /* allocation group block number */
-
/*
* Compute first and last byte offsets for the fields given.
* Interprets the offsets table, which contains struct field offsets.
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (8 preceding siblings ...)
2020-01-24 5:19 ` [PATCH 09/12] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
@ 2020-01-24 5:20 ` Darrick J. Wong
2020-01-24 22:07 ` Dave Chinner
2020-01-24 23:12 ` Christoph Hellwig
2020-01-24 5:20 ` [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
2020-01-24 5:20 ` [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
11 siblings, 2 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:20 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
From: Darrick J. Wong <darrick.wong@oracle.com>
Refactor xfs_read_agf and xfs_alloc_read_agf to return EAGAIN if the
caller passed TRYLOCK and we weren't able to get the lock; and change
the callers to recognize this.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 36 ++++++++++++++----------------------
fs/xfs/libxfs/xfs_bmap.c | 11 ++++++-----
fs/xfs/xfs_filestream.c | 11 +++++------
3 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 34b65635ee34..d8053bc96c4d 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2502,12 +2502,11 @@ xfs_alloc_fix_freelist(
if (!pag->pagf_init) {
error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
- if (error)
+ if (error) {
+ /* Couldn't lock the AGF so skip this AG. */
+ if (error == -EAGAIN)
+ error = 0;
goto out_no_agbp;
- if (!pag->pagf_init) {
- ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
- ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
- goto out_agbp_relse;
}
}
@@ -2533,11 +2532,10 @@ xfs_alloc_fix_freelist(
*/
if (!agbp) {
error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
- if (error)
- goto out_no_agbp;
- if (!agbp) {
- ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
- ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
+ if (error) {
+ /* Couldn't lock the AGF so skip this AG. */
+ if (error == -EAGAIN)
+ error = 0;
goto out_no_agbp;
}
}
@@ -2768,11 +2766,10 @@ xfs_alloc_pagf_init(
xfs_buf_t *bp;
int error;
- if ((error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp)))
- return error;
- if (bp)
+ error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp);
+ if (!error)
xfs_trans_brelse(tp, bp);
- return 0;
+ return error;
}
/*
@@ -2961,12 +2958,6 @@ xfs_read_agf(
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;
@@ -2992,14 +2983,15 @@ xfs_alloc_read_agf(
trace_xfs_alloc_read_agf(mp, agno);
+ /* We don't support trylock when freeing. */
+ ASSERT((flags & (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK)) !=
+ (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK));
ASSERT(agno != NULLAGNUMBER);
error = xfs_read_agf(mp, tp, agno,
(flags & XFS_ALLOC_FLAG_TRYLOCK) ? XBF_TRYLOCK : 0,
bpp);
if (error)
return error;
- if (!*bpp)
- return 0;
ASSERT(!(*bpp)->b_error);
agf = XFS_BUF_TO_AGF(*bpp);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index cfcef076c72f..9a6d7a84689a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3311,11 +3311,12 @@ xfs_bmap_longest_free_extent(
pag = xfs_perag_get(mp, ag);
if (!pag->pagf_init) {
error = xfs_alloc_pagf_init(mp, tp, ag, XFS_ALLOC_FLAG_TRYLOCK);
- if (error)
- goto out;
-
- if (!pag->pagf_init) {
- *notinit = 1;
+ if (error) {
+ /* Couldn't lock the AGF, so skip this AG. */
+ if (error == -EAGAIN) {
+ *notinit = 1;
+ error = 0;
+ }
goto out;
}
}
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 5f12b5d8527a..1a88025e68a3 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -159,16 +159,15 @@ xfs_filestream_pick_ag(
if (!pag->pagf_init) {
err = xfs_alloc_pagf_init(mp, NULL, ag, trylock);
- if (err && !trylock) {
+ if (err) {
xfs_perag_put(pag);
- return err;
+ if (err != -EAGAIN)
+ return err;
+ /* Couldn't lock the AGF, skip this AG. */
+ continue;
}
}
- /* Might fail sometimes during the 1st pass with trylock set. */
- if (!pag->pagf_init)
- goto next_ag;
-
/* Keep track of the AG with the most free blocks. */
if (pag->pagf_freeblks > maxfree) {
maxfree = pag->pagf_freeblks;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (9 preceding siblings ...)
2020-01-24 5:20 ` [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
@ 2020-01-24 5:20 ` Darrick J. Wong
2020-01-24 5:20 ` [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
11 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:20 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Drop the null buffer pointer checks in all code that calls
xfs_alloc_read_agf and doesn't pass XFS_ALLOC_FLAG_TRYLOCK because
they're no longer necessary.
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_refcount.c | 6 ------
| 4 ----
fs/xfs/scrub/fscounters.c | 3 ---
fs/xfs/scrub/repair.c | 2 --
fs/xfs/xfs_discard.c | 2 +-
fs/xfs/xfs_reflink.c | 2 --
6 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index d7d702ee4d1a..6e1665f2cb67 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1177,8 +1177,6 @@ xfs_refcount_finish_one(
XFS_ALLOC_FLAG_FREEING, &agbp);
if (error)
return error;
- if (XFS_IS_CORRUPT(tp->t_mountp, !agbp))
- return -EFSCORRUPTED;
rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
if (!rcur) {
@@ -1718,10 +1716,6 @@ xfs_refcount_recover_cow_leftovers(
error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
if (error)
goto out_trans;
- if (!agbp) {
- error = -ENOMEM;
- goto out_trans;
- }
cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
/* Find all the leftover CoW staging extents. */
--git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 7a1a38b636a9..d5e6db9af434 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -659,8 +659,6 @@ xrep_agfl(
error = xfs_alloc_read_agf(mp, sc->tp, sc->sa.agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
/*
* Make sure we have the AGFL buffer, as scrub might have decided it
@@ -735,8 +733,6 @@ xrep_agi_find_btrees(
error = xfs_alloc_read_agf(mp, sc->tp, sc->sa.agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
/* Find the btree roots. */
error = xrep_find_ag_btree_roots(sc, agf_bp, fab, NULL);
diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index 7251c66a82c9..ec2064ed3c30 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -83,9 +83,6 @@ xchk_fscount_warmup(
error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &agf_bp);
if (error)
break;
- error = -ENOMEM;
- if (!agf_bp || !agi_bp)
- break;
/*
* These are supposed to be initialized by the header read
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 3df49d487940..e489d7a8446a 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -546,8 +546,6 @@ xrep_reap_block(
error = xfs_alloc_read_agf(sc->mp, sc->tp, agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
} else {
agf_bp = sc->sa.agf_bp;
}
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index cae613620175..0b8350e84d28 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -45,7 +45,7 @@ xfs_trim_extents(
xfs_log_force(mp, XFS_LOG_SYNC);
error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
- if (error || !agbp)
+ if (error)
goto out_put_perag;
cur = xfs_allocbt_init_cursor(mp, NULL, agbp, agno, XFS_BTNUM_CNT);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index e723b267a247..b0ce04ffd3cd 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -143,8 +143,6 @@ xfs_reflink_find_shared(
error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
if (error)
return error;
- if (!agbp)
- return -ENOMEM;
cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
` (10 preceding siblings ...)
2020-01-24 5:20 ` [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
@ 2020-01-24 5:20 ` Darrick J. Wong
2020-01-24 23:12 ` Christoph Hellwig
11 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-24 5:20 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Dave Chinner
From: Darrick J. Wong <darrick.wong@oracle.com>
Instead of passing __func__ to the error reporting function, let's use
the return address builtins so that the messages actually tell you which
higher level function called the buffer functions. This was previously
true for the xfs_buf_read callers, but not for the xfs_trans_read_buf
callers.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_buf.c | 12 +++++++-----
fs/xfs/xfs_buf.h | 7 ++++---
fs/xfs/xfs_buf_item.c | 2 +-
fs/xfs/xfs_log_recover.c | 4 ++--
fs/xfs/xfs_trans_buf.c | 5 +++--
5 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index b420e865b32e..217e4f82a44a 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -803,7 +803,8 @@ xfs_buf_read_map(
int nmaps,
xfs_buf_flags_t flags,
struct xfs_buf **bpp,
- const struct xfs_buf_ops *ops)
+ const struct xfs_buf_ops *ops,
+ xfs_failaddr_t fa)
{
struct xfs_buf *bp;
int error;
@@ -852,7 +853,7 @@ xfs_buf_read_map(
*/
if (error) {
if (!XFS_FORCED_SHUTDOWN(target->bt_mount))
- xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_ioerror_alert(bp, fa);
bp->b_flags &= ~XBF_DONE;
xfs_buf_stale(bp);
@@ -885,7 +886,8 @@ xfs_buf_readahead_map(
return;
xfs_buf_read_map(target, map, nmaps,
- XBF_TRYLOCK | XBF_ASYNC | XBF_READ_AHEAD, &bp, ops);
+ XBF_TRYLOCK | XBF_ASYNC | XBF_READ_AHEAD, &bp, ops,
+ __this_address);
}
/*
@@ -1234,10 +1236,10 @@ __xfs_buf_ioerror(
void
xfs_buf_ioerror_alert(
struct xfs_buf *bp,
- const char *func)
+ xfs_failaddr_t func)
{
xfs_alert(bp->b_mount,
-"metadata I/O error in \"%s\" at daddr 0x%llx len %d error %d",
+"metadata I/O error in \"%pS\" at daddr 0x%llx len %d error %d",
func, (uint64_t)XFS_BUF_ADDR(bp), bp->b_length,
-bp->b_error);
}
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index aa145ad25e9a..d79a1fe5d738 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -196,7 +196,7 @@ 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);
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);
+ const struct xfs_buf_ops *ops, xfs_failaddr_t fa);
void xfs_buf_readahead_map(struct xfs_buftarg *target,
struct xfs_buf_map *map, int nmaps,
const struct xfs_buf_ops *ops);
@@ -224,7 +224,8 @@ xfs_buf_read(
{
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_read_map(target, &map, 1, flags, bpp, ops);
+ return xfs_buf_read_map(target, &map, 1, flags, bpp, ops,
+ __builtin_return_address(0));
}
static inline void
@@ -261,7 +262,7 @@ extern void xfs_buf_ioend(struct xfs_buf *bp);
extern void __xfs_buf_ioerror(struct xfs_buf *bp, int error,
xfs_failaddr_t failaddr);
#define xfs_buf_ioerror(bp, err) __xfs_buf_ioerror((bp), (err), __this_address)
-extern void xfs_buf_ioerror_alert(struct xfs_buf *, const char *func);
+extern void xfs_buf_ioerror_alert(struct xfs_buf *bp, xfs_failaddr_t fa);
extern int __xfs_buf_submit(struct xfs_buf *bp, bool);
static inline int xfs_buf_submit(struct xfs_buf *bp)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 5be8973a452c..663810e6cd59 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1113,7 +1113,7 @@ xfs_buf_iodone_callback_error(
if (bp->b_target != lasttarg ||
time_after(jiffies, (lasttime + 5*HZ))) {
lasttime = jiffies;
- xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_ioerror_alert(bp, __this_address);
}
lasttarg = bp->b_target;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index ac79537d3275..25cfc85dbaa7 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -294,7 +294,7 @@ xlog_recover_iodone(
* this during recovery. One strike!
*/
if (!XFS_FORCED_SHUTDOWN(bp->b_mount)) {
- xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_ioerror_alert(bp, __this_address);
xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR);
}
}
@@ -5627,7 +5627,7 @@ xlog_do_recover(
error = xfs_buf_submit(bp);
if (error) {
if (!XFS_FORCED_SHUTDOWN(mp)) {
- xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_ioerror_alert(bp, __this_address);
ASSERT(0);
}
xfs_buf_relse(bp);
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 83470998f87b..08174ffa2118 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -280,7 +280,7 @@ xfs_trans_read_buf_map(
ASSERT(bp->b_ops != NULL);
error = xfs_buf_reverify(bp, ops);
if (error) {
- xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_ioerror_alert(bp, __return_address);
if (tp->t_flags & XFS_TRANS_DIRTY)
xfs_force_shutdown(tp->t_mountp,
@@ -302,7 +302,8 @@ xfs_trans_read_buf_map(
return 0;
}
- error = xfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
+ error = xfs_buf_read_map(target, map, nmaps, flags, &bp, ops,
+ __return_address);
switch (error) {
case 0:
break;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 02/12] xfs: make xfs_buf_get_map return an error code
2020-01-24 5:19 ` [PATCH 02/12] xfs: make xfs_buf_get_map " Darrick J. Wong
@ 2020-01-24 22:02 ` Dave Chinner
2020-01-24 22:59 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Dave Chinner @ 2020-01-24 22:02 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Jan 23, 2020 at 09:19:08PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert xfs_buf_get_map() to return numeric error codes like most
> everywhere else in xfs.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_buf.c | 44 ++++++++++++++++----------------------------
> fs/xfs/xfs_buf.h | 13 +++++++++----
> fs/xfs/xfs_trans_buf.c | 14 +++++++++-----
> 3 files changed, 34 insertions(+), 37 deletions(-)
Looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/12] xfs: make xfs_buf_read_map return an error code
2020-01-24 5:19 ` [PATCH 03/12] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:07 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Dave Chinner @ 2020-01-24 22:05 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Jan 23, 2020 at 09:19:15PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> 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>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 11 ++++---
> fs/xfs/libxfs/xfs_attr_remote.c | 10 ------
> fs/xfs/xfs_buf.c | 63 +++++++++++++++++++++++++++++----------
> fs/xfs/xfs_buf.h | 15 ++++++---
> fs/xfs/xfs_log_recover.c | 10 ------
> fs/xfs/xfs_symlink.c | 10 ------
> fs/xfs/xfs_trans_buf.c | 36 +++++-----------------
> 7 files changed, 72 insertions(+), 83 deletions(-)
Error handling in xfs_trans_read_buf_map() looks ok now.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/12] xfs: make xfs_buf_read return an error code
2020-01-24 5:19 ` [PATCH 06/12] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:08 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Dave Chinner @ 2020-01-24 22:05 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Jan 23, 2020 at 09:19:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert xfs_buf_read() to return numeric error codes like most
> everywhere else in xfs.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr_remote.c | 8 ++++----
> fs/xfs/xfs_buf.h | 10 +++-------
> fs/xfs/xfs_log_recover.c | 16 +++++++---------
> fs/xfs/xfs_symlink.c | 8 ++++----
> 4 files changed, 18 insertions(+), 24 deletions(-)
looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
2020-01-24 5:20 ` [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
@ 2020-01-24 22:07 ` Dave Chinner
2020-01-24 23:12 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Dave Chinner @ 2020-01-24 22:07 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Jan 23, 2020 at 09:20:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor xfs_read_agf and xfs_alloc_read_agf to return EAGAIN if the
> caller passed TRYLOCK and we weren't able to get the lock; and change
> the callers to recognize this.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 36 ++++++++++++++----------------------
> fs/xfs/libxfs/xfs_bmap.c | 11 ++++++-----
> fs/xfs/xfs_filestream.c | 11 +++++------
> 3 files changed, 25 insertions(+), 33 deletions(-)
The alloc flag assert changes make the error handling much simpler
:)
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 02/12] xfs: make xfs_buf_get_map return an error code
2020-01-24 5:19 ` [PATCH 02/12] xfs: make xfs_buf_get_map " Darrick J. Wong
2020-01-24 22:02 ` Dave Chinner
@ 2020-01-24 22:59 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-24 22:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
On Thu, Jan 23, 2020 at 09:19:08PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert xfs_buf_get_map() to return numeric error codes like most
> everywhere else in xfs.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/12] xfs: make xfs_buf_read_map return an error code
2020-01-24 5:19 ` [PATCH 03/12] xfs: make xfs_buf_read_map " Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
@ 2020-01-24 23:07 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-24 23:07 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/12] xfs: make xfs_buf_read return an error code
2020-01-24 5:19 ` [PATCH 06/12] xfs: make xfs_buf_read " Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
@ 2020-01-24 23:08 ` Christoph Hellwig
1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-24 23:08 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
On Thu, Jan 23, 2020 at 09:19:35PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert xfs_buf_read() to return numeric error codes like most
> everywhere else in xfs.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
2020-01-24 5:20 ` [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
2020-01-24 22:07 ` Dave Chinner
@ 2020-01-24 23:12 ` Christoph Hellwig
2020-01-26 2:43 ` Christoph Hellwig
1 sibling, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-24 23:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
On Thu, Jan 23, 2020 at 09:20:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor xfs_read_agf and xfs_alloc_read_agf to return EAGAIN if the
> caller passed TRYLOCK and we weren't able to get the lock; and change
> the callers to recognize this.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 36 ++++++++++++++----------------------
> fs/xfs/libxfs/xfs_bmap.c | 11 ++++++-----
> fs/xfs/xfs_filestream.c | 11 +++++------
> 3 files changed, 25 insertions(+), 33 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 34b65635ee34..d8053bc96c4d 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2502,12 +2502,11 @@ xfs_alloc_fix_freelist(
>
> if (!pag->pagf_init) {
> error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
> - if (error)
> + if (error) {
> + /* Couldn't lock the AGF so skip this AG. */
> + if (error == -EAGAIN)
> + error = 0;
> goto out_no_agbp;
> - if (!pag->pagf_init) {
> - ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
> - ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
> - goto out_agbp_relse;
> }
> }
>
> @@ -2533,11 +2532,10 @@ xfs_alloc_fix_freelist(
> */
> if (!agbp) {
> error = xfs_alloc_read_agf(mp, tp, args->agno, flags, &agbp);
> - if (error)
> - goto out_no_agbp;
> - if (!agbp) {
> - ASSERT(flags & XFS_ALLOC_FLAG_TRYLOCK);
> - ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING));
> + if (error) {
> + /* Couldn't lock the AGF so skip this AG. */
> + if (error == -EAGAIN)
> + error = 0;
> goto out_no_agbp;
> }
> }
> @@ -2768,11 +2766,10 @@ xfs_alloc_pagf_init(
> xfs_buf_t *bp;
> int error;
>
> - if ((error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp)))
> - return error;
> - if (bp)
> + error = xfs_alloc_read_agf(mp, tp, agno, flags, &bp);
> + if (!error)
> xfs_trans_brelse(tp, bp);
> - return 0;
> + return error;
> }
>
> /*
> @@ -2961,12 +2958,6 @@ xfs_read_agf(
> 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;
>
> @@ -2992,14 +2983,15 @@ xfs_alloc_read_agf(
>
> trace_xfs_alloc_read_agf(mp, agno);
>
> + /* We don't support trylock when freeing. */
> + ASSERT((flags & (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK)) !=
> + (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK));
Does this assert really work? Shouldn't it be
ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING) ||
!(flags & XFS_ALLOC_FLAG_TRYLOCK));
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting
2020-01-24 5:20 ` [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
@ 2020-01-24 23:12 ` Christoph Hellwig
0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-24 23:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david, Dave Chinner
On Thu, Jan 23, 2020 at 09:20:16PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Instead of passing __func__ to the error reporting function, let's use
> the return address builtins so that the messages actually tell you which
> higher level function called the buffer functions. This was previously
> true for the xfs_buf_read callers, but not for the xfs_trans_read_buf
> callers.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
2020-01-24 23:12 ` Christoph Hellwig
@ 2020-01-26 2:43 ` Christoph Hellwig
0 siblings, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-26 2:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
On Fri, Jan 24, 2020 at 03:12:07PM -0800, Christoph Hellwig wrote:
> > + /* We don't support trylock when freeing. */
> > + ASSERT((flags & (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK)) !=
> > + (XFS_ALLOC_FLAG_FREEING | XFS_ALLOC_FLAG_TRYLOCK));
>
> Does this assert really work? Shouldn't it be
>
> ASSERT(!(flags & XFS_ALLOC_FLAG_FREEING) ||
> !(flags & XFS_ALLOC_FLAG_TRYLOCK));
Actually, I'll take that back - your version is doing the right thing.
It just isn't quite as obvious to read.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers
2020-01-23 7:43 ` [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
@ 2020-01-24 2:01 ` Dave Chinner
0 siblings, 0 replies; 25+ messages in thread
From: Dave Chinner @ 2020-01-24 2:01 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, Christoph Hellwig
On Wed, Jan 22, 2020 at 11:43:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Drop the null buffer pointer checks in all code that calls
> xfs_alloc_read_agf and doesn't pass XFS_ALLOC_FLAG_TRYLOCK because
> they're no longer necessary.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_refcount.c | 6 ------
> fs/xfs/scrub/agheader_repair.c | 4 ----
> fs/xfs/scrub/fscounters.c | 3 ---
> fs/xfs/scrub/repair.c | 2 --
> fs/xfs/xfs_discard.c | 2 +-
> fs/xfs/xfs_reflink.c | 2 --
> 6 files changed, 1 insertion(+), 18 deletions(-)
looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers
2020-01-23 7:41 [PATCH v4 00/12] xfs: make buffer functions return error codes Darrick J. Wong
@ 2020-01-23 7:43 ` Darrick J. Wong
2020-01-24 2:01 ` Dave Chinner
0 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-23 7:43 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
From: Darrick J. Wong <darrick.wong@oracle.com>
Drop the null buffer pointer checks in all code that calls
xfs_alloc_read_agf and doesn't pass XFS_ALLOC_FLAG_TRYLOCK because
they're no longer necessary.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_refcount.c | 6 ------
| 4 ----
fs/xfs/scrub/fscounters.c | 3 ---
fs/xfs/scrub/repair.c | 2 --
fs/xfs/xfs_discard.c | 2 +-
fs/xfs/xfs_reflink.c | 2 --
6 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index d7d702ee4d1a..6e1665f2cb67 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1177,8 +1177,6 @@ xfs_refcount_finish_one(
XFS_ALLOC_FLAG_FREEING, &agbp);
if (error)
return error;
- if (XFS_IS_CORRUPT(tp->t_mountp, !agbp))
- return -EFSCORRUPTED;
rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
if (!rcur) {
@@ -1718,10 +1716,6 @@ xfs_refcount_recover_cow_leftovers(
error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
if (error)
goto out_trans;
- if (!agbp) {
- error = -ENOMEM;
- goto out_trans;
- }
cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
/* Find all the leftover CoW staging extents. */
--git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 7a1a38b636a9..d5e6db9af434 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -659,8 +659,6 @@ xrep_agfl(
error = xfs_alloc_read_agf(mp, sc->tp, sc->sa.agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
/*
* Make sure we have the AGFL buffer, as scrub might have decided it
@@ -735,8 +733,6 @@ xrep_agi_find_btrees(
error = xfs_alloc_read_agf(mp, sc->tp, sc->sa.agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
/* Find the btree roots. */
error = xrep_find_ag_btree_roots(sc, agf_bp, fab, NULL);
diff --git a/fs/xfs/scrub/fscounters.c b/fs/xfs/scrub/fscounters.c
index 7251c66a82c9..ec2064ed3c30 100644
--- a/fs/xfs/scrub/fscounters.c
+++ b/fs/xfs/scrub/fscounters.c
@@ -83,9 +83,6 @@ xchk_fscount_warmup(
error = xfs_alloc_read_agf(mp, sc->tp, agno, 0, &agf_bp);
if (error)
break;
- error = -ENOMEM;
- if (!agf_bp || !agi_bp)
- break;
/*
* These are supposed to be initialized by the header read
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 3df49d487940..e489d7a8446a 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -546,8 +546,6 @@ xrep_reap_block(
error = xfs_alloc_read_agf(sc->mp, sc->tp, agno, 0, &agf_bp);
if (error)
return error;
- if (!agf_bp)
- return -ENOMEM;
} else {
agf_bp = sc->sa.agf_bp;
}
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index cae613620175..0b8350e84d28 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -45,7 +45,7 @@ xfs_trim_extents(
xfs_log_force(mp, XFS_LOG_SYNC);
error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
- if (error || !agbp)
+ if (error)
goto out_put_perag;
cur = xfs_allocbt_init_cursor(mp, NULL, agbp, agno, XFS_BTNUM_CNT);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index e723b267a247..b0ce04ffd3cd 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -143,8 +143,6 @@ xfs_reflink_find_shared(
error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
if (error)
return error;
- if (!agbp)
- return -ENOMEM;
cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-01-26 2:43 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 5:18 [PATCH v5 00/12] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-24 5:19 ` [PATCH 01/12] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
2020-01-24 5:19 ` [PATCH 02/12] xfs: make xfs_buf_get_map " Darrick J. Wong
2020-01-24 22:02 ` Dave Chinner
2020-01-24 22:59 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 03/12] xfs: make xfs_buf_read_map " Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:07 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 04/12] xfs: make xfs_buf_get " Darrick J. Wong
2020-01-24 5:19 ` [PATCH 05/12] xfs: make xfs_buf_get_uncached " Darrick J. Wong
2020-01-24 5:19 ` [PATCH 06/12] xfs: make xfs_buf_read " Darrick J. Wong
2020-01-24 22:05 ` Dave Chinner
2020-01-24 23:08 ` Christoph Hellwig
2020-01-24 5:19 ` [PATCH 07/12] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
2020-01-24 5:19 ` [PATCH 08/12] xfs: make xfs_trans_get_buf " Darrick J. Wong
2020-01-24 5:19 ` [PATCH 09/12] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
2020-01-24 5:20 ` [PATCH 10/12] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
2020-01-24 22:07 ` Dave Chinner
2020-01-24 23:12 ` Christoph Hellwig
2020-01-26 2:43 ` Christoph Hellwig
2020-01-24 5:20 ` [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
2020-01-24 5:20 ` [PATCH 12/12] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
2020-01-24 23:12 ` Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2020-01-23 7:41 [PATCH v4 00/12] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-23 7:43 ` [PATCH 11/12] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
2020-01-24 2:01 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.