Hi,
Recently, our use-case ran into 2 bugs in case doing xfs_remove when the
disk space is in-pressure, which may cause xfs shutdown and kernel crash
in the xfs log recovery procedure. Here are 2 patches to fix the
problem, and a patch adding a helper to optimize the code structure.
The 1st patch fixes an uninitialized variable issue.
The 2nd patch ensures the blkno in the xfs_buf is updated when doing
xfs_da3_swap_lastblock().
The 3rd patch adds a xfs_buf copy helper to optimize the code structure.
Changes of v2:
- directly set the *logflagsp value to make the code more robust in the
1st commit,
- check xfs's crc-feature rather than magic in the 2nd commit, and
- fixed code style and rebased onto the master branch.
Changes of v3:
- fix code style, and
- add a new patch which does xfs_buf memcpy in a helper.
Changes of v4:
- optimize comments.
Thanks,
Jiachen
Jiachen Zhang (1):
xfs: ensure logflagsp is initialized in xfs_bmap_del_extent_real
Zhang Tianci (2):
xfs: update dir3 leaf block metadata after swap
xfs: extract xfs_da_buf_copy() helper function
fs/xfs/libxfs/xfs_attr_leaf.c | 12 ++----
fs/xfs/libxfs/xfs_bmap.c | 73 +++++++++++++++--------------------
fs/xfs/libxfs/xfs_da_btree.c | 69 +++++++++++++++------------------
fs/xfs/libxfs/xfs_da_btree.h | 2 +
4 files changed, 68 insertions(+), 88 deletions(-)
--
2.20.1
From: Zhang Tianci <[email protected]>
xfs_da3_swap_lastblock() copy the last block content to the dead block,
but do not update the metadata in it. We need update some metadata
for some kinds of type block, such as dir3 leafn block records its
blkno, we shall update it to the dead block blkno. Otherwise,
before write the xfs_buf to disk, the verify_write() will fail in
blk_hdr->blkno != xfs_buf->b_bn, then xfs will be shutdown.
We will get this warning:
XFS (dm-0): Metadata corruption detected at xfs_dir3_leaf_verify+0xa8/0xe0 [xfs], xfs_dir3_leafn block 0x178
XFS (dm-0): Unmount and run xfs_repair
XFS (dm-0): First 128 bytes of corrupted metadata buffer:
00000000e80f1917: 00 80 00 0b 00 80 00 07 3d ff 00 00 00 00 00 00 ........=.......
000000009604c005: 00 00 00 00 00 00 01 a0 00 00 00 00 00 00 00 00 ................
000000006b6fb2bf: e4 44 e3 97 b5 64 44 41 8b 84 60 0e 50 43 d9 bf .D...dDA..`.PC..
00000000678978a2: 00 00 00 00 00 00 00 83 01 73 00 93 00 00 00 00 .........s......
00000000b28b247c: 99 29 1d 38 00 00 00 00 99 29 1d 40 00 00 00 00 .).8.....).@....
000000002b2a662c: 99 29 1d 48 00 00 00 00 99 49 11 00 00 00 00 00 .).H.....I......
00000000ea2ffbb8: 99 49 11 08 00 00 45 25 99 49 11 10 00 00 48 fe .I....E%.I....H.
0000000069e86440: 99 49 11 18 00 00 4c 6b 99 49 11 20 00 00 4d 97 .I....Lk.I. ..M.
XFS (dm-0): xfs_do_force_shutdown(0x8) called from line 1423 of file fs/xfs/xfs_buf.c. Return address = 00000000c0ff63c1
XFS (dm-0): Corruption of in-memory data detected. Shutting down filesystem
XFS (dm-0): Please umount the filesystem and rectify the problem(s)
From the log above, we know xfs_buf->b_no is 0x178, but the block's hdr record
its blkno is 0x1a0.
Fixes: 24df33b45ecf ("xfs: add CRC checking to dir2 leaf blocks")
Signed-off-by: Zhang Tianci <[email protected]>
Suggested-by: Dave Chinner <[email protected]>
---
fs/xfs/libxfs/xfs_da_btree.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index e576560b46e9..282c7cf032f4 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2316,10 +2316,17 @@ xfs_da3_swap_lastblock(
return error;
/*
* Copy the last block into the dead buffer and log it.
+ * On CRC-enabled file systems, also update the stamped in blkno.
*/
memcpy(dead_buf->b_addr, last_buf->b_addr, args->geo->blksize);
+ if (xfs_has_crc(mp)) {
+ struct xfs_da3_blkinfo *da3 = dead_buf->b_addr;
+
+ da3->blkno = cpu_to_be64(xfs_buf_daddr(dead_buf));
+ }
xfs_trans_log_buf(tp, dead_buf, 0, args->geo->blksize - 1);
dead_info = dead_buf->b_addr;
+
/*
* Get values from the moved block.
*/
--
2.20.1
On Tue, Dec 05, 2023 at 01:58:59PM +0800, Jiachen Zhang wrote:
> From: Zhang Tianci <[email protected]>
>
> xfs_da3_swap_lastblock() copy the last block content to the dead block,
> but do not update the metadata in it. We need update some metadata
> for some kinds of type block, such as dir3 leafn block records its
> blkno, we shall update it to the dead block blkno. Otherwise,
> before write the xfs_buf to disk, the verify_write() will fail in
> blk_hdr->blkno != xfs_buf->b_bn, then xfs will be shutdown.
>
> We will get this warning:
>
> XFS (dm-0): Metadata corruption detected at xfs_dir3_leaf_verify+0xa8/0xe0 [xfs], xfs_dir3_leafn block 0x178
> XFS (dm-0): Unmount and run xfs_repair
> XFS (dm-0): First 128 bytes of corrupted metadata buffer:
> 00000000e80f1917: 00 80 00 0b 00 80 00 07 3d ff 00 00 00 00 00 00 ........=.......
> 000000009604c005: 00 00 00 00 00 00 01 a0 00 00 00 00 00 00 00 00 ................
> 000000006b6fb2bf: e4 44 e3 97 b5 64 44 41 8b 84 60 0e 50 43 d9 bf .D...dDA..`.PC..
> 00000000678978a2: 00 00 00 00 00 00 00 83 01 73 00 93 00 00 00 00 .........s......
> 00000000b28b247c: 99 29 1d 38 00 00 00 00 99 29 1d 40 00 00 00 00 .).8.....).@....
> 000000002b2a662c: 99 29 1d 48 00 00 00 00 99 49 11 00 00 00 00 00 .).H.....I......
> 00000000ea2ffbb8: 99 49 11 08 00 00 45 25 99 49 11 10 00 00 48 fe .I....E%.I....H.
> 0000000069e86440: 99 49 11 18 00 00 4c 6b 99 49 11 20 00 00 4d 97 .I....Lk.I. ..M.
> XFS (dm-0): xfs_do_force_shutdown(0x8) called from line 1423 of file fs/xfs/xfs_buf.c. Return address = 00000000c0ff63c1
> XFS (dm-0): Corruption of in-memory data detected. Shutting down filesystem
> XFS (dm-0): Please umount the filesystem and rectify the problem(s)
>
> From the log above, we know xfs_buf->b_no is 0x178, but the block's hdr record
> its blkno is 0x1a0.
>
> Fixes: 24df33b45ecf ("xfs: add CRC checking to dir2 leaf blocks")
> Signed-off-by: Zhang Tianci <[email protected]>
> Suggested-by: Dave Chinner <[email protected]>
still looks fine to me
Reviewed-by: Darrick J. Wong <[email protected]>
--D
> ---
> fs/xfs/libxfs/xfs_da_btree.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index e576560b46e9..282c7cf032f4 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2316,10 +2316,17 @@ xfs_da3_swap_lastblock(
> return error;
> /*
> * Copy the last block into the dead buffer and log it.
> + * On CRC-enabled file systems, also update the stamped in blkno.
> */
> memcpy(dead_buf->b_addr, last_buf->b_addr, args->geo->blksize);
> + if (xfs_has_crc(mp)) {
> + struct xfs_da3_blkinfo *da3 = dead_buf->b_addr;
> +
> + da3->blkno = cpu_to_be64(xfs_buf_daddr(dead_buf));
> + }
> xfs_trans_log_buf(tp, dead_buf, 0, args->geo->blksize - 1);
> dead_info = dead_buf->b_addr;
> +
> /*
> * Get values from the moved block.
> */
> --
> 2.20.1
>
>