Message-ID:
 <SEZPR06MB5269FA31FE21CD9799DA17ABE89AA@SEZPR06MB5269.apcprd06.prod.outlook.com>
Date: Sun, 25 May 2025 15:03:07 +0000
From: 李扬韬 <frank.li@...o.com>
To: Viacheslav Dubeyko <slava@...eyko.com>, "glaubitz@...sik.fu-berlin.de"
	<glaubitz@...sik.fu-berlin.de>, Andrew Morton <akpm@...ux-foundation.org>,
	Ernesto A. Fernández <ernesto.mnd.fernandez@...il.com>
CC: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"syzbot+8c0bc9f818702ff75b76@...kaller.appspotmail.com"
	<syzbot+8c0bc9f818702ff75b76@...kaller.appspotmail.com>
Subject:
 回复: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents

Hi Slava,

> Which particular xfstests' test-case(s) triggers the issue? Do we have the easy reproducing path of it? How can I check the fix, finally?

generic/013 triggers the issue. Here is the reproducing path.

[Origin]

We got fsck error, reason is the same as [1].

[1] https://lore.kernel.org/all/20250430001211.1912533-1-slava@dubeyko.com/

root@...ntu:/home/ubuntu/xfstests-dev# ./check generic/013
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 ubuntu 6.15.0-rc4-00055-g71bfd66b8583-dirty #421 SMP PREEMPT_DYNAMIC Fri May 23 18:30:10 CST 2025
MKFS_OPTIONS  -- /dev/nvme1n1
MOUNT_OPTIONS -- /dev/nvme1n1 /mnt/scratch

generic/013 35s ... [  380.286618] hfsplus: xattr exists yet
[  382.410297] hfsplus: xattr exists yet
[  383.872844] hfsplus: cannot replace xattr
[  385.802529] hfsplus: cannot replace xattr
[  393.125897] hfsplus: xattr exists yet
[  396.222921] hfsplus: cannot replace xattr
[  399.084012] hfsplus: cannot replace xattr
[  403.233816] hfsplus: cannot replace xattr
_check_generic_filesystem: filesystem on /dev/nvme0n1 is inconsistent
(see /home/ubuntu/xfstests-dev/results//generic/013.full for details)
_check_dmesg: something found in dmesg (see /home/ubuntu/xfstests-dev/results//generic/013.dmesg)

Ran: generic/013
Failures: generic/013
Failed 1 of 1 tests

[w/ bnode patch]

The fsck error is related to the node not being cleared, which may be related to the implementation of the fsck tool. 
We can continue to discuss this in the previous email. For this, we can ignore it and continue the analysis based on the bnode patch.

diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 079ea80534f7..f2424acd3636 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -633,7 +633,7 @@ void hfs_bnode_put(struct hfs_bnode *node)
                if (test_bit(HFS_BNODE_DELETED, &node->flags)) {
                        hfs_bnode_unhash(node);
                        spin_unlock(&tree->hash_lock);
-                       if (hfs_bnode_need_zeroout(tree))
+               //      if (hfs_bnode_need_zeroout(tree))
                                hfs_bnode_clear(node, 0, tree->node_size);
                        hfs_bmap_free(node);
                        hfs_bnode_free(node);

After apply bnode patch. We got error from dmesg, which warn at fs/hfsplus/extents.c:346.

root@...ntu:/home/ubuntu/xfstests-dev# ./check generic/013
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 ubuntu 6.15.0-rc4-00055-g71bfd66b8583-dirty #422 SMP PREEMPT_DYNAMIC Sun May 25 22:37:55 CST 2025
MKFS_OPTIONS  -- /dev/nvme1n1
MOUNT_OPTIONS -- /dev/nvme1n1 /mnt/scratch

generic/013 35s ... [  236.356697] hfsplus: xattr exists yet
[  238.288269] hfsplus: xattr exists yet
[  240.673488] hfsplus: cannot replace xattr
[  242.133163] hfsplus: xattr exists yet
[  242.172538] hfsplus: xattr exists yet
[  243.702797] hfsplus: xattr exists yet
[  245.943067] hfsplus: xattr exists yet
[  249.502186] hfsplus: cannot replace xattr
[  252.544517] hfsplus: xattr exists yet
[  253.538462] hfsplus: cannot replace xattr
[  263.456784] hfsplus: cannot replace xattr
_check_dmesg: something found in dmesg (see /home/ubuntu/xfstests-dev/results//generic/013.dmesg)

Ran: generic/013
Failures: generic/013
Failed 1 of 1 tests

# demsg
[  225.975852] run fstests generic/013 at 2025-05-25 14:42:11
[  231.718234] ------------[ cut here ]------------
[  231.718677] WARNING: CPU: 3 PID: 1091 at fs/hfsplus/extents.c:346 hfsplus_free_extents+0xfc/0x110
[  231.719117] Modules linked in:
[  231.719895] CPU: 3 UID: 0 PID: 1091 Comm: fsstress Not tainted 6.15.0-rc4-00055-g71bfd66b8583-dirty #422 PREEMPT(voluntary)
[  231.719996] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[  231.720170] RIP: 0010:hfsplus_free_extents+0xfc/0x110
[  231.720383] Code: 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 48 c7 c7 4d 62 46 b2 e8 b5 58 cf ff eb 95 48 c7 c7 4d 62 46 b2 e8 a7 58 cf ff eb cd 90 <0f> 0b 90 e9 30 ff ff ff 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90
[  231.720492] RSP: 0018:ffffaaa9813bbd28 EFLAGS: 00000202
[  231.720563] RAX: ffff995582666701 RBX: 00000000000000ed RCX: 00000000000001b5
[  231.720592] RDX: 00000000000000ed RSI: ffff9955818e8ad8 RDI: ffff995588c80048
[  231.720617] RBP: ffff9955818e8ad8 R08: 0000000000000002 R09: ffffaaa9813bbcda
[  231.720641] R10: ffff995585cfdb90 R11: 0000000000000002 R12: 0000000000000000
[  231.720672] R13: 00000000000001b5 R14: 00000000000000c8 R15: ffff995587f40800
[  231.720778] FS:  00007f81e3bd8740(0000) GS:ffff99564ac46000(0000) knlGS:0000000000000000
[  231.720813] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  231.720838] CR2: 00005634430a6108 CR3: 00000000147d5000 CR4: 00000000000006f0
[  231.720960] Call Trace:
[  231.721831]  <TASK>
[  231.722111]  hfsplus_file_truncate+0x2b6/0x3e0
[  231.722222]  hfsplus_delete_inode+0x54/0x70
[  231.722325]  hfsplus_unlink+0x17f/0x1c0
[  231.722384]  ? security_inode_permission+0x23/0x40
[  231.722415]  vfs_unlink+0x110/0x2b0
[  231.722442]  do_unlinkat+0x251/0x2c0
[  231.722471]  __x64_sys_unlink+0x1c/0x30
[  231.722491]  do_syscall_64+0x9e/0x190
[  231.722551]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  231.722726] RIP: 0033:0x7f81e3cf740b
[  231.723023] Code: 30 ff ff ff e9 74 fd ff ff e8 a1 ba 01 00 90 f3 0f 1e fa b8 5f 00 00 00 0f 05 c3 0f 1f 40 00 f3 0f 1e fa b8 57 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 d9 69 0e 00 f7 d8
[  231.723047] RSP: 002b:00007ffe97ed66b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000057
[  231.723089] RAX: ffffffffffffffda RBX: 0000000000000035 RCX: 00007f81e3cf740b
[  231.723104] RDX: 0000000000000000 RSI: 00007ffe97ed6690 RDI: 00005634430875c0
[  231.723116] RBP: 00007ffe97ed6830 R08: 000000000000ffff R09: 0000000000000000
[  231.723128] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe97ed66d0
[  231.723141] R13: 8f5c28f5c28f5c29 R14: 00007ffe97ed68a0 R15: 000056341fe3c7c0
[  231.723219]  </TASK>
[  231.723427] ---[ end trace 0000000000000000 ]---
[  233.296305] ------------[ cut here ]------------

[w/ bnode &this patch]

Test pass without any error.

root@...ntu:/home/ubuntu/xfstests-dev# ./check generic/013
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 ubuntu 6.15.0-rc4-00055-g71bfd66b8583-dirty #423 SMP PREEMPT_DYNAMIC Sun May 25 22:54:51 CST 2025
MKFS_OPTIONS  -- /dev/nvme1n1
MOUNT_OPTIONS -- /dev/nvme1n1 /mnt/scratch

generic/013 35s ... [  106.018643] hfsplus: xattr exists yet
[  110.155138] hfsplus: cannot replace xattr
[  112.061738] hfsplus: cannot replace xattr
[  113.215120] hfsplus: cannot replace xattr
[  118.308974] hfsplus: xattr exists yet
[  133.279630] hfsplus: cannot replace xattr
[  134.581764] hfsplus: cannot replace xattr
[  135.557120] hfsplus: xattr exists yet
 46s
Ran: generic/013
Passed all 1 tests

> I don't think that I follow the point. The two mutexes are namely the basis for potential deadlocks. Currently, I am not sure that we are fixing the issue. Probably, we are trying to hide the symptoms of the real issue without the clear understanding what is going wrong. I would like to hear the explanation how the issue is happening and why the warning removal can help here.

I don't know if the above description is clear enough. Actually, this warning is not helpful at all. 
The comment above this warning also describes one of the easy triggering situations, which can easily trigger and cause xfstest&syzbot to report errors.

> I am not sure that it's the good idea to remove any warning because, probably, we could not understand the real reason of the issue and we simply trying to hind the symptoms of something more serious.
> 
> Current explanation doesn't sound reasonably well to me. I am not convinced yet that it is proper fix and we see the reason of the issue.
> I would like to hear more clear justification that we have to remove this check.

If there is indeed an exception or you can point out the problem, then we should fix it. Otherwise, in my opinion, this warning has no purpose and should be removed.

Thx,
Yangtao

Powered by blists - more mailing lists