* [PATCH v3 00/13] xfs: make buffer functions return error codes
@ 2020-01-20 22:56 Darrick J. Wong
2020-01-20 22:56 ` [PATCH 01/13] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
` (12 more replies)
0 siblings, 13 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:56 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 refactor the code that handles buffer state
changes and logging after an IO/verifier error so that we handle all of
that from within the buffer functions instead of open-coding that
everywhere.
This has been lightly tested with fstests. Enjoy!
Comments and questions are, as always, welcome.
--D
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/13] xfs: make xfs_buf_alloc return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
@ 2020-01-20 22:56 ` Darrick J. Wong
2020-01-20 22:56 ` [PATCH 02/13] xfs: make xfs_buf_read " Darrick J. Wong
` (11 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:56 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
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] 18+ messages in thread
* [PATCH 02/13] xfs: make xfs_buf_read return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-20 22:56 ` [PATCH 01/13] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
@ 2020-01-20 22:56 ` Darrick J. Wong
2020-01-20 22:56 ` [PATCH 03/13] xfs: make xfs_buf_get " Darrick J. Wong
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:56 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 | 13 +++++++++++--
fs/xfs/xfs_log_recover.c | 16 +++++++---------
fs/xfs/xfs_symlink.c | 8 ++++----
4 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index a266d05df146..d82985571a5f 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 = bp->b_error;
if (error) {
xfs_buf_ioerror_alert(bp, __func__);
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 56e081dd1d96..fb60c36a8a5b 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -213,16 +213,25 @@ xfs_buf_get(
return xfs_buf_get_map(target, &map, 1, 0);
}
-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;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_read_map(target, &map, 1, flags, ops);
+
+ *bpp = NULL;
+ bp = xfs_buf_read_map(target, &map, 1, flags, ops);
+ if (!bp)
+ return -ENOMEM;
+
+ *bpp = bp;
+ return 0;
}
static inline void
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 0d683fb96396..b29806846916 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;
error = bp->b_error;
if (error) {
xfs_buf_ioerror_alert(bp, "xlog_recover_do..(read#1)");
@@ -2950,12 +2950,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;
- }
error = bp->b_error;
if (error) {
xfs_buf_ioerror_alert(bp, "xlog_recover_do..(read#2)");
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index a25502bc2071..4f10d764163b 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;
error = bp->b_error;
if (error) {
xfs_buf_ioerror_alert(bp, __func__);
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/13] xfs: make xfs_buf_get return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-20 22:56 ` [PATCH 01/13] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
2020-01-20 22:56 ` [PATCH 02/13] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-01-20 22:56 ` Darrick J. Wong
2020-01-20 22:56 ` [PATCH 04/13] xfs: make xfs_buf_get_uncached " Darrick J. Wong
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:56 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
fs/xfs/libxfs/xfs_attr_remote.c | 6 +++---
fs/xfs/libxfs/xfs_sb.c | 8 ++++----
fs/xfs/xfs_buf.h | 15 ++++++++++++---
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index d82985571a5f..46f055804433 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -555,9 +555,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 fb60c36a8a5b..fe9ad8ee4eea 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -203,14 +203,23 @@ 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;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_get_map(target, &map, 1, 0);
+
+ *bpp = NULL;
+ bp = xfs_buf_get_map(target, &map, 1, 0);
+ if (!bp)
+ return -ENOMEM;
+
+ *bpp = bp;
+ return 0;
}
static inline int
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/13] xfs: make xfs_buf_get_uncached return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (2 preceding siblings ...)
2020-01-20 22:56 ` [PATCH 03/13] xfs: make xfs_buf_get " Darrick J. Wong
@ 2020-01-20 22:56 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 05/13] xfs: make xfs_buf_read_map " Darrick J. Wong
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:56 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
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 f9a6cf71f4ab..09b8f00d4182 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -883,12 +883,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);
@@ -899,7 +900,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;
}
@@ -908,17 +909,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)
@@ -931,8 +935,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;
@@ -944,7 +950,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)
@@ -954,7 +961,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 fe9ad8ee4eea..eace3e285157 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -254,8 +254,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] 18+ messages in thread
* [PATCH 05/13] xfs: make xfs_buf_read_map return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (3 preceding siblings ...)
2020-01-20 22:56 ` [PATCH 04/13] xfs: make xfs_buf_get_uncached " Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-21 22:52 ` Christoph Hellwig
2020-01-20 22:57 ` [PATCH 06/13] xfs: make xfs_buf_get_map " Darrick J. Wong
` (7 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 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.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 7 +++----
fs/xfs/xfs_buf.c | 18 ++++++++++++------
fs/xfs/xfs_buf.h | 21 ++++++---------------
fs/xfs/xfs_trans_buf.c | 9 +++------
4 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index fc93fd88ec89..df25024275a1 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2956,14 +2956,13 @@ 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);
+ 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/xfs_buf.c b/fs/xfs/xfs_buf.c
index 09b8f00d4182..460821844ee5 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -809,21 +809,23 @@ 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;
flags |= XBF_READ;
+ *bpp = NULL;
bp = xfs_buf_get_map(target, map, nmaps, flags);
if (!bp)
- return NULL;
+ return -ENOMEM;
trace_xfs_buf_read(bp, flags, _RET_IP_);
@@ -831,7 +833,8 @@ xfs_buf_read_map(
XFS_STATS_INC(target->bt_mount, xb_get_read);
bp->b_ops = ops;
_xfs_buf_read(bp, flags);
- return bp;
+ *bpp = bp;
+ return 0;
}
xfs_buf_reverify(bp, ops);
@@ -842,13 +845,14 @@ xfs_buf_read_map(
* drop the buffer
*/
xfs_buf_relse(bp);
- return NULL;
+ return 0;
}
/* 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;
}
/*
@@ -862,11 +866,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 eace3e285157..14209db07684 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -195,13 +195,11 @@ struct xfs_buf *xfs_buf_incore(struct xfs_buftarg *target,
struct xfs_buf *xfs_buf_get_map(struct xfs_buftarg *target,
struct xfs_buf_map *map, int nmaps,
xfs_buf_flags_t flags);
-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);
-void xfs_buf_readahead_map(struct xfs_buftarg *target,
- struct xfs_buf_map *map, int nmaps,
- 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);
static inline int
xfs_buf_get(
@@ -231,16 +229,9 @@ xfs_buf_read(
struct xfs_buf **bpp,
const struct xfs_buf_ops *ops)
{
- struct xfs_buf *bp;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- *bpp = NULL;
- bp = xfs_buf_read_map(target, &map, 1, flags, ops);
- if (!bp)
- return -ENOMEM;
-
- *bpp = bp;
- return 0;
+ return xfs_buf_read_map(target, &map, 1, flags, bpp, ops);
}
static inline void
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index b5b3a78ef31c..a2af6dec310d 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -298,12 +298,9 @@ 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;
- }
+ error = xfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
+ if (error)
+ return error;
/*
* If we've had a read error, then the contents of the buffer are
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/13] xfs: make xfs_buf_get_map return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (4 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 05/13] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 07/13] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_buf.c | 46 +++++++++++++++++-----------------------------
fs/xfs/xfs_buf.h | 14 +++-----------
fs/xfs/xfs_trans_buf.c | 14 +++++++++-----
3 files changed, 29 insertions(+), 45 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 460821844ee5..306394300863 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
@@ -819,13 +806,14 @@ xfs_buf_read_map(
const struct xfs_buf_ops *ops)
{
struct xfs_buf *bp;
+ int error;
flags |= XBF_READ;
*bpp = NULL;
- bp = xfs_buf_get_map(target, map, nmaps, flags);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_get_map(target, map, nmaps, flags, &bp);
+ if (error)
+ return error;
trace_xfs_buf_read(bp, flags, _RET_IP_);
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 14209db07684..d1908a5038a2 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);
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);
@@ -208,16 +207,9 @@ xfs_buf_get(
size_t numblks,
struct xfs_buf **bpp)
{
- struct xfs_buf *bp;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- *bpp = NULL;
- bp = xfs_buf_get_map(target, &map, 1, 0);
- if (!bp)
- return -ENOMEM;
-
- *bpp = bp;
- return 0;
+ return xfs_buf_get_map(target, &map, 1, 0, bpp);
}
static inline int
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index a2af6dec310d..bf61e61b38c7 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] 18+ messages in thread
* [PATCH 07/13] xfs: make xfs_trans_get_buf_map return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (5 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 06/13] xfs: make xfs_buf_get_map " Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 08/13] xfs: make xfs_trans_get_buf " Darrick J. Wong
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
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 bf61e61b38c7..babbfddbe505 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] 18+ messages in thread
* [PATCH 08/13] xfs: make xfs_trans_get_buf return an error code
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (6 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 07/13] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 09/13] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
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 c75840a9e478..eddd5d311b0c 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 4f10d764163b..a283d6d6fb46 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -290,12 +290,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);
@@ -433,13 +431,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] 18+ messages in thread
* [PATCH 09/13] xfs: remove the xfs_btree_get_buf[ls] functions
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (7 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 08/13] xfs: make xfs_trans_get_buf " Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 10/13] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david, Christoph Hellwig
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>
---
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 df25024275a1..7be6c8fbfcf9 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] 18+ messages in thread
* [PATCH 10/13] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (8 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 09/13] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 11/13] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 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 | 34 +++++++++++++++-------------------
fs/xfs/libxfs/xfs_bmap.c | 11 ++++++-----
fs/xfs/xfs_filestream.c | 11 ++++++-----
3 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 7be6c8fbfcf9..ed0172850400 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,8 +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);
- if (error == -EAGAIN)
- return 0;
if (error)
return error;
@@ -2992,10 +2987,11 @@ xfs_alloc_read_agf(
error = xfs_read_agf(mp, tp, agno,
(flags & XFS_ALLOC_FLAG_TRYLOCK) ? XBF_TRYLOCK : 0,
bpp);
- if (error)
+ if (error) {
+ /* We don't support trylock when freeing. */
+ ASSERT(error != -EAGAIN || !(flags & XFS_ALLOC_FLAG_FREEING));
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..3ccdab463359 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -159,16 +159,17 @@ xfs_filestream_pick_ag(
if (!pag->pagf_init) {
err = xfs_alloc_pagf_init(mp, NULL, ag, trylock);
- if (err && !trylock) {
+ if (err == -EAGAIN) {
+ /* Couldn't lock the AGF, skip this AG. */
+ xfs_perag_put(pag);
+ continue;
+ }
+ if (err) {
xfs_perag_put(pag);
return err;
}
}
- /* 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] 18+ messages in thread
* [PATCH 11/13] xfs: remove unnecessary null pointer checks from _read_agf callers
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (9 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 10/13] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 12/13] xfs: move buffer read io error logging to xfs_buf_read_map Darrick J. Wong
2020-01-20 22:57 ` [PATCH 13/13] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 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] 18+ messages in thread
* [PATCH 12/13] xfs: move buffer read io error logging to xfs_buf_read_map
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (10 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 11/13] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 13/13] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
From: Darrick J. Wong <darrick.wong@oracle.com>
Move the open-coded logic that reports metadata IO read / corruption
errors and stales the buffer into xfs_buf_read_map so that it's all
consistent.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr_remote.c | 10 -------
fs/xfs/xfs_buf.c | 53 +++++++++++++++++++++++++++++----------
fs/xfs/xfs_log_recover.c | 10 -------
fs/xfs/xfs_symlink.c | 10 -------
fs/xfs/xfs_trans_buf.c | 34 +++++++------------------
5 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 46f055804433..8b7f74b3bea2 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(
0, &bp, &xfs_attr3_rmt_buf_ops);
if (error)
return error;
- 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 306394300863..c0b4d516fa2c 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -817,28 +817,55 @@ xfs_buf_read_map(
trace_xfs_buf_read(bp, flags, _RET_IP_);
- if (!(bp->b_flags & XBF_DONE)) {
+ if (bp->b_flags & XBF_DONE) {
+ /* Buffer already read; all we need to do is check it. */
+ 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);
+ } else {
+ /* Initiate the buffer read and wait. */
XFS_STATS_INC(target->bt_mount, xb_get_read);
bp->b_ops = ops;
_xfs_buf_read(bp, flags);
- *bpp = bp;
- return 0;
+
+ /* Readahead iodone already dropped the buffer, so exit. */
+ if (flags & XBF_ASYNC)
+ return 0;
}
- 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 (bp->b_error) {
+ error = bp->b_error;
- if (flags & XBF_ASYNC) {
- /*
- * Read ahead call which is already satisfied,
- * drop the buffer
- */
+ if (!XFS_FORCED_SHUTDOWN(target->bt_mount))
+ xfs_buf_ioerror_alert(bp, __func__);
+
+ bp->b_flags &= ~XBF_DONE;
+ xfs_buf_stale(bp);
xfs_buf_relse(bp);
- return 0;
+
+ /* 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);
*bpp = bp;
return 0;
}
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index b29806846916..ac79537d3275 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, &bp, NULL);
if (error)
return error;
- 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
@@ -2954,11 +2949,6 @@ xlog_recover_inode_pass2(
0, &bp, &xfs_inode_buf_ops);
if (error)
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 a283d6d6fb46..d762d42ed0ff 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -57,16 +57,6 @@ xfs_readlink_bmap_ilocked(
&bp, &xfs_symlink_buf_ops);
if (error)
return error;
- 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 babbfddbe505..449be0f8c10f 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -303,32 +303,16 @@ xfs_trans_read_buf_map(
}
error = xfs_buf_read_map(target, map, nmaps, flags, &bp, ops);
- if (error)
- return error;
-
- /*
- * 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);
-
+ switch (error) {
+ case 0:
+ break;
+ case -EFSCORRUPTED:
+ case -EIO:
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;
+ xfs_force_shutdown(tp->t_mountp,
+ SHUTDOWN_META_IO_ERROR);
+ /* fall through */
+ default:
return error;
}
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 13/13] xfs: fix xfs_buf_ioerror_alert location reporting
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
` (11 preceding siblings ...)
2020-01-20 22:57 ` [PATCH 12/13] xfs: move buffer read io error logging to xfs_buf_read_map Darrick J. Wong
@ 2020-01-20 22:57 ` Darrick J. Wong
12 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-20 22:57 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch, david
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>
---
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 c0b4d516fa2c..122e9b3d0cba 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;
@@ -854,7 +855,7 @@ xfs_buf_read_map(
error = bp->b_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);
@@ -887,7 +888,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);
}
/*
@@ -1236,10 +1238,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 d1908a5038a2..567ec2c24244 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);
@@ -223,7 +223,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
@@ -260,7 +261,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 449be0f8c10f..2ffa401ebcc6 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] 18+ messages in thread
* Re: [PATCH 05/13] xfs: make xfs_buf_read_map return an error code
2020-01-20 22:57 ` [PATCH 05/13] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-21 22:52 ` Christoph Hellwig
2020-01-22 0:20 ` Darrick J. Wong
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2020-01-21 22:52 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch, david
On Mon, Jan 20, 2020 at 02:57:01PM -0800, Darrick J. Wong wrote:
> @@ -831,7 +833,8 @@ xfs_buf_read_map(
> XFS_STATS_INC(target->bt_mount, xb_get_read);
> bp->b_ops = ops;
> _xfs_buf_read(bp, flags);
> - return bp;
> + *bpp = bp;
> + return 0;
_xfs_buf_read can return an error, and we are losing that here. So
we should return the value from _xfs_buf_read, an ensure *bpp is NULL
if it returns an error. That also means all the b_error check in the
callers of xfs_buf_read_map and xfs_buf_read (and with that the biggest
wart in the buffer cache API) can go away.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 05/13] xfs: make xfs_buf_read_map return an error code
2020-01-21 22:52 ` Christoph Hellwig
@ 2020-01-22 0:20 ` Darrick J. Wong
2020-01-22 22:12 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-22 0:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, david
On Tue, Jan 21, 2020 at 02:52:28PM -0800, Christoph Hellwig wrote:
> On Mon, Jan 20, 2020 at 02:57:01PM -0800, Darrick J. Wong wrote:
> > @@ -831,7 +833,8 @@ xfs_buf_read_map(
> > XFS_STATS_INC(target->bt_mount, xb_get_read);
> > bp->b_ops = ops;
> > _xfs_buf_read(bp, flags);
> > - return bp;
> > + *bpp = bp;
> > + return 0;
>
> _xfs_buf_read can return an error, and we are losing that here. So
> we should return the value from _xfs_buf_read, an ensure *bpp is NULL
> if it returns an error. That also means all the b_error check in the
> callers of xfs_buf_read_map and xfs_buf_read (and with that the biggest
> wart in the buffer cache API) can go away.
I rearrange responsibility for dealing with buffer error handling in the
patch "xfs: move buffer read io error logging to xfs_buf_read_map" later
in this series. Was that not what you were expecting?
Though looking at that patch I guess we could set @error directly to the
return values of xfs_buf_reverify/_xfs_buf_read.
--D
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 05/13] xfs: make xfs_buf_read_map return an error code
2020-01-22 0:20 ` Darrick J. Wong
@ 2020-01-22 22:12 ` Christoph Hellwig
2020-01-23 7:40 ` Darrick J. Wong
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2020-01-22 22:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs, david
On Tue, Jan 21, 2020 at 04:20:46PM -0800, Darrick J. Wong wrote:
> I rearrange responsibility for dealing with buffer error handling in the
> patch "xfs: move buffer read io error logging to xfs_buf_read_map" later
> in this series. Was that not what you were expecting?
I defintively don't expect a patch talking about logging to change error
handling behavior. And yes, I also expect that if we change a function
to return an error code that is actually uses that to return errors.
> Though looking at that patch I guess we could set @error directly to the
> return values of xfs_buf_reverify/_xfs_buf_read.
Yes. Code outside xfs_buf.c and maybe xfs_buf_item.c really ever access
b_error.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 05/13] xfs: make xfs_buf_read_map return an error code
2020-01-22 22:12 ` Christoph Hellwig
@ 2020-01-23 7:40 ` Darrick J. Wong
0 siblings, 0 replies; 18+ messages in thread
From: Darrick J. Wong @ 2020-01-23 7:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, david
On Wed, Jan 22, 2020 at 02:12:07PM -0800, Christoph Hellwig wrote:
> On Tue, Jan 21, 2020 at 04:20:46PM -0800, Darrick J. Wong wrote:
> > I rearrange responsibility for dealing with buffer error handling in the
> > patch "xfs: move buffer read io error logging to xfs_buf_read_map" later
> > in this series. Was that not what you were expecting?
>
> I defintively don't expect a patch talking about logging to change error
> handling behavior. And yes, I also expect that if we change a function
> to return an error code that is actually uses that to return errors.
Fair enough, I'll fold that one in here.
> > Though looking at that patch I guess we could set @error directly to the
> > return values of xfs_buf_reverify/_xfs_buf_read.
>
> Yes. Code outside xfs_buf.c and maybe xfs_buf_item.c really ever access
> b_error.
<nod>
--D
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-01-23 7:40 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 22:56 [PATCH v3 00/13] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-20 22:56 ` [PATCH 01/13] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
2020-01-20 22:56 ` [PATCH 02/13] xfs: make xfs_buf_read " Darrick J. Wong
2020-01-20 22:56 ` [PATCH 03/13] xfs: make xfs_buf_get " Darrick J. Wong
2020-01-20 22:56 ` [PATCH 04/13] xfs: make xfs_buf_get_uncached " Darrick J. Wong
2020-01-20 22:57 ` [PATCH 05/13] xfs: make xfs_buf_read_map " Darrick J. Wong
2020-01-21 22:52 ` Christoph Hellwig
2020-01-22 0:20 ` Darrick J. Wong
2020-01-22 22:12 ` Christoph Hellwig
2020-01-23 7:40 ` Darrick J. Wong
2020-01-20 22:57 ` [PATCH 06/13] xfs: make xfs_buf_get_map " Darrick J. Wong
2020-01-20 22:57 ` [PATCH 07/13] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
2020-01-20 22:57 ` [PATCH 08/13] xfs: make xfs_trans_get_buf " Darrick J. Wong
2020-01-20 22:57 ` [PATCH 09/13] xfs: remove the xfs_btree_get_buf[ls] functions Darrick J. Wong
2020-01-20 22:57 ` [PATCH 10/13] xfs: make xfs_*read_agf return EAGAIN to ALLOC_FLAG_TRYLOCK callers Darrick J. Wong
2020-01-20 22:57 ` [PATCH 11/13] xfs: remove unnecessary null pointer checks from _read_agf callers Darrick J. Wong
2020-01-20 22:57 ` [PATCH 12/13] xfs: move buffer read io error logging to xfs_buf_read_map Darrick J. Wong
2020-01-20 22:57 ` [PATCH 13/13] xfs: fix xfs_buf_ioerror_alert location reporting Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).