On 2022/11/8 0:58, Darrick J. Wong wrote:
On Thu, Nov 03, 2022 at 07:37:09PM +0800, Guo Xuenan wrote:
xfs_btree_check_block contains a tag XFS_ERRTAG_BTREE_CHECK_{L,S}BLOCK,
it is a fault injection tag, better not use it in the macro ASSERT.
Since with XFS_DEBUG setting up, we can always trigger assert by `echo 1
/sys/fs/xfs/${disk}/errortag/btree_chk_{s,l}blk`.
It's confusing and strange.
Please be more specific about how this is confusing or strange.
I meant in current code, the ASSERT will alway happen,when we
`echo 1 > /sys/fs/xfs/${disk}/errortag/btree_chk_{s,l}blk`.
xfs_btree_islastblock
->ASSERT(block && xfs_btree_check_block(cur, block, level, bp) == 0);
->xfs_btree_check_{l/s}block
->XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BTREE_CHECK_{S,L}BLOCK)
we can use error injection to trigger this ASSERT.
I think ASERRT macro and error injection are to find some effective
problems,
not to create some kernel panic. So, putting the error injection
function in
ASSERT is a little strange.
Instead of using it in ASSERT, replace it with
xfs_warn.
Fixes: 27d9ee577dcc ("xfs: actually check xfs_btree_check_block return in xfs_btree_islastblock")
Signed-off-by: Guo Xuenan <guoxuenan@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_btree.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index eef27858a013..637513087c18 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -556,8 +556,11 @@ xfs_btree_islastblock(
struct xfs_buf *bp;
block = xfs_btree_get_block(cur, level, &bp);
- ASSERT(block && xfs_btree_check_block(cur, block, level, bp) == 0);
-
+ ASSERT(block);
+#if defined(DEBUG) || defined(XFS_WARN)
+ if (xfs_btree_check_block(cur, block, level, bp))
+ xfs_warn(cur->bc_mp, "%s: xfs_btree_check_block() error.", __func__);
+#endif
...because this seems like open-coding ASSERT, possibly without the
panic on errors part.
yes，exactly！I also think it can be deleted, but i have no idea if this
is necessary,
I just retain it in this fix patch; looking forward to your decision :)
--D
if (cur->bc_flags & XFS_BTREE_LONG_PTRS)
return block->bb_u.l.bb_rightsib == cpu_to_be64(NULLFSBLOCK);
return block->bb_u.s.bb_rightsib == cpu_to_be32(NULLAGBLOCK);
--
2.31.1
.
--
Guo Xuenan [OS Kernel Lab]
-----------------------------
Email: guoxuenan@xxxxxxxxxx
