* [PATCH 1/9] xfs: make xfs_buf_alloc return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
@ 2020-01-15 17:03 ` Darrick J. Wong
2020-01-16 16:13 ` Christoph Hellwig
2020-01-15 17:03 ` [PATCH 2/9] xfs: make xfs_buf_read " Darrick J. Wong
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:03 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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>
---
fs/xfs/xfs_buf.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index a0229c368e78..546af31bb375 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -198,12 +198,13 @@ 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;
@@ -211,7 +212,7 @@ _xfs_buf_alloc(
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 +240,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 +257,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 +717,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 (unlikely(error))
return NULL;
error = xfs_buf_allocate_memory(new_bp, flags);
@@ -917,8 +919,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 (unlikely(error))
goto fail;
page_count = PAGE_ALIGN(numblks << BBSHIFT) >> PAGE_SHIFT;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/9] xfs: make xfs_buf_read return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-15 17:03 ` [PATCH 1/9] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
@ 2020-01-15 17:03 ` Darrick J. Wong
2020-01-16 16:15 ` Christoph Hellwig
2020-01-15 17:03 ` [PATCH 3/9] xfs: make xfs_buf_get " Darrick J. Wong
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:03 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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 | 16 +++-------------
fs/xfs/xfs_buf.c | 32 ++++++++++++++++++++++++++++++++
fs/xfs/xfs_buf.h | 14 +++-----------
fs/xfs/xfs_log_recover.c | 26 +++++++-------------------
fs/xfs/xfs_symlink.c | 17 ++++-------------
5 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index a266d05df146..46c516809086 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -418,20 +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 = 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;
+ 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.c b/fs/xfs/xfs_buf.c
index 546af31bb375..e11fc43c52de 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -850,6 +850,38 @@ xfs_buf_read_map(
return bp;
}
+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);
+ bp = xfs_buf_read_map(target, &map, 1, flags, ops);
+ if (!bp)
+ return -ENOMEM;
+ error = bp->b_error;
+ if (error) {
+ xfs_buf_ioerror_alert(bp, __func__);
+ xfs_buf_stale(bp);
+ xfs_buf_relse(bp);
+
+ /* bad CRC means corrupted metadata */
+ if (error == -EFSBADCRC)
+ error = -EFSCORRUPTED;
+ return error;
+ }
+
+ *bpp = bp;
+ return 0;
+}
+
/*
* If we are not low on memory then do the readahead in a deadlock
* safe manner.
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index 56e081dd1d96..f04722554f63 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -213,17 +213,9 @@ xfs_buf_get(
return xfs_buf_get_map(target, &map, 1, 0);
}
-static inline struct xfs_buf *
-xfs_buf_read(
- struct xfs_buftarg *target,
- xfs_daddr_t blkno,
- size_t numblks,
- xfs_buf_flags_t flags,
- const struct xfs_buf_ops *ops)
-{
- DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_read_map(target, &map, 1, flags, ops);
-}
+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);
static inline void
xfs_buf_readahead(
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 0d683fb96396..ac79537d3275 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2745,15 +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 = bp->b_error;
- if (error) {
- xfs_buf_ioerror_alert(bp, "xlog_recover_do..(read#1)");
- goto out_release;
- }
+ 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
@@ -2950,17 +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;
- }
- 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..f4b14569039f 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -53,20 +53,11 @@ 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 = 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;
+ error = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt), 0,
+ &bp, &xfs_symlink_buf_ops);
+ if (error)
goto out;
- }
+
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
* Re: [PATCH 2/9] xfs: make xfs_buf_read return an error code
2020-01-15 17:03 ` [PATCH 2/9] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-01-16 16:15 ` Christoph Hellwig
2020-01-16 22:30 ` Darrick J. Wong
0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:15 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Wed, Jan 15, 2020 at 09:03:52AM -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.
It also does a few more things:
> + error = bp->b_error;
> + if (error) {
> + xfs_buf_ioerror_alert(bp, __func__);
Adds a new call to xfs_buf_ioerror_alert, which exists in most callers.
> + xfs_buf_stale(bp);
> + xfs_buf_relse(bp);
> +
> + /* bad CRC means corrupted metadata */
> + if (error == -EFSBADCRC)
> + error = -EFSCORRUPTED;
.. and it remaps this error value.
Both of which look sensible to me, so with that mentioned in the
commit log:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] xfs: make xfs_buf_read return an error code
2020-01-16 16:15 ` Christoph Hellwig
@ 2020-01-16 22:30 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-16 22:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jan 16, 2020 at 08:15:38AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 15, 2020 at 09:03:52AM -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.
>
> It also does a few more things:
>
> > + error = bp->b_error;
> > + if (error) {
> > + xfs_buf_ioerror_alert(bp, __func__);
>
> Adds a new call to xfs_buf_ioerror_alert, which exists in most callers.
>
> > + xfs_buf_stale(bp);
> > + xfs_buf_relse(bp);
> > +
> > + /* bad CRC means corrupted metadata */
> > + if (error == -EFSBADCRC)
> > + error = -EFSCORRUPTED;
>
> .. and it remaps this error value.
>
> Both of which look sensible to me, so with that mentioned in the
> commit log:
Fixed.
--D
> Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/9] xfs: make xfs_buf_get return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
2020-01-15 17:03 ` [PATCH 1/9] xfs: make xfs_buf_alloc return an error code Darrick J. Wong
2020-01-15 17:03 ` [PATCH 2/9] xfs: make xfs_buf_read " Darrick J. Wong
@ 2020-01-15 17:03 ` Darrick J. Wong
2020-01-16 16:16 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 4/9] xfs: make xfs_buf_get_uncached " Darrick J. Wong
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:03 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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>
---
fs/xfs/libxfs/xfs_attr_remote.c | 6 +++---
fs/xfs/libxfs/xfs_sb.c | 9 +++++----
fs/xfs/xfs_buf.c | 18 ++++++++++++++++++
fs/xfs/xfs_buf.h | 11 ++---------
4 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 46c516809086..8b7f74b3bea2 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..28763c283851 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -985,9 +985,10 @@ 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 +996,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.c b/fs/xfs/xfs_buf.c
index e11fc43c52de..643173cc58f6 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -759,6 +759,24 @@ xfs_buf_get_map(
return bp;
}
+int
+xfs_buf_get(
+ struct xfs_buftarg *target,
+ xfs_daddr_t blkno,
+ size_t numblks,
+ struct xfs_buf **bpp)
+{
+ struct xfs_buf *bp;
+
+ DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
+ bp = xfs_buf_get_map(target, &map, 1, 0);
+ if (!bp)
+ return -ENOMEM;
+
+ *bpp = bp;
+ return 0;
+}
+
STATIC int
_xfs_buf_read(
xfs_buf_t *bp,
diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index f04722554f63..267b35daf662 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -203,15 +203,8 @@ 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 *
-xfs_buf_get(
- struct xfs_buftarg *target,
- xfs_daddr_t blkno,
- size_t numblks)
-{
- DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return xfs_buf_get_map(target, &map, 1, 0);
-}
+int xfs_buf_get(struct xfs_buftarg *target, xfs_daddr_t blkno, size_t numblks,
+ struct xfs_buf **bpp);
int xfs_buf_read(struct xfs_buftarg *target, xfs_daddr_t blkno, size_t numblks,
xfs_buf_flags_t flags, struct xfs_buf **bpp,
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 3/9] xfs: make xfs_buf_get return an error code
2020-01-15 17:03 ` [PATCH 3/9] xfs: make xfs_buf_get " Darrick J. Wong
@ 2020-01-16 16:16 ` Christoph Hellwig
2020-01-16 22:30 ` Darrick J. Wong
0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:16 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Wed, Jan 15, 2020 at 09:03:58AM -0800, Darrick J. Wong wrote:
> - XFS_FSS_TO_BB(mp, 1));
> + XFS_FSS_TO_BB(mp, 1),
> + &bp);
Do we need the extra line here?
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/9] xfs: make xfs_buf_get return an error code
2020-01-16 16:16 ` Christoph Hellwig
@ 2020-01-16 22:30 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-16 22:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jan 16, 2020 at 08:16:42AM -0800, Christoph Hellwig wrote:
> On Wed, Jan 15, 2020 at 09:03:58AM -0800, Darrick J. Wong wrote:
> > - XFS_FSS_TO_BB(mp, 1));
> > + XFS_FSS_TO_BB(mp, 1),
> > + &bp);
>
> Do we need the extra line here?
Whoops, will fix.
--D
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/9] xfs: make xfs_buf_get_uncached return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (2 preceding siblings ...)
2020-01-15 17:03 ` [PATCH 3/9] xfs: make xfs_buf_get " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:17 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 5/9] xfs: make xfs_buf_read_map " Darrick J. Wong
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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>
---
fs/xfs/libxfs/xfs_ag.c | 21 ++++++++++++---------
fs/xfs/xfs_buf.c | 23 ++++++++++++++---------
fs/xfs/xfs_buf.h | 4 ++--
3 files changed, 28 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 643173cc58f6..a7d538b89bb5 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -932,12 +932,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);
@@ -948,7 +949,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;
}
@@ -957,11 +958,12 @@ 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;
@@ -980,8 +982,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;
@@ -993,7 +997,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)
@@ -1003,7 +1008,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 267b35daf662..e0e8b0769758 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -221,8 +221,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 5/9] xfs: make xfs_buf_read_map return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (3 preceding siblings ...)
2020-01-15 17:04 ` [PATCH 4/9] xfs: make xfs_buf_get_uncached " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:21 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 6/9] xfs: make xfs_buf_get_map " Darrick J. Wong
` (3 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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 | 4 ++++
fs/xfs/xfs_buf.c | 24 +++++++++++++++---------
fs/xfs/xfs_buf.h | 12 +++++-------
fs/xfs/xfs_trans_buf.c | 9 +++------
4 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index fc93fd88ec89..53410819691b 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2960,6 +2960,10 @@ xfs_read_agf(
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 && (flags & XBF_TRYLOCK)) {
+ *bpp = NULL;
+ return 0;
+ }
if (error)
return error;
if (!*bpp)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index a7d538b89bb5..1a3dfecb93e0 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -826,12 +826,13 @@ 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;
@@ -840,7 +841,7 @@ xfs_buf_read_map(
bp = xfs_buf_get_map(target, map, nmaps, flags);
if (!bp)
- return NULL;
+ return -ENOMEM;
trace_xfs_buf_read(bp, flags, _RET_IP_);
@@ -848,7 +849,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);
@@ -859,13 +861,15 @@ xfs_buf_read_map(
* drop the buffer
*/
xfs_buf_relse(bp);
- return NULL;
+ *bpp = 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;
}
int
@@ -881,9 +885,9 @@ xfs_buf_read(
int error;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- bp = xfs_buf_read_map(target, &map, 1, flags, ops);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
+ if (error)
+ return error;
error = bp->b_error;
if (error) {
xfs_buf_ioerror_alert(bp, __func__);
@@ -911,11 +915,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 e0e8b0769758..7e01d168e437 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);
int xfs_buf_get(struct xfs_buftarg *target, xfs_daddr_t blkno, size_t numblks,
struct xfs_buf **bpp);
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] 25+ messages in thread
* Re: [PATCH 5/9] xfs: make xfs_buf_read_map return an error code
2020-01-15 17:04 ` [PATCH 5/9] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-16 16:21 ` Christoph Hellwig
2020-01-16 23:26 ` Darrick J. Wong
0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:21 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
> @@ -2960,6 +2960,10 @@ xfs_read_agf(
> 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 && (flags & XBF_TRYLOCK)) {
Given that EAGAIN is only returned in the XBF_TRYLOCK case the check
for XBF_TRYLOCK should not be required.
> + *bpp = NULL;
> + return 0;
Also we should make sure in the lowest layers to never return a
non-NULL bpp if returning an error to avoid this boilerplate code.
> - if (!bp)
> - return -ENOMEM;
> + error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
> + if (error)
> + return error;
> error = bp->b_error;
> if (error) {
> xfs_buf_ioerror_alert(bp, __func__);
The extra checking of bp->b_error shoudn't be required now. That almost
means we might have to move the xfs_buf_ioerror_alert into
xfs_buf_read_map.
That also means xfs_buf_read can be turned into a simple inline
function again.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] xfs: make xfs_buf_read_map return an error code
2020-01-16 16:21 ` Christoph Hellwig
@ 2020-01-16 23:26 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-16 23:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jan 16, 2020 at 08:21:48AM -0800, Christoph Hellwig wrote:
> > @@ -2960,6 +2960,10 @@ xfs_read_agf(
> > 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 && (flags & XBF_TRYLOCK)) {
>
> Given that EAGAIN is only returned in the XBF_TRYLOCK case the check
> for XBF_TRYLOCK should not be required.
Ok.
> > + *bpp = NULL;
> > + return 0;
>
> Also we should make sure in the lowest layers to never return a
> non-NULL bpp if returning an error to avoid this boilerplate code.
<nod>
At some point I was planning to audit the ALLOC_TRYLOCK cases to make
sure that they can handle an EAGAIN, so we can get rid of this chunk
entirely.
> > - if (!bp)
> > - return -ENOMEM;
> > + error = xfs_buf_read_map(target, &map, 1, flags, &bp, ops);
> > + if (error)
> > + return error;
> > error = bp->b_error;
> > if (error) {
> > xfs_buf_ioerror_alert(bp, __func__);
>
> The extra checking of bp->b_error shoudn't be required now. That almost
> means we might have to move the xfs_buf_ioerror_alert into
> xfs_buf_read_map.
>
> That also means xfs_buf_read can be turned into a simple inline
> function again.
Yeah, I'll add another patch to factor that out.
--D
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/9] xfs: make xfs_buf_get_map return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (4 preceding siblings ...)
2020-01-15 17:04 ` [PATCH 5/9] xfs: make xfs_buf_read_map " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:28 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 7/9] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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 | 45 ++++++++++++++++++++++-----------------------
fs/xfs/xfs_buf.h | 5 ++---
fs/xfs/xfs_trans_buf.c | 14 +++++++++-----
3 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 1a3dfecb93e0..d974954b4fc9 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -684,53 +684,49 @@ 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;
error = xfs_buf_find(target, map, nmaps, flags, NULL, &bp);
-
switch (error) {
case 0:
/* cache hit */
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 -EAGAIN:
+ /* cache hit, trylock failure, caller handles failure */
+ ASSERT(flags & XBF_TRYLOCK);
+ /* fall through */
case -EFSCORRUPTED:
default:
- /*
- * None of the higher layers understand failure types
- * yet, so return NULL to signal a fatal lookup error.
- */
- return NULL;
+ return error;
}
error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
if (unlikely(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)
@@ -743,7 +739,7 @@ xfs_buf_get_map(
xfs_warn(target->bt_mount,
"%s: failed to map pagesn", __func__);
xfs_buf_relse(bp);
- return NULL;
+ return error;
}
}
@@ -756,7 +752,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;
}
int
@@ -767,11 +764,12 @@ xfs_buf_get(
struct xfs_buf **bpp)
{
struct xfs_buf *bp;
+ int error;
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- bp = xfs_buf_get_map(target, &map, 1, 0);
- if (!bp)
- return -ENOMEM;
+ error = xfs_buf_get_map(target, &map, 1, 0, &bp);
+ if (error)
+ return error;
*bpp = bp;
return 0;
@@ -836,12 +834,13 @@ 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)
- 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 7e01d168e437..8702e9099468 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);
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] 25+ messages in thread
* Re: [PATCH 6/9] xfs: make xfs_buf_get_map return an error code
2020-01-15 17:04 ` [PATCH 6/9] xfs: make xfs_buf_get_map " Darrick J. Wong
@ 2020-01-16 16:28 ` Christoph Hellwig
2020-01-16 22:33 ` Darrick J. Wong
0 siblings, 1 reply; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:28 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
> error = xfs_buf_find(target, map, nmaps, flags, NULL, &bp);
> -
> switch (error) {
> case 0:
> /* cache hit */
> 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 -EAGAIN:
> + /* cache hit, trylock failure, caller handles failure */
> + ASSERT(flags & XBF_TRYLOCK);
> + /* fall through */
> case -EFSCORRUPTED:
> default:
> - /*
> - * None of the higher layers understand failure types
> - * yet, so return NULL to signal a fatal lookup error.
> - */
> - return NULL;
> + return error;
I think two little if statements would be cleaner with two ifs instead
of the switch statement:
if (!error)
goto found;
if (error != -ENOENT)
return error;
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/9] xfs: make xfs_buf_get_map return an error code
2020-01-16 16:28 ` Christoph Hellwig
@ 2020-01-16 22:33 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-16 22:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jan 16, 2020 at 08:28:56AM -0800, Christoph Hellwig wrote:
> > error = xfs_buf_find(target, map, nmaps, flags, NULL, &bp);
> > -
> > switch (error) {
> > case 0:
> > /* cache hit */
> > 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 -EAGAIN:
> > + /* cache hit, trylock failure, caller handles failure */
> > + ASSERT(flags & XBF_TRYLOCK);
> > + /* fall through */
> > case -EFSCORRUPTED:
> > default:
> > - /*
> > - * None of the higher layers understand failure types
> > - * yet, so return NULL to signal a fatal lookup error.
> > - */
> > - return NULL;
> > + return error;
>
> I think two little if statements would be cleaner with two ifs instead
> of the switch statement:
>
> if (!error)
> goto found;
> if (error != -ENOENT)
> return error;
>
> Otherwise looks good:
Will fix.
--D
> Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/9] xfs: make xfs_trans_get_buf_map return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (5 preceding siblings ...)
2020-01-15 17:04 ` [PATCH 6/9] xfs: make xfs_buf_get_map " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:29 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 8/9] xfs: make xfs_trans_get_buf " Darrick J. Wong
2020-01-15 17:04 ` [PATCH 9/9] xfs: make xfs_btree_get_buf functions " Darrick J. Wong
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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>
---
fs/xfs/libxfs/xfs_da_btree.c | 8 ++------
fs/xfs/xfs_trans.h | 15 ++++++++++-----
fs/xfs/xfs_trans_buf.c | 21 ++++++++++-----------
3 files changed, 22 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..fda8b39f6c3c 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -112,24 +112,21 @@ 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;
- }
+ 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 +148,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 8/9] xfs: make xfs_trans_get_buf return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (6 preceding siblings ...)
2020-01-15 17:04 ` [PATCH 7/9] xfs: make xfs_trans_get_buf_map " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:31 ` Christoph Hellwig
2020-01-15 17:04 ` [PATCH 9/9] xfs: make xfs_btree_get_buf functions " Darrick J. Wong
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
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>
---
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 28763c283851..d671dd4b80c2 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1186,13 +1186,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 27cb6bf614c5..4dd0c8b4d0a6 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -207,11 +207,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);
@@ -300,10 +301,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 f4b14569039f..2c879d0948e6 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -281,12 +281,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);
@@ -424,13 +422,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 9/9] xfs: make xfs_btree_get_buf functions return an error code
2020-01-15 17:03 [PATCH 0/9] xfs: make buffer functions return error codes Darrick J. Wong
` (7 preceding siblings ...)
2020-01-15 17:04 ` [PATCH 8/9] xfs: make xfs_trans_get_buf " Darrick J. Wong
@ 2020-01-15 17:04 ` Darrick J. Wong
2020-01-16 16:33 ` Christoph Hellwig
8 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-15 17:04 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert both xfs_btree_get_buf() functions 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 | 13 ++++++-------
fs/xfs/libxfs/xfs_bmap.c | 10 +++++-----
fs/xfs/libxfs/xfs_btree.c | 36 ++++++++++++++----------------------
fs/xfs/libxfs/xfs_btree.h | 16 +++++-----------
4 files changed, 30 insertions(+), 45 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 53410819691b..542f6ad7e5b4 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1070,11 +1070,10 @@ 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_btree_get_bufs(args->mp, args->tp, args->agno,
+ fbno, &bp);
+ if (error)
goto error;
- }
xfs_trans_binval(args->tp, bp);
}
*fbnop = args->agbno = fbno;
@@ -2347,9 +2346,9 @@ 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_btree_get_bufs(tp->t_mountp, tp, agno, agbno, &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..4544732d09a5 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -730,11 +730,9 @@ 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_btree_get_bufl(mp, tp, args.fsbno, &abp);
+ if (error)
goto out_unreserve_dquot;
- }
/*
* Fill in the child block.
@@ -878,7 +876,9 @@ 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_btree_get_bufl(args.mp, tp, args.fsbno, &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..0a7e2265dec1 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -682,46 +682,38 @@ xfs_btree_get_block(
* Get a buffer for the block, return it with no data read.
* Long-form addressing.
*/
-xfs_buf_t * /* buffer for fsbno */
+int
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_mount *mp,
+ struct xfs_trans *tp,
+ xfs_fsblock_t fsbno,
+ struct xfs_buf **bpp)
{
- 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;
+ return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
}
/*
* Get a buffer for the block, return it with no data read.
* Short-form addressing.
*/
-xfs_buf_t * /* buffer for agno/agbno */
+int
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_mount *mp,
+ struct xfs_trans *tp,
+ xfs_agnumber_t agno,
+ xfs_agblock_t agbno,
+ struct xfs_buf **bpp)
{
- struct xfs_buf *bp;
- xfs_daddr_t d; /* real disk block address */
- int error;
+ xfs_daddr_t d;
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;
+ return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
}
/*
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index fb9b2121c628..97c2b5e75210 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -300,22 +300,16 @@ xfs_btree_dup_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 */
+int xfs_btree_get_bufl(struct xfs_mount *mp, struct xfs_trans *tp,
+ xfs_fsblock_t fsbno, struct xfs_buf **bpp);
/*
* 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 */
+int xfs_btree_get_bufs(struct xfs_mount *mp, struct xfs_trans *tp,
+ xfs_agnumber_t agno, xfs_agblock_t agbno,
+ struct xfs_buf **bpp);
/*
* Compute first and last byte offsets for the fields given.
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 9/9] xfs: make xfs_btree_get_buf functions return an error code
2020-01-15 17:04 ` [PATCH 9/9] xfs: make xfs_btree_get_buf functions " Darrick J. Wong
@ 2020-01-16 16:33 ` Christoph Hellwig
2020-01-16 16:43 ` Christoph Hellwig
2020-01-16 22:40 ` Darrick J. Wong
0 siblings, 2 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
> - if (XFS_IS_CORRUPT(args->mp, !bp)) {
> - error = -EFSCORRUPTED;
> + error = xfs_btree_get_bufs(args->mp, args->tp, args->agno,
> + fbno, &bp);
> + if (error)
Should we keep the XFS_IS_CORRUPT checks in some form? Not sure they
matter all that much, though.
> 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;
> + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
Maybe kill the local variable while you're at it?
> 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;
> + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
Same here.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 9/9] xfs: make xfs_btree_get_buf functions return an error code
2020-01-16 16:33 ` Christoph Hellwig
@ 2020-01-16 16:43 ` Christoph Hellwig
2020-01-16 22:40 ` Darrick J. Wong
1 sibling, 0 replies; 25+ messages in thread
From: Christoph Hellwig @ 2020-01-16 16:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Jan 16, 2020 at 08:33:55AM -0800, Christoph Hellwig wrote:
> > 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;
> > + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
>
> Maybe kill the local variable while you're at it?
Or maybe just kill xfs_btree_get_bufl and xfs_btree_get_bufs? They
are completely trivial now, and each one just has two callers.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 9/9] xfs: make xfs_btree_get_buf functions return an error code
2020-01-16 16:33 ` Christoph Hellwig
2020-01-16 16:43 ` Christoph Hellwig
@ 2020-01-16 22:40 ` Darrick J. Wong
1 sibling, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2020-01-16 22:40 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Jan 16, 2020 at 08:33:55AM -0800, Christoph Hellwig wrote:
> > - if (XFS_IS_CORRUPT(args->mp, !bp)) {
> > - error = -EFSCORRUPTED;
> > + error = xfs_btree_get_bufs(args->mp, args->tp, args->agno,
> > + fbno, &bp);
> > + if (error)
>
> Should we keep the XFS_IS_CORRUPT checks in some form? Not sure they
> matter all that much, though.
The XFS_IS_CORRUPT checks exist to report a corrupted filesystem, and
the only errors that the _get_buf* variants can return are runtime
errors like ENOMEM.
> > 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;
> > + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
>
> Maybe kill the local variable while you're at it?
>
> > 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;
> > + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
>
> Same here.
Will just get rid of them as you suggest later in this thread.
--D
^ permalink raw reply [flat|nested] 25+ messages in thread