* [PATCH v3 0/2] xfs: refactor corruption checking and reporting
@ 2019-11-08 7:05 Darrick J. Wong
2019-11-08 7:05 ` [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate Darrick J. Wong
2019-11-08 7:05 ` [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT Darrick J. Wong
0 siblings, 2 replies; 9+ messages in thread
From: Darrick J. Wong @ 2019-11-08 7:05 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
Hi all,
In this series, we finish converting various forms of metadata
corruption checking to use the new XFS_IS_CORRUPT macro. The first
patch refactors a predicate function in preparation for the second
patch, which does all the converting.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This has been lightly tested with fstests. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=refactor-corruption-checks
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=refactor-corruption-checks
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate
2019-11-08 7:05 [PATCH v3 0/2] xfs: refactor corruption checking and reporting Darrick J. Wong
@ 2019-11-08 7:05 ` Darrick J. Wong
2019-11-08 7:15 ` Christoph Hellwig
2019-11-08 7:05 ` [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT Darrick J. Wong
1 sibling, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2019-11-08 7:05 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Replace the open-coded checks for whether or not an inode fork maps
blocks with a macro that will implant the code for us. This helps us
declutter the bmap code a bit.
Note that I had to use a macro instead of a static inline function
because of C header dependency problems between xfs_inode.h and
xfs_inode_fork.h.
Conversion was performed with the following Coccinelle script:
@@
expression ip, w;
@@
- XFS_IFORK_FORMAT(ip, w) == XFS_DINODE_FMT_EXTENTS || XFS_IFORK_FORMAT(ip, w) == XFS_DINODE_FMT_BTREE
+ xfs_ifork_has_extents(ip, w)
@@
expression ip, w;
@@
- XFS_IFORK_FORMAT(ip, w) != XFS_DINODE_FMT_EXTENTS && XFS_IFORK_FORMAT(ip, w) != XFS_DINODE_FMT_BTREE
+ !xfs_ifork_has_extents(ip, w)
@@
expression ip, w;
@@
- XFS_IFORK_FORMAT(ip, w) == XFS_DINODE_FMT_BTREE || XFS_IFORK_FORMAT(ip, w) == XFS_DINODE_FMT_EXTENTS
+ xfs_ifork_has_extents(ip, w)
@@
expression ip, w;
@@
- XFS_IFORK_FORMAT(ip, w) != XFS_DINODE_FMT_BTREE && XFS_IFORK_FORMAT(ip, w) != XFS_DINODE_FMT_EXTENTS
+ !xfs_ifork_has_extents(ip, w)
@@
expression ip, w;
@@
- (xfs_ifork_has_extents(ip, w))
+ xfs_ifork_has_extents(ip, w)
@@
expression ip, w;
@@
- (!xfs_ifork_has_extents(ip, w))
+ !xfs_ifork_has_extents(ip, w)
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_bmap.c | 28 +++++++++-------------------
fs/xfs/libxfs/xfs_inode_fork.h | 4 ++++
fs/xfs/scrub/dabtree.c | 3 +--
fs/xfs/xfs_iomap.c | 3 +--
4 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index d08bdb066555..97b6a1fd3246 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1288,8 +1288,7 @@ xfs_bmap_first_unused(
xfs_fileoff_t lowest, max;
int error;
- ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE ||
- XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS ||
+ ASSERT(xfs_ifork_has_extents(ip, whichfork) ||
XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
@@ -1445,8 +1444,7 @@ xfs_bmap_last_offset(
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL)
return 0;
- if (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS) {
+ if (!xfs_ifork_has_extents(ip, whichfork)) {
ASSERT(0);
return -EFSCORRUPTED;
}
@@ -3909,8 +3907,7 @@ xfs_bmapi_read(
ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL));
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmapi_read", XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
@@ -4421,8 +4418,7 @@ xfs_bmapi_write(
(XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO));
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
@@ -4691,8 +4687,7 @@ xfs_bmapi_remap(
(XFS_BMAPI_ATTRFORK | XFS_BMAPI_PREALLOC));
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmapi_remap", XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
@@ -5333,9 +5328,7 @@ __xfs_bunmapi(
whichfork = xfs_bmapi_whichfork(flags);
ASSERT(whichfork != XFS_COW_FORK);
ifp = XFS_IFORK_PTR(ip, whichfork);
- if (unlikely(
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
+ if (unlikely(!xfs_ifork_has_extents(ip, whichfork))) {
XFS_ERROR_REPORT("xfs_bunmapi", XFS_ERRLEVEL_LOW,
ip->i_mount);
return -EFSCORRUPTED;
@@ -5834,8 +5827,7 @@ xfs_bmap_collapse_extents(
int logflags = 0;
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
@@ -5954,8 +5946,7 @@ xfs_bmap_insert_extents(
int logflags = 0;
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
@@ -6063,8 +6054,7 @@ xfs_bmap_split_extent_at(
int i = 0;
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, whichfork),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmap_split_extent_at",
XFS_ERRLEVEL_LOW, mp);
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 7b845c052fb4..500333d0101e 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -87,6 +87,10 @@ struct xfs_ifork {
#define XFS_IFORK_MAXEXT(ip, w) \
(XFS_IFORK_SIZE(ip, w) / sizeof(xfs_bmbt_rec_t))
+#define xfs_ifork_has_extents(ip, w) \
+ (XFS_IFORK_FORMAT((ip), (w)) == XFS_DINODE_FMT_EXTENTS || \
+ XFS_IFORK_FORMAT((ip), (w)) == XFS_DINODE_FMT_BTREE)
+
struct xfs_ifork *xfs_iext_state_to_fork(struct xfs_inode *ip, int state);
int xfs_iformat_fork(struct xfs_inode *, struct xfs_dinode *);
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index 77ff9f97bcda..ff30429d6e51 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -485,8 +485,7 @@ xchk_da_btree(
int error;
/* Skip short format data structures; no btree to scan. */
- if (XFS_IFORK_FORMAT(sc->ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(sc->ip, whichfork) != XFS_DINODE_FMT_BTREE)
+ if (!xfs_ifork_has_extents(sc->ip, whichfork))
return 0;
/* Set up initial da state. */
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 153262c76051..1471bcd6cb70 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -847,8 +847,7 @@ xfs_buffered_write_iomap_begin(
xfs_ilock(ip, XFS_ILOCK_EXCL);
if (unlikely(XFS_TEST_ERROR(
- (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
- XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
+ !xfs_ifork_has_extents(ip, XFS_DATA_FORK),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-08 7:05 [PATCH v3 0/2] xfs: refactor corruption checking and reporting Darrick J. Wong
2019-11-08 7:05 ` [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate Darrick J. Wong
@ 2019-11-08 7:05 ` Darrick J. Wong
2019-11-08 7:21 ` Christoph Hellwig
2019-11-09 22:32 ` Dave Chinner
1 sibling, 2 replies; 9+ messages in thread
From: Darrick J. Wong @ 2019-11-08 7:05 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Convert the last of the open coded corruption check and report idioms to
use the XFS_IS_CORRUPT macro.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_alloc.c | 7 +---
fs/xfs/libxfs/xfs_bmap.c | 67 ++++++++++++++---------------------------
fs/xfs/libxfs/xfs_btree.c | 14 +++------
fs/xfs/libxfs/xfs_da_btree.c | 46 ++++++++--------------------
fs/xfs/libxfs/xfs_dir2.c | 9 ++----
fs/xfs/libxfs/xfs_dir2_node.c | 9 ++----
fs/xfs/libxfs/xfs_refcount.c | 9 ++----
fs/xfs/libxfs/xfs_rmap.c | 2 +
fs/xfs/libxfs/xfs_rtbitmap.c | 4 +-
fs/xfs/xfs_attr_list.c | 21 ++++---------
fs/xfs/xfs_dir2_readdir.c | 16 ++++------
fs/xfs/xfs_iomap.c | 6 +---
fs/xfs/xfs_iops.c | 4 +-
fs/xfs/xfs_log_recover.c | 64 +++++++++++++--------------------------
fs/xfs/xfs_mount.c | 7 +---
fs/xfs/xfs_qm.c | 12 ++-----
16 files changed, 100 insertions(+), 197 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 65d870189548..d3764de08f5f 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1066,8 +1066,7 @@ xfs_alloc_ag_vextent_small(
struct xfs_buf *bp;
bp = xfs_btree_get_bufs(args->mp, args->tp, args->agno, fbno);
- if (!bp) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, args->mp);
+ if (XFS_IS_CORRUPT(args->mp, !bp)) {
error = -EFSCORRUPTED;
goto error;
}
@@ -2334,10 +2333,8 @@ xfs_free_agfl_block(
return error;
bp = xfs_btree_get_bufs(tp->t_mountp, tp, agno, agbno);
- if (!bp) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, tp->t_mountp);
+ if (XFS_IS_CORRUPT(tp->t_mountp, !bp))
return -EFSCORRUPTED;
- }
xfs_trans_binval(tp, bp);
return 0;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 97b6a1fd3246..f11f4d25c56b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -731,8 +731,7 @@ xfs_bmap_extents_to_btree(
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 (!abp) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !abp)) {
error = -EFSCORRUPTED;
goto out_unreserve_dquot;
}
@@ -1090,8 +1089,7 @@ xfs_bmap_add_attrfork(
goto trans_cancel;
if (XFS_IFORK_Q(ip))
goto trans_cancel;
- if (ip->i_d.di_anextents != 0) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, ip->i_d.di_anextents != 0)) {
error = -EFSCORRUPTED;
goto trans_cancel;
}
@@ -1238,8 +1236,8 @@ xfs_iread_extents(
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp,
+ XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
error = -EFSCORRUPTED;
goto out;
}
@@ -1253,8 +1251,8 @@ xfs_iread_extents(
if (error)
goto out;
- if (ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp,
+ ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
error = -EFSCORRUPTED;
goto out;
}
@@ -1444,10 +1442,8 @@ xfs_bmap_last_offset(
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL)
return 0;
- if (!xfs_ifork_has_extents(ip, whichfork)) {
- ASSERT(0);
+ if (XFS_IS_CORRUPT(ip->i_mount, !xfs_ifork_has_extents(ip, whichfork)))
return -EFSCORRUPTED;
- }
error = xfs_bmap_last_extent(NULL, ip, whichfork, &rec, &is_empty);
if (error || is_empty)
@@ -3906,10 +3902,8 @@ xfs_bmapi_read(
XFS_BMAPI_COWFORK)));
ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL));
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT("xfs_bmapi_read", XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
@@ -4417,10 +4411,8 @@ xfs_bmapi_write(
ASSERT((flags & (XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO)) !=
(XFS_BMAPI_PREALLOC | XFS_BMAPI_ZERO));
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
@@ -4686,10 +4678,8 @@ xfs_bmapi_remap(
ASSERT((flags & (XFS_BMAPI_ATTRFORK | XFS_BMAPI_PREALLOC)) !=
(XFS_BMAPI_ATTRFORK | XFS_BMAPI_PREALLOC));
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT("xfs_bmapi_remap", XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
@@ -5311,7 +5301,7 @@ __xfs_bunmapi(
int isrt; /* freeing in rt area */
int logflags; /* transaction logging flags */
xfs_extlen_t mod; /* rt extent offset */
- struct xfs_mount *mp; /* mount structure */
+ struct xfs_mount *mp = ip->i_mount;
int tmp_logflags; /* partial logging flags */
int wasdel; /* was a delayed alloc extent */
int whichfork; /* data or attribute fork */
@@ -5328,12 +5318,8 @@ __xfs_bunmapi(
whichfork = xfs_bmapi_whichfork(flags);
ASSERT(whichfork != XFS_COW_FORK);
ifp = XFS_IFORK_PTR(ip, whichfork);
- if (unlikely(!xfs_ifork_has_extents(ip, whichfork))) {
- XFS_ERROR_REPORT("xfs_bunmapi", XFS_ERRLEVEL_LOW,
- ip->i_mount);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)))
return -EFSCORRUPTED;
- }
- mp = ip->i_mount;
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
@@ -5826,10 +5812,8 @@ xfs_bmap_collapse_extents(
int error = 0;
int logflags = 0;
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
@@ -5945,10 +5929,8 @@ xfs_bmap_insert_extents(
int error = 0;
int logflags = 0;
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
@@ -5986,8 +5968,8 @@ xfs_bmap_insert_extents(
goto del_cursor;
}
- if (stop_fsb >= got.br_startoff + got.br_blockcount) {
- ASSERT(0);
+ if (XFS_IS_CORRUPT(mp,
+ stop_fsb >= got.br_startoff + got.br_blockcount)) {
error = -EFSCORRUPTED;
goto del_cursor;
}
@@ -6053,11 +6035,8 @@ xfs_bmap_split_extent_at(
int logflags = 0;
int i = 0;
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, whichfork),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT("xfs_bmap_split_extent_at",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, whichfork)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
return -EFSCORRUPTED;
}
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 37b4fa93085c..962020574a2d 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -105,11 +105,10 @@ xfs_btree_check_lblock(
xfs_failaddr_t fa;
fa = __xfs_btree_check_lblock(cur, block, level, bp);
- if (unlikely(XFS_TEST_ERROR(fa != NULL, mp,
- XFS_ERRTAG_BTREE_CHECK_LBLOCK))) {
+ if (XFS_IS_CORRUPT(mp, fa != NULL) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BTREE_CHECK_LBLOCK)) {
if (bp)
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
return 0;
@@ -169,11 +168,10 @@ xfs_btree_check_sblock(
xfs_failaddr_t fa;
fa = __xfs_btree_check_sblock(cur, block, level, bp);
- if (unlikely(XFS_TEST_ERROR(fa != NULL, mp,
- XFS_ERRTAG_BTREE_CHECK_SBLOCK))) {
+ if (XFS_IS_CORRUPT(mp, fa != NULL) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BTREE_CHECK_SBLOCK)) {
if (bp)
trace_xfs_btree_corrupt(bp, _RET_IP_);
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
return 0;
@@ -1873,10 +1871,8 @@ xfs_btree_lookup(
XFS_BTREE_STATS_INC(cur, lookup);
/* No such thing as a zero-level tree. */
- if (cur->bc_nlevels == 0) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, cur->bc_mp);
+ if (XFS_IS_CORRUPT(cur->bc_mp, cur->bc_nlevels == 0))
return -EFSCORRUPTED;
- }
block = NULL;
keyno = 0;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 9925d4a7dc33..588d286935f9 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -1619,17 +1619,12 @@ xfs_da3_node_lookup_int(
}
/* We can't point back to the root. */
- if (blkno == args->geo->leafblk) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- dp->i_mount);
+ if (XFS_IS_CORRUPT(dp->i_mount, blkno == args->geo->leafblk))
return -EFSCORRUPTED;
- }
}
- if (expected_level != 0) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, dp->i_mount);
+ if (XFS_IS_CORRUPT(dp->i_mount, expected_level != 0))
return -EFSCORRUPTED;
- }
/*
* A leaf block that ends in the hashval that we are interested in
@@ -2225,11 +2220,8 @@ xfs_da3_swap_lastblock(
error = xfs_bmap_last_before(tp, dp, &lastoff, w);
if (error)
return error;
- if (unlikely(lastoff == 0)) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(1)", XFS_ERRLEVEL_LOW,
- mp);
+ if (XFS_IS_CORRUPT(mp, lastoff == 0))
return -EFSCORRUPTED;
- }
/*
* Read the last block in the btree space.
*/
@@ -2274,11 +2266,9 @@ xfs_da3_swap_lastblock(
if (error)
goto done;
sib_info = sib_buf->b_addr;
- if (unlikely(
+ if (XFS_IS_CORRUPT(mp,
be32_to_cpu(sib_info->forw) != last_blkno ||
sib_info->magic != dead_info->magic)) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(2)",
- XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
goto done;
}
@@ -2296,11 +2286,9 @@ xfs_da3_swap_lastblock(
if (error)
goto done;
sib_info = sib_buf->b_addr;
- if (unlikely(
- be32_to_cpu(sib_info->back) != last_blkno ||
- sib_info->magic != dead_info->magic)) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp,
+ be32_to_cpu(sib_info->back) != last_blkno ||
+ sib_info->magic != dead_info->magic)) {
error = -EFSCORRUPTED;
goto done;
}
@@ -2321,9 +2309,8 @@ xfs_da3_swap_lastblock(
goto done;
par_node = par_buf->b_addr;
dp->d_ops->node_hdr_from_disk(&par_hdr, par_node);
- if (level >= 0 && level != par_hdr.level + 1) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(4)",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp,
+ level >= 0 && level != par_hdr.level + 1)) {
error = -EFSCORRUPTED;
goto done;
}
@@ -2334,9 +2321,7 @@ xfs_da3_swap_lastblock(
be32_to_cpu(btree[entno].hashval) < dead_hash;
entno++)
continue;
- if (entno == par_hdr.count) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(5)",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, entno == par_hdr.count)) {
error = -EFSCORRUPTED;
goto done;
}
@@ -2361,9 +2346,7 @@ xfs_da3_swap_lastblock(
par_blkno = par_hdr.forw;
xfs_trans_brelse(tp, par_buf);
par_buf = NULL;
- if (unlikely(par_blkno == 0)) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(6)",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, par_blkno == 0)) {
error = -EFSCORRUPTED;
goto done;
}
@@ -2372,9 +2355,7 @@ xfs_da3_swap_lastblock(
goto done;
par_node = par_buf->b_addr;
dp->d_ops->node_hdr_from_disk(&par_hdr, par_node);
- if (par_hdr.level != level) {
- XFS_ERROR_REPORT("xfs_da_swap_lastblock(7)",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, par_hdr.level != level)) {
error = -EFSCORRUPTED;
goto done;
}
@@ -2568,7 +2549,7 @@ xfs_dabuf_map(
if (!xfs_da_map_covers_blocks(nirecs, irecs, bno, nfsb)) {
/* Caller ok with no mapping. */
- if (mappedbno == -2) {
+ if (!XFS_IS_CORRUPT(mp, mappedbno != -2)) {
error = -1;
goto out;
}
@@ -2589,7 +2570,6 @@ xfs_dabuf_map(
irecs[i].br_state);
}
}
- XFS_ERROR_REPORT("xfs_da_do_buf(1)", XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
goto out;
}
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 452d04ae10ce..86eec7ad920b 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -191,10 +191,10 @@ xfs_dir_ino_validate(
{
bool ino_ok = xfs_verify_dir_ino(mp, ino);
- if (unlikely(XFS_TEST_ERROR(!ino_ok, mp, XFS_ERRTAG_DIR_INO_VALIDATE))) {
+ if (XFS_IS_CORRUPT(mp, !ino_ok) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_DIR_INO_VALIDATE)) {
xfs_warn(mp, "Invalid inode number 0x%Lx",
(unsigned long long) ino);
- XFS_ERROR_REPORT("xfs_dir_ino_validate", XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
return 0;
@@ -600,10 +600,9 @@ xfs_dir2_isblock(
if ((rval = xfs_bmap_last_offset(args->dp, &last, XFS_DATA_FORK)))
return rval;
rval = XFS_FSB_TO_B(args->dp->i_mount, last) == args->geo->blksize;
- if (rval != 0 && args->dp->i_d.di_size != args->geo->blksize) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, args->dp->i_mount);
+ if (XFS_IS_CORRUPT(args->dp->i_mount,
+ rval != 0 && args->dp->i_d.di_size != args->geo->blksize))
return -EFSCORRUPTED;
- }
*vp = rval;
return 0;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 72d7ed17eef5..f6d16cd3004d 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -670,9 +670,8 @@ xfs_dir2_leafn_lookup_for_addname(
* If it has room, return it.
*/
bests = dp->d_ops->free_bests_p(free);
- if (unlikely(bests[fi] == cpu_to_be16(NULLDATAOFF))) {
- XFS_ERROR_REPORT("xfs_dir2_leafn_lookup_int",
- XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp,
+ bests[fi] == cpu_to_be16(NULLDATAOFF))) {
if (curfdb != newfdb)
xfs_trans_brelse(tp, curbp);
return -EFSCORRUPTED;
@@ -1671,7 +1670,8 @@ xfs_dir2_node_add_datablk(
if (error)
return error;
- if (dp->d_ops->db_to_fdb(args->geo, *dbno) != fbno) {
+ if (XFS_IS_CORRUPT(mp,
+ dp->d_ops->db_to_fdb(args->geo, *dbno) != fbno)) {
xfs_alert(mp,
"%s: dir ino %llu needed freesp block %lld for data block %lld, got %lld",
__func__, (unsigned long long)dp->i_ino,
@@ -1685,7 +1685,6 @@ xfs_dir2_node_add_datablk(
} else {
xfs_alert(mp, " ... fblk is NULL");
}
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 5a3e8aec2a8e..05061edc4e15 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1177,7 +1177,7 @@ xfs_refcount_finish_one(
XFS_ALLOC_FLAG_FREEING, &agbp);
if (error)
return error;
- if (!agbp)
+ if (XFS_IS_CORRUPT(tp->t_mountp, !agbp))
return -EFSCORRUPTED;
rcur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno);
@@ -1661,17 +1661,16 @@ struct xfs_refcount_recovery {
/* Stuff an extent on the recovery list. */
STATIC int
xfs_refcount_recover_extent(
- struct xfs_btree_cur *cur,
+ struct xfs_btree_cur *cur,
union xfs_btree_rec *rec,
void *priv)
{
struct list_head *debris = priv;
struct xfs_refcount_recovery *rr;
- if (be32_to_cpu(rec->refc.rc_refcount) != 1) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, cur->bc_mp);
+ if (XFS_IS_CORRUPT(cur->bc_mp,
+ be32_to_cpu(rec->refc.rc_refcount) != 1))
return -EFSCORRUPTED;
- }
rr = kmem_alloc(sizeof(struct xfs_refcount_recovery), 0);
xfs_refcount_btrec_to_irec(rec, &rr->rr_rrec);
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index b2043691325b..61d4596931ff 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -2394,7 +2394,7 @@ xfs_rmap_finish_one(
error = xfs_free_extent_fix_freelist(tp, agno, &agbp);
if (error)
return error;
- if (!agbp)
+ if (XFS_IS_CORRUPT(tp->t_mountp, !agbp))
return -EFSCORRUPTED;
rcur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno);
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index d8aaa1de921c..f42c74cb8be5 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -70,10 +70,8 @@ xfs_rtbuf_get(
if (error)
return error;
- if (nmap == 0 || !xfs_bmap_is_real_extent(&map)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, nmap == 0 || !xfs_bmap_is_real_extent(&map)))
return -EFSCORRUPTED;
- }
ASSERT(map.br_startblock != NULLFSBLOCK);
error = xfs_trans_read_buf(mp, tp, mp->m_ddev_targp,
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 64f6ceba9254..d0fac1061d6d 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -86,11 +86,9 @@ xfs_attr_shortform_list(
(XFS_ISRESET_CURSOR(cursor) &&
(dp->i_afp->if_bytes + sf->hdr.count * 16) < context->bufsize)) {
for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
- if (!xfs_attr_namecheck(sfe->nameval, sfe->namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- context->dp->i_mount);
+ if (XFS_IS_CORRUPT(context->dp->i_mount,
+ !xfs_attr_namecheck(sfe->nameval, sfe->namelen)))
return -EFSCORRUPTED;
- }
context->put_listent(context,
sfe->flags,
sfe->nameval,
@@ -179,9 +177,8 @@ xfs_attr_shortform_list(
cursor->hashval = sbp->hash;
cursor->offset = 0;
}
- if (!xfs_attr_namecheck(sbp->name, sbp->namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- context->dp->i_mount);
+ if (XFS_IS_CORRUPT(context->dp->i_mount,
+ !xfs_attr_namecheck(sbp->name, sbp->namelen))) {
error = -EFSCORRUPTED;
goto out;
}
@@ -269,10 +266,8 @@ xfs_attr_node_list_lookup(
return 0;
/* We can't point back to the root. */
- if (cursor->blkno == 0) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, cursor->blkno == 0))
return -EFSCORRUPTED;
- }
}
if (expected_level != 0)
@@ -473,11 +468,9 @@ xfs_attr3_leaf_list_int(
valuelen = be32_to_cpu(name_rmt->valuelen);
}
- if (!xfs_attr_namecheck(name, namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- context->dp->i_mount);
+ if (XFS_IS_CORRUPT(context->dp->i_mount,
+ !xfs_attr_namecheck(name, namelen)))
return -EFSCORRUPTED;
- }
context->put_listent(context, entry->flags,
name, namelen, valuelen);
if (context->seen_enough)
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index a0bec0931f3b..17f883c212f3 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -116,11 +116,9 @@ xfs_dir2_sf_getdents(
ino = dp->d_ops->sf_get_ino(sfp, sfep);
filetype = dp->d_ops->sf_get_ftype(sfep);
ctx->pos = off & 0x7fffffff;
- if (!xfs_dir2_namecheck(sfep->name, sfep->namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- dp->i_mount);
+ if (XFS_IS_CORRUPT(dp->i_mount,
+ !xfs_dir2_namecheck(sfep->name, sfep->namelen)))
return -EFSCORRUPTED;
- }
if (!dir_emit(ctx, (char *)sfep->name, sfep->namelen, ino,
xfs_dir3_get_dtype(dp->i_mount, filetype)))
return 0;
@@ -214,9 +212,8 @@ xfs_dir2_block_getdents(
/*
* If it didn't fit, set the final offset to here & return.
*/
- if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- dp->i_mount);
+ if (XFS_IS_CORRUPT(dp->i_mount,
+ !xfs_dir2_namecheck(dep->name, dep->namelen))) {
error = -EFSCORRUPTED;
goto out_rele;
}
@@ -467,9 +464,8 @@ xfs_dir2_leaf_getdents(
filetype = dp->d_ops->data_get_ftype(dep);
ctx->pos = xfs_dir2_byte_to_dataptr(curoff) & 0x7fffffff;
- if (!xfs_dir2_namecheck(dep->name, dep->namelen)) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- dp->i_mount);
+ if (XFS_IS_CORRUPT(dp->i_mount,
+ !xfs_dir2_namecheck(dep->name, dep->namelen))) {
error = -EFSCORRUPTED;
break;
}
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 1471bcd6cb70..c5fa5d83021d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -846,10 +846,8 @@ xfs_buffered_write_iomap_begin(
xfs_ilock(ip, XFS_ILOCK_EXCL);
- if (unlikely(XFS_TEST_ERROR(
- !xfs_ifork_has_extents(ip, XFS_DATA_FORK),
- mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ip, XFS_DATA_FORK)) ||
+ XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) {
error = -EFSCORRUPTED;
goto out_unlock;
}
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 4c7962ccb0c4..6e8eed4f4955 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -481,10 +481,8 @@ xfs_vn_get_link_inline(
* if_data is junk.
*/
link = ip->i_df.if_u1.if_data;
- if (!link) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, ip->i_mount);
+ if (XFS_IS_CORRUPT(ip->i_mount, !link))
return ERR_PTR(-EFSCORRUPTED);
- }
return link;
}
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 02f2147952b3..dc01bc0071a5 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -103,10 +103,9 @@ xlog_alloc_buffer(
* Pass log block 0 since we don't have an addr yet, buffer will be
* verified on read.
*/
- if (!xlog_verify_bno(log, 0, nbblks)) {
+ if (XFS_IS_CORRUPT(log->l_mp, !xlog_verify_bno(log, 0, nbblks))) {
xfs_warn(log->l_mp, "Invalid block length (0x%x) for buffer",
nbblks);
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_HIGH, log->l_mp);
return NULL;
}
@@ -152,11 +151,10 @@ xlog_do_io(
{
int error;
- if (!xlog_verify_bno(log, blk_no, nbblks)) {
+ if (XFS_IS_CORRUPT(log->l_mp, !xlog_verify_bno(log, blk_no, nbblks))) {
xfs_warn(log->l_mp,
"Invalid log block/length (0x%llx, 0x%x) for buffer",
blk_no, nbblks);
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_HIGH, log->l_mp);
return -EFSCORRUPTED;
}
@@ -244,19 +242,17 @@ xlog_header_check_recover(
* (XLOG_FMT_UNKNOWN). This stops us from trying to recover
* a dirty log created in IRIX.
*/
- if (unlikely(head->h_fmt != cpu_to_be32(XLOG_FMT))) {
+ if (XFS_IS_CORRUPT(mp, head->h_fmt != cpu_to_be32(XLOG_FMT))) {
xfs_warn(mp,
"dirty log written in incompatible format - can't recover");
xlog_header_check_dump(mp, head);
- XFS_ERROR_REPORT("xlog_header_check_recover(1)",
- XFS_ERRLEVEL_HIGH, mp);
return -EFSCORRUPTED;
- } else if (unlikely(!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
+ }
+ if (XFS_IS_CORRUPT(mp,
+ !uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
xfs_warn(mp,
"dirty log entry has mismatched uuid - can't recover");
xlog_header_check_dump(mp, head);
- XFS_ERROR_REPORT("xlog_header_check_recover(2)",
- XFS_ERRLEVEL_HIGH, mp);
return -EFSCORRUPTED;
}
return 0;
@@ -279,11 +275,10 @@ xlog_header_check_mount(
* by IRIX and continue.
*/
xfs_warn(mp, "null uuid in log - IRIX style log");
- } else if (unlikely(!uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
+ } else if (XFS_IS_CORRUPT(mp,
+ !uuid_equal(&mp->m_sb.sb_uuid, &head->h_fs_uuid))) {
xfs_warn(mp, "log has mismatched uuid - can't recover");
xlog_header_check_dump(mp, head);
- XFS_ERROR_REPORT("xlog_header_check_mount",
- XFS_ERRLEVEL_HIGH, mp);
return -EFSCORRUPTED;
}
return 0;
@@ -1699,11 +1694,9 @@ xlog_clear_stale_blocks(
* the distance from the beginning of the log to the
* tail.
*/
- if (unlikely(head_block < tail_block || head_block >= log->l_logBBsize)) {
- XFS_ERROR_REPORT("xlog_clear_stale_blocks(1)",
- XFS_ERRLEVEL_LOW, log->l_mp);
+ if (XFS_IS_CORRUPT(log->l_mp,
+ head_block < tail_block || head_block >= log->l_logBBsize))
return -EFSCORRUPTED;
- }
tail_distance = tail_block + (log->l_logBBsize - head_block);
} else {
/*
@@ -1711,11 +1704,9 @@ xlog_clear_stale_blocks(
* so the distance from the head to the tail is just
* the tail block minus the head block.
*/
- if (unlikely(head_block >= tail_block || head_cycle != (tail_cycle + 1))){
- XFS_ERROR_REPORT("xlog_clear_stale_blocks(2)",
- XFS_ERRLEVEL_LOW, log->l_mp);
+ if (XFS_IS_CORRUPT(log->l_mp,
+ head_block >= tail_block || head_cycle != tail_cycle + 1))
return -EFSCORRUPTED;
- }
tail_distance = tail_block - head_block;
}
@@ -2135,13 +2126,11 @@ xlog_recover_do_inode_buffer(
*/
logged_nextp = item->ri_buf[item_index].i_addr +
next_unlinked_offset - reg_buf_offset;
- if (unlikely(*logged_nextp == 0)) {
+ if (XFS_IS_CORRUPT(mp, *logged_nextp == 0)) {
xfs_alert(mp,
"Bad inode buffer log record (ptr = "PTR_FMT", bp = "PTR_FMT"). "
"Trying to replay bad (0) inode di_next_unlinked field.",
item, bp);
- XFS_ERROR_REPORT("xlog_recover_do_inode_buf",
- XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
@@ -2969,22 +2958,18 @@ xlog_recover_inode_pass2(
* Make sure the place we're flushing out to really looks
* like an inode!
*/
- if (unlikely(!xfs_verify_magic16(bp, dip->di_magic))) {
+ if (XFS_IS_CORRUPT(mp, !xfs_verify_magic16(bp, dip->di_magic))) {
xfs_alert(mp,
"%s: Bad inode magic number, dip = "PTR_FMT", dino bp = "PTR_FMT", ino = %Ld",
__func__, dip, bp, in_f->ilf_ino);
- XFS_ERROR_REPORT("xlog_recover_inode_pass2(1)",
- XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
goto out_release;
}
ldip = item->ri_buf[1].i_addr;
- if (unlikely(ldip->di_magic != XFS_DINODE_MAGIC)) {
+ if (XFS_IS_CORRUPT(mp, ldip->di_magic != XFS_DINODE_MAGIC)) {
xfs_alert(mp,
"%s: Bad inode log record, rec ptr "PTR_FMT", ino %Ld",
__func__, item, in_f->ilf_ino);
- XFS_ERROR_REPORT("xlog_recover_inode_pass2(2)",
- XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
goto out_release;
}
@@ -5209,12 +5194,10 @@ xlog_valid_rec_header(
{
int hlen;
- if (unlikely(rhead->h_magicno != cpu_to_be32(XLOG_HEADER_MAGIC_NUM))) {
- XFS_ERROR_REPORT("xlog_valid_rec_header(1)",
- XFS_ERRLEVEL_LOW, log->l_mp);
+ if (XFS_IS_CORRUPT(log->l_mp,
+ rhead->h_magicno != cpu_to_be32(XLOG_HEADER_MAGIC_NUM)))
return -EFSCORRUPTED;
- }
- if (unlikely(
+ if (XFS_IS_CORRUPT(log->l_mp,
(!rhead->h_version ||
(be32_to_cpu(rhead->h_version) & (~XLOG_VERSION_OKBITS))))) {
xfs_warn(log->l_mp, "%s: unrecognised log version (%d).",
@@ -5224,16 +5207,11 @@ xlog_valid_rec_header(
/* LR body must have data or it wouldn't have been written */
hlen = be32_to_cpu(rhead->h_len);
- if (unlikely( hlen <= 0 || hlen > INT_MAX )) {
- XFS_ERROR_REPORT("xlog_valid_rec_header(2)",
- XFS_ERRLEVEL_LOW, log->l_mp);
+ if (XFS_IS_CORRUPT(log->l_mp, hlen <= 0 || hlen > INT_MAX))
return -EFSCORRUPTED;
- }
- if (unlikely( blkno > log->l_logBBsize || blkno > INT_MAX )) {
- XFS_ERROR_REPORT("xlog_valid_rec_header(3)",
- XFS_ERRLEVEL_LOW, log->l_mp);
+ if (XFS_IS_CORRUPT(log->l_mp,
+ blkno > log->l_logBBsize || blkno > INT_MAX))
return -EFSCORRUPTED;
- }
return 0;
}
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 5ea95247a37f..fca65109cf24 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -761,9 +761,8 @@ xfs_mountfs(
goto out_free_dir;
}
- if (!sbp->sb_logblocks) {
+ if (XFS_IS_CORRUPT(mp, !sbp->sb_logblocks)) {
xfs_warn(mp, "no log defined");
- XFS_ERROR_REPORT("xfs_mountfs", XFS_ERRLEVEL_LOW, mp);
error = -EFSCORRUPTED;
goto out_free_perag;
}
@@ -801,12 +800,10 @@ xfs_mountfs(
ASSERT(rip != NULL);
- if (unlikely(!S_ISDIR(VFS_I(rip)->i_mode))) {
+ if (XFS_IS_CORRUPT(mp, !S_ISDIR(VFS_I(rip)->i_mode))) {
xfs_warn(mp, "corrupted root inode %llu: not a directory",
(unsigned long long)rip->i_ino);
xfs_iunlock(rip, XFS_ILOCK_EXCL);
- XFS_ERROR_REPORT("xfs_mountfs_int(2)", XFS_ERRLEVEL_LOW,
- mp);
error = -EFSCORRUPTED;
goto out_rele_rip;
}
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 66ea8e4fca86..3d8a08d02649 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -755,19 +755,15 @@ xfs_qm_qino_alloc(
if ((flags & XFS_QMOPT_PQUOTA) &&
(mp->m_sb.sb_gquotino != NULLFSINO)) {
ino = mp->m_sb.sb_gquotino;
- if (mp->m_sb.sb_pquotino != NULLFSINO) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- mp);
+ if (XFS_IS_CORRUPT(mp,
+ mp->m_sb.sb_pquotino != NULLFSINO))
return -EFSCORRUPTED;
- }
} else if ((flags & XFS_QMOPT_GQUOTA) &&
(mp->m_sb.sb_pquotino != NULLFSINO)) {
ino = mp->m_sb.sb_pquotino;
- if (mp->m_sb.sb_gquotino != NULLFSINO) {
- XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
- mp);
+ if (XFS_IS_CORRUPT(mp,
+ mp->m_sb.sb_gquotino != NULLFSINO))
return -EFSCORRUPTED;
- }
}
if (ino != NULLFSINO) {
error = xfs_iget(mp, NULL, ino, 0, 0, ip);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate
2019-11-08 7:05 ` [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate Darrick J. Wong
@ 2019-11-08 7:15 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2019-11-08 7:15 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Nov 07, 2019 at 11:05:15PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Replace the open-coded checks for whether or not an inode fork maps
> blocks with a macro that will implant the code for us. This helps us
> declutter the bmap code a bit.
>
> Note that I had to use a macro instead of a static inline function
> because of C header dependency problems between xfs_inode.h and
> xfs_inode_fork.h.
>
> Conversion was performed with the following Coccinelle script:
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-08 7:05 ` [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT Darrick J. Wong
@ 2019-11-08 7:21 ` Christoph Hellwig
2019-11-09 22:32 ` Dave Chinner
1 sibling, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2019-11-08 7:21 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert the last of the open coded corruption check and report idioms to
> use the XFS_IS_CORRUPT macro.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-08 7:05 ` [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT Darrick J. Wong
2019-11-08 7:21 ` Christoph Hellwig
@ 2019-11-09 22:32 ` Dave Chinner
2019-11-10 0:18 ` Darrick J. Wong
1 sibling, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2019-11-09 22:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Convert the last of the open coded corruption check and report idioms to
> use the XFS_IS_CORRUPT macro.
hmmm.
> + if (XFS_IS_CORRUPT(mp,
> + ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
This pattern is weird. It looks like there are two separate logic
statements to the if() condition, when in fact the second line is
part of the XFS_IS_CORRUPT() macro.
It just looks wrong to me, especially when everything other
multi-line macro is indented based on the indenting of the macro
parameters....
Yes, in this case it looks a bit strange, too:
if (XFS_IS_CORRUPT(mp,
ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
but there is no mistaking it for separate logic statements.
I kinda value being able to glance at the indent levels to see
separate logic elements....
> - if (unlikely(
> - be32_to_cpu(sib_info->back) != last_blkno ||
> - sib_info->magic != dead_info->magic)) {
> - XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
> - XFS_ERRLEVEL_LOW, mp);
> + if (XFS_IS_CORRUPT(mp,
> + be32_to_cpu(sib_info->back) != last_blkno ||
> + sib_info->magic != dead_info->magic)) {
> error = -EFSCORRUPTED;
> goto done;
> }
This is kind of what I mean - is it two or three logic statments
here? No, it's actually one, but it has two nested checks...
There's a few other list this that are somewhat non-obvious as to
the logic...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-09 22:32 ` Dave Chinner
@ 2019-11-10 0:18 ` Darrick J. Wong
2019-11-10 2:49 ` Dave Chinner
0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2019-11-10 0:18 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, hch
On Sun, Nov 10, 2019 at 09:32:38AM +1100, Dave Chinner wrote:
> On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Convert the last of the open coded corruption check and report idioms to
> > use the XFS_IS_CORRUPT macro.
>
> hmmm.
>
> > + if (XFS_IS_CORRUPT(mp,
> > + ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
>
> This pattern is weird. It looks like there are two separate logic
> statements to the if() condition, when in fact the second line is
> part of the XFS_IS_CORRUPT() macro.
>
> It just looks wrong to me, especially when everything other
> multi-line macro is indented based on the indenting of the macro
> parameters....
>
> Yes, in this case it looks a bit strange, too:
>
> if (XFS_IS_CORRUPT(mp,
> ir.loaded != XFS_IFORK_NEXTENTS(ip, whichfork))) {
>
> but there is no mistaking it for separate logic statements.
They're all ugly, because of all the stupid identing when the
conditional gets too long.
> I kinda value being able to glance at the indent levels to see
> separate logic elements....
>
> > - if (unlikely(
> > - be32_to_cpu(sib_info->back) != last_blkno ||
> > - sib_info->magic != dead_info->magic)) {
> > - XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
> > - XFS_ERRLEVEL_LOW, mp);
> > + if (XFS_IS_CORRUPT(mp,
> > + be32_to_cpu(sib_info->back) != last_blkno ||
> > + sib_info->magic != dead_info->magic)) {
They're both ugly, IMHO. One has horrible indentation that's too close
to the code in the if statement body, the other is hard to read as an if
statement.
> > error = -EFSCORRUPTED;
> > goto done;
> > }
>
> This is kind of what I mean - is it two or three logic statments
> here? No, it's actually one, but it has two nested checks...
>
> There's a few other list this that are somewhat non-obvious as to
> the logic...
I'd thought about giving it the shortest name possible, not bothering to
log the fsname that goes with the error report, and making the if part
of the macro:
#define IFBAD(cond) if ((unlikely(cond) ? assert(...), true : false))
IFBAD(be32_to_cpu(sib_info->back) != last_blkno ||
sib_info->magic != dead_info->magic)) {
xfs_whatever();
return -EFSCORRUPTED;
}
Is that better?
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-10 0:18 ` Darrick J. Wong
@ 2019-11-10 2:49 ` Dave Chinner
2019-11-10 18:20 ` Darrick J. Wong
0 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2019-11-10 2:49 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Sat, Nov 09, 2019 at 04:18:03PM -0800, Darrick J. Wong wrote:
> On Sun, Nov 10, 2019 at 09:32:38AM +1100, Dave Chinner wrote:
> > On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > - if (unlikely(
> > > - be32_to_cpu(sib_info->back) != last_blkno ||
> > > - sib_info->magic != dead_info->magic)) {
> > > - XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
> > > - XFS_ERRLEVEL_LOW, mp);
> > > + if (XFS_IS_CORRUPT(mp,
> > > + be32_to_cpu(sib_info->back) != last_blkno ||
> > > + sib_info->magic != dead_info->magic)) {
>
> They're both ugly, IMHO. One has horrible indentation that's too close
> to the code in the if statement body, the other is hard to read as an if
> statement.
I was more commenting on the new code. The old code is horrible,
yes, but I don't think the new code is much better. :(
> > > error = -EFSCORRUPTED;
> > > goto done;
> > > }
> >
> > This is kind of what I mean - is it two or three logic statments
> > here? No, it's actually one, but it has two nested checks...
> >
> > There's a few other list this that are somewhat non-obvious as to
> > the logic...
>
> I'd thought about giving it the shortest name possible, not bothering to
> log the fsname that goes with the error report, and making the if part
> of the macro:
>
> #define IFBAD(cond) if ((unlikely(cond) ? assert(...), true : false))
>
> IFBAD(be32_to_cpu(sib_info->back) != last_blkno ||
> sib_info->magic != dead_info->magic)) {
> xfs_whatever();
> return -EFSCORRUPTED;
> }
>
> Is that better?
Look at what quoting did to it - it'll look the same as above in
patches, unfortunately, so I don't think "short as possible" works
any better.
Perhaps s/IFBAD/XFS_CORRUPT_IF/ ?
XFS_CORRUPT_IF(be32_to_cpu(sib_info->back) != last_blkno ||
sib_info->magic != dead_info->magic)) {
xfs_error(mp, "user readable error message");
return -EFSCORRUPTED;
}
That solves the patch/quote indent problem, documents the code well,
and only sacrifices a single tab for the condition statements...
/me gets back on his bike and leaves the shed coated in wet paint.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT
2019-11-10 2:49 ` Dave Chinner
@ 2019-11-10 18:20 ` Darrick J. Wong
0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2019-11-10 18:20 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs, hch
On Sun, Nov 10, 2019 at 01:49:19PM +1100, Dave Chinner wrote:
> On Sat, Nov 09, 2019 at 04:18:03PM -0800, Darrick J. Wong wrote:
> > On Sun, Nov 10, 2019 at 09:32:38AM +1100, Dave Chinner wrote:
> > > On Thu, Nov 07, 2019 at 11:05:21PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > - if (unlikely(
> > > > - be32_to_cpu(sib_info->back) != last_blkno ||
> > > > - sib_info->magic != dead_info->magic)) {
> > > > - XFS_ERROR_REPORT("xfs_da_swap_lastblock(3)",
> > > > - XFS_ERRLEVEL_LOW, mp);
> > > > + if (XFS_IS_CORRUPT(mp,
> > > > + be32_to_cpu(sib_info->back) != last_blkno ||
> > > > + sib_info->magic != dead_info->magic)) {
> >
> > They're both ugly, IMHO. One has horrible indentation that's too close
> > to the code in the if statement body, the other is hard to read as an if
> > statement.
>
> I was more commenting on the new code. The old code is horrible,
> yes, but I don't think the new code is much better. :(
>
> > > > error = -EFSCORRUPTED;
> > > > goto done;
> > > > }
> > >
> > > This is kind of what I mean - is it two or three logic statments
> > > here? No, it's actually one, but it has two nested checks...
> > >
> > > There's a few other list this that are somewhat non-obvious as to
> > > the logic...
> >
> > I'd thought about giving it the shortest name possible, not bothering to
> > log the fsname that goes with the error report, and making the if part
> > of the macro:
> >
> > #define IFBAD(cond) if ((unlikely(cond) ? assert(...), true : false))
> >
> > IFBAD(be32_to_cpu(sib_info->back) != last_blkno ||
> > sib_info->magic != dead_info->magic)) {
> > xfs_whatever();
> > return -EFSCORRUPTED;
> > }
> >
> > Is that better?
>
> Look at what quoting did to it - it'll look the same as above in
> patches, unfortunately, so I don't think "short as possible" works
> any better.
>
> Perhaps s/IFBAD/XFS_CORRUPT_IF/ ?
>
> XFS_CORRUPT_IF(be32_to_cpu(sib_info->back) != last_blkno ||
> sib_info->magic != dead_info->magic)) {
> xfs_error(mp, "user readable error message");
> return -EFSCORRUPTED;
> }
>
> That solves the patch/quote indent problem, documents the code well,
> and only sacrifices a single tab for the condition statements...
...but that's one character short of two full tabs, and I dislike typing
<tab><space><space><space><space><space><space><space> and having the
alignment be off by a single column.
> /me gets back on his bike and leaves the shed coated in wet paint.
/me takes out his paint sprayer and drowns everything in paint.
XCORRUPT_WHEN(
IF_XFS_CORRUPT(
XFS_CORRUPT_LOG(
LOG_XFS_CORRUPT(
IF_XFSCORRUPTED(
XFS_CORRUPT_IFF(
if_meta_corrupt(
if_xfs_meta_bad(moo,
whatever) {
grumble();
}
Ok, I'll change the whole thing to if_xfs_meta_bad(test) and hopes this
is the end of bikeshedding because changing this series is a pita.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-10 18:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 7:05 [PATCH v3 0/2] xfs: refactor corruption checking and reporting Darrick J. Wong
2019-11-08 7:05 ` [PATCH 1/2] xfs: refactor "does this fork map blocks" predicate Darrick J. Wong
2019-11-08 7:15 ` Christoph Hellwig
2019-11-08 7:05 ` [PATCH 2/2] xfs: convert open coded corruption check to use XFS_IS_CORRUPT Darrick J. Wong
2019-11-08 7:21 ` Christoph Hellwig
2019-11-09 22:32 ` Dave Chinner
2019-11-10 0:18 ` Darrick J. Wong
2019-11-10 2:49 ` Dave Chinner
2019-11-10 18:20 ` 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).