* [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents
@ 2025-05-11 11:08 Yangtao Li
2025-05-15 2:01 ` Viacheslav Dubeyko
0 siblings, 1 reply; 6+ messages in thread
From: Yangtao Li @ 2025-05-11 11:08 UTC (permalink / raw)
To: slava, glaubitz, Yangtao Li, Andrew Morton, Ernesto A. Fernández
Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76
Syzbot reported an issue in hfsplus subsystem:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 4400 at fs/hfsplus/extents.c:346
hfsplus_free_extents+0x700/0xad0
Call Trace:
<TASK>
hfsplus_file_truncate+0x768/0xbb0 fs/hfsplus/extents.c:606
hfsplus_write_begin+0xc2/0xd0 fs/hfsplus/inode.c:56
cont_expand_zero fs/buffer.c:2383 [inline]
cont_write_begin+0x2cf/0x860 fs/buffer.c:2446
hfsplus_write_begin+0x86/0xd0 fs/hfsplus/inode.c:52
generic_cont_expand_simple+0x151/0x250 fs/buffer.c:2347
hfsplus_setattr+0x168/0x280 fs/hfsplus/inode.c:263
notify_change+0xe38/0x10f0 fs/attr.c:420
do_truncate+0x1fb/0x2e0 fs/open.c:65
do_sys_ftruncate+0x2eb/0x380 fs/open.c:193
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
To avoid deadlock, Commit 31651c607151 ("hfsplus: avoid deadlock
on file truncation") unlock extree before hfsplus_free_extents(),
and add check wheather extree is locked in hfsplus_free_extents().
However, when operations such as hfsplus_file_release,
hfsplus_setattr, hfsplus_unlink, and hfsplus_unlink are executed
concurrently in different files, it is very likely to trigger the
WARN_ON, which will lead syzbot and xfstest to consider it as an
abnormality.
Since we already have alloc_mutex to protect alloc file modify,
let's remove mutex_lock check.
Fixes: 31651c607151f ("hfsplus: avoid deadlock on file truncation")
Reported-by: syzbot+8c0bc9f818702ff75b76@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/00000000000057fa4605ef101c4c@google.com/
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
fs/hfsplus/extents.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index a6d61685ae79..b1699b3c246a 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -342,9 +342,6 @@ static int hfsplus_free_extents(struct super_block *sb,
int i;
int err = 0;
- /* Mapping the allocation file may lock the extent tree */
- WARN_ON(mutex_is_locked(&HFSPLUS_SB(sb)->ext_tree->tree_lock));
-
hfsplus_dump_extent(extent);
for (i = 0; i < 8; extent++, i++) {
count = be32_to_cpu(extent->block_count);
--
2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents 2025-05-11 11:08 [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents Yangtao Li @ 2025-05-15 2:01 ` Viacheslav Dubeyko 2025-05-25 15:03 ` 回复: " 李扬韬 0 siblings, 1 reply; 6+ messages in thread From: Viacheslav Dubeyko @ 2025-05-15 2:01 UTC (permalink / raw) To: Yangtao Li, glaubitz, Andrew Morton, Ernesto A. Fernández Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76 On Sun, 2025-05-11 at 05:08 -0600, Yangtao Li wrote: > Syzbot reported an issue in hfsplus subsystem: > Could you please add the issue into the issues list [1] (if it is not there yet) and to assign it on yourself? > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 4400 at fs/hfsplus/extents.c:346 > hfsplus_free_extents+0x700/0xad0 > Call Trace: > <TASK> > hfsplus_file_truncate+0x768/0xbb0 fs/hfsplus/extents.c:606 > hfsplus_write_begin+0xc2/0xd0 fs/hfsplus/inode.c:56 > cont_expand_zero fs/buffer.c:2383 [inline] > cont_write_begin+0x2cf/0x860 fs/buffer.c:2446 > hfsplus_write_begin+0x86/0xd0 fs/hfsplus/inode.c:52 > generic_cont_expand_simple+0x151/0x250 fs/buffer.c:2347 > hfsplus_setattr+0x168/0x280 fs/hfsplus/inode.c:263 > notify_change+0xe38/0x10f0 fs/attr.c:420 > do_truncate+0x1fb/0x2e0 fs/open.c:65 > do_sys_ftruncate+0x2eb/0x380 fs/open.c:193 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > To avoid deadlock, Commit 31651c607151 ("hfsplus: avoid deadlock > on file truncation") unlock extree before hfsplus_free_extents(), > and add check wheather extree is locked in hfsplus_free_extents(). > > However, when operations such as hfsplus_file_release, > hfsplus_setattr, hfsplus_unlink, and hfsplus_unlink are executed > concurrently in different files, it is very likely to trigger the > WARN_ON, which will lead syzbot and xfstest to consider it as an > abnormality. > 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? > Since we already have alloc_mutex to protect alloc file modify, > let's remove mutex_lock check. > 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. > Fixes: 31651c607151f ("hfsplus: avoid deadlock on file truncation") > Reported-by: syzbot+8c0bc9f818702ff75b76@syzkaller.appspotmail.com > Closes: > https://lore.kernel.org/all/00000000000057fa4605ef101c4c@google.com/ > Signed-off-by: Yangtao Li <frank.li@vivo.com> > --- > fs/hfsplus/extents.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c > index a6d61685ae79..b1699b3c246a 100644 > --- a/fs/hfsplus/extents.c > +++ b/fs/hfsplus/extents.c > @@ -342,9 +342,6 @@ static int hfsplus_free_extents(struct > super_block *sb, > int i; > int err = 0; > > - /* Mapping the allocation file may lock the extent tree */ > - WARN_ON(mutex_is_locked(&HFSPLUS_SB(sb)->ext_tree- > >tree_lock)); > - 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. Thanks, Slava. > hfsplus_dump_extent(extent); > for (i = 0; i < 8; extent++, i++) { > count = be32_to_cpu(extent->block_count); [1] https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues ^ permalink raw reply [flat|nested] 6+ messages in thread
* 回复: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents 2025-05-15 2:01 ` Viacheslav Dubeyko @ 2025-05-25 15:03 ` 李扬韬 2025-05-27 21:49 ` Viacheslav Dubeyko 0 siblings, 1 reply; 6+ messages in thread From: 李扬韬 @ 2025-05-25 15:03 UTC (permalink / raw) To: Viacheslav Dubeyko, glaubitz, Andrew Morton, Ernesto A. Fernández Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76 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@ubuntu:/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@ubuntu:/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@ubuntu:/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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 回复: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents 2025-05-25 15:03 ` 回复: " 李扬韬 @ 2025-05-27 21:49 ` Viacheslav Dubeyko 2025-05-28 17:01 ` 回复: " 李扬韬 0 siblings, 1 reply; 6+ messages in thread From: Viacheslav Dubeyko @ 2025-05-27 21:49 UTC (permalink / raw) To: 李扬韬, glaubitz, Andrew Morton, Ernesto A. Fernández Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76, Slava.Dubeyko On Sun, 2025-05-25 at 15:03 +0000, 李扬韬 wrote: > 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. > Great! Could you please add generic/013 issues analysis in the patch comment? I mean the dmesg output here. > [Origin] > > We got fsck error, reason is the same as [1]. > > [1] > https://lore.kernel.org/all/20250430001211.1912533-1-slava@dubeyko.com/ > Mentioning [1] could be confusing because it was HFS related fix. But we are discussion HFS+ here. > root@ubuntu:/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. Let's discuss this issue in independent thread. I assume you would like to share the patch with the fix. Do you mean that hfs_bnode_need_zeroout() works not completely correct? Because, I had impression that HFS+ makes clearing of deleted nodes. > > 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@ubuntu:/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@ubuntu:/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 see your point. We need accurately explain here that several threads could try to lock the shared extents tree. And warning can be triggered in one thread when another thread has locked the tree. This is the wrong behavior of the code and we need to remove the warning. Could you please rework the patch comment by means of adding precise explanation of this? > > 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. We have a lot of problems in the code and good warnings make sense. The intentions of this warning was to prevent wrong using of locks. But it was missed that multiple threads can try to lock the shared extents tree. So, yes, it makes sense to remove this particular warning but, potentially, we still could have issues in the code. However, we need to explain why this warning works in wrong way in really precise manner. :) Thanks, Slava. ^ permalink raw reply [flat|nested] 6+ messages in thread
* 回复: 回复: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents 2025-05-27 21:49 ` Viacheslav Dubeyko @ 2025-05-28 17:01 ` 李扬韬 2025-05-28 17:18 ` Viacheslav Dubeyko 0 siblings, 1 reply; 6+ messages in thread From: 李扬韬 @ 2025-05-28 17:01 UTC (permalink / raw) To: Viacheslav Dubeyko, glaubitz, Andrew Morton, Ernesto A. Fernández Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76, Slava.Dubeyko Hi Slava, > Could you please rework the patch comment by means of adding precise explanation of this? I don't quite get your point. Would you mind adding a comment or editing it here? : ) Thx, Yangtao ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 回复: 回复: [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents 2025-05-28 17:01 ` 回复: " 李扬韬 @ 2025-05-28 17:18 ` Viacheslav Dubeyko 0 siblings, 0 replies; 6+ messages in thread From: Viacheslav Dubeyko @ 2025-05-28 17:18 UTC (permalink / raw) To: 李扬韬, glaubitz, Andrew Morton, Ernesto A. Fernández Cc: linux-fsdevel, linux-kernel, syzbot+8c0bc9f818702ff75b76, Slava.Dubeyko On Wed, 2025-05-28 at 17:01 +0000, 李扬韬 wrote: > Hi Slava, > > > Could you please rework the patch comment by means of adding > > precise explanation of this? > > I don't quite get your point. Would you mind adding a comment or > editing it here? : ) > I see your point. We need accurately explain here that several threads could try to lock the shared extents tree. And warning can be triggered in one thread when another thread has locked the tree. This is the wrong behavior of the code and we need to remove the warning. This is what I meant here. Thanks, Slava. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-28 17:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-05-11 11:08 [PATCH] hfsplus: remove mutex_lock check in hfsplus_free_extents Yangtao Li 2025-05-15 2:01 ` Viacheslav Dubeyko 2025-05-25 15:03 ` 回复: " 李扬韬 2025-05-27 21:49 ` Viacheslav Dubeyko 2025-05-28 17:01 ` 回复: " 李扬韬 2025-05-28 17:18 ` Viacheslav Dubeyko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®