* [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace
@ 2026-08-21 17:50 FAN YE via B4 Relay
2026-08-21 22:12 ` Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: FAN YE via B4 Relay @ 2026-08-21 17:50 UTC (permalink / raw)
To: Nick Terrell, David Sterba, Chris Mason; +Cc: linux-btrfs, linux-kernel
From: FAN YE <fy15309206903@gmail.com>
A writer can sleep forever in zstd_get_workspace() even though a workspace
is free. When zstd_alloc_workspace() fails, the task is queued on
zwsm->wait and schedules unconditionally, never re-testing the pool.
zstd_put_workspace() publishes the workspace and then calls cond_wake_up(),
which only wakes when a sleeper is already visible, so a workspace returned
between the failed allocation and prepare_to_wait() wakes nobody. The
window is wide: zstd_alloc_workspace() goes through kvmalloc() and may
enter reclaim.
Only a max level workspace triggers the wakeup and one is deliberately kept
allocated as the fallback every waiter waits for, so once its wakeup is
lost the writer stays in TASK_UNINTERRUPTIBLE until some other task happens
to return one. Re-check the pool after prepare_to_wait() has published the
waiter, and use the workspace if one turned up.
Fixes: 3f93aef535c8 ("btrfs: add zstd compression level support")
Assisted-by: Claude:claude-opus-5
Signed-off-by: FAN YE <fy15309206903@gmail.com>
---
Reproduced under QEMU/TCG: CONFIG_FAULT_INJECTION_STACKTRACE_FILTER forces
zstd_alloc_workspace() to fail exactly once and widens the pre-wait window
to 400ms while six concurrent zstd:15 writers race it. Unpatched, a
btrfs-delalloc kworker hangs in zstd_get_workspace()'s schedule() (hung_task
warning, >120s); the identical race against the patched code does not hang.
Compile-tested (W=1, x86_64 defconfig + CONFIG_BTRFS_FS=y).
---
fs/btrfs/zstd.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 86919293fd54..cb15cbd737c4 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -307,8 +307,17 @@ struct list_head *zstd_get_workspace(struct btrfs_fs_info *fs_info, int level)
DEFINE_WAIT(wait);
prepare_to_wait(&zwsm->wait, &wait, TASK_UNINTERRUPTIBLE);
- schedule();
+ /*
+ * Re-check after being queued: zstd_put_workspace() only
+ * wakes a queue that already has a sleeper, so a workspace
+ * returned since the failed allocation woke nobody.
+ */
+ ws = zstd_find_workspace(fs_info, level);
+ if (!ws)
+ schedule();
finish_wait(&zwsm->wait, &wait);
+ if (ws)
+ return ws;
goto again;
}
---
base-commit: 531ed942bb0df04f6747983fecdedce76a22d07f
change-id: 20260821-btrfs-zstd-lost-wakeup-0b0ee88ed52f
Best regards,
--
FAN YE <fy15309206903@gmail.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace 2026-08-21 17:50 [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace FAN YE via B4 Relay @ 2026-08-21 22:12 ` Qu Wenruo 2026-08-22 8:01 ` Qu Wenruo 2026-08-27 8:40 ` Qu Wenruo 2 siblings, 0 replies; 5+ messages in thread From: Qu Wenruo @ 2026-08-21 22:12 UTC (permalink / raw) To: fy15309206903, Nick Terrell, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel 在 2026/8/22 03:20, FAN YE via B4 Relay 写道: > From: FAN YE <fy15309206903@gmail.com> > > A writer can sleep forever in zstd_get_workspace() even though a workspace > is free. When zstd_alloc_workspace() fails, the task is queued on > zwsm->wait and schedules unconditionally, never re-testing the pool. > zstd_put_workspace() publishes the workspace and then calls cond_wake_up(), > which only wakes when a sleeper is already visible, so a workspace returned > between the failed allocation and prepare_to_wait() wakes nobody. The > window is wide: zstd_alloc_workspace() goes through kvmalloc() and may > enter reclaim. > > Only a max level workspace triggers the wakeup Explain why only a max level wq triggers the wakeup, and why we can not simply allow every workqueue to do the wakeup and fix the problem. > and one is deliberately kept > allocated as the fallback every waiter waits for, so once its wakeup is > lost the writer stays in TASK_UNINTERRUPTIBLE until some other task happens > to return one. Re-check the pool after prepare_to_wait() has published the > waiter, and use the workspace if one turned up. > > Fixes: 3f93aef535c8 ("btrfs: add zstd compression level support") > Assisted-by: Claude:claude-opus-5 > Signed-off-by: FAN YE <fy15309206903@gmail.com> > --- > Reproduced under QEMU/TCG: CONFIG_FAULT_INJECTION_STACKTRACE_FILTER forces > zstd_alloc_workspace() to fail exactly once and widens the pre-wait window > to 400ms while six concurrent zstd:15 writers race it. Unpatched, a > btrfs-delalloc kworker hangs in zstd_get_workspace()'s schedule() (hung_task > warning, >120s); the identical race against the patched code does not hang. > Compile-tested (W=1, x86_64 defconfig + CONFIG_BTRFS_FS=y). > --- > fs/btrfs/zstd.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c > index 86919293fd54..cb15cbd737c4 100644 > --- a/fs/btrfs/zstd.c > +++ b/fs/btrfs/zstd.c > @@ -307,8 +307,17 @@ struct list_head *zstd_get_workspace(struct btrfs_fs_info *fs_info, int level) > DEFINE_WAIT(wait); > > prepare_to_wait(&zwsm->wait, &wait, TASK_UNINTERRUPTIBLE); > - schedule(); > + /* > + * Re-check after being queued: zstd_put_workspace() only > + * wakes a queue that already has a sleeper, so a workspace > + * returned since the failed allocation woke nobody. > + */ > + ws = zstd_find_workspace(fs_info, level); > + if (!ws) > + schedule(); > finish_wait(&zwsm->wait, &wait); > + if (ws) > + return ws; > > goto again; > } > > --- > base-commit: 531ed942bb0df04f6747983fecdedce76a22d07f > change-id: 20260821-btrfs-zstd-lost-wakeup-0b0ee88ed52f > > Best regards, > -- > FAN YE <fy15309206903@gmail.com> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace 2026-08-21 17:50 [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace FAN YE via B4 Relay 2026-08-21 22:12 ` Qu Wenruo @ 2026-08-22 8:01 ` Qu Wenruo 2026-08-22 9:37 ` old king 2026-08-27 8:40 ` Qu Wenruo 2 siblings, 1 reply; 5+ messages in thread From: Qu Wenruo @ 2026-08-22 8:01 UTC (permalink / raw) To: fy15309206903, Nick Terrell, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel 在 2026/8/22 03:20, FAN YE via B4 Relay 写道: > From: FAN YE <fy15309206903@gmail.com> > > A writer can sleep forever in zstd_get_workspace() even though a workspace > is free. When zstd_alloc_workspace() fails, the task is queued on > zwsm->wait and schedules unconditionally, never re-testing the pool. > zstd_put_workspace() publishes the workspace and then calls cond_wake_up(), > which only wakes when a sleeper is already visible, so a workspace returned > between the failed allocation and prepare_to_wait() wakes nobody. The > window is wide: zstd_alloc_workspace() goes through kvmalloc() and may > enter reclaim. > > Only a max level workspace triggers the wakeup and one is deliberately kept > allocated as the fallback every waiter waits for, so once its wakeup is > lost the writer stays in TASK_UNINTERRUPTIBLE until some other task happens > to return one. Re-check the pool after prepare_to_wait() has published the > waiter, and use the workspace if one turned up. > > Fixes: 3f93aef535c8 ("btrfs: add zstd compression level support") > Assisted-by: Claude:claude-opus-5 > Signed-off-by: FAN YE <fy15309206903@gmail.com> > --- > Reproduced under QEMU/TCG: CONFIG_FAULT_INJECTION_STACKTRACE_FILTER forces > zstd_alloc_workspace() to fail exactly once and widens the pre-wait window > to 400ms while six concurrent zstd:15 writers race it. Unpatched, a > btrfs-delalloc kworker hangs in zstd_get_workspace()'s schedule() (hung_task > warning, >120s); the identical race against the patched code does not hang. > Compile-tested (W=1, x86_64 defconfig + CONFIG_BTRFS_FS=y). > --- > fs/btrfs/zstd.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c > index 86919293fd54..cb15cbd737c4 100644 > --- a/fs/btrfs/zstd.c > +++ b/fs/btrfs/zstd.c > @@ -307,8 +307,17 @@ struct list_head *zstd_get_workspace(struct btrfs_fs_info *fs_info, int level) > DEFINE_WAIT(wait); > > prepare_to_wait(&zwsm->wait, &wait, TASK_UNINTERRUPTIBLE); > - schedule(); > + /* > + * Re-check after being queued: zstd_put_workspace() only > + * wakes a queue that already has a sleeper, so a workspace > + * returned since the failed allocation woke nobody. > + */ > + ws = zstd_find_workspace(fs_info, level); > + if (!ws) > + schedule(); > finish_wait(&zwsm->wait, &wait); > + if (ws) > + return ws; This doesn't work. Thread A (last ws holder) | Thread B (new ws user) -----------------------------------+-------------------------------- zstd_put_workspace() | | | zstd_get_workspace() | | |- zstd_find_workspace() | | | No WS found | | | | | |- zstd_alloc_workspace() | | | Allocation failure | | | | | |- zstd_find_workspace() | | | Still no WS found |- spin_lock_bh() | | |- spin_unlock_bh() | | |- cond_wake_up() | | No one is waiting | |- prepare_to_wait() | |- schedule() | |- finish_wait() This is the typical LLM behavior, if your human brain can not examine every possibly wrong conclusion/fix from LLM, LLM will only waste time of everyone. > > goto again; > } > > --- > base-commit: 531ed942bb0df04f6747983fecdedce76a22d07f > change-id: 20260821-btrfs-zstd-lost-wakeup-0b0ee88ed52f > > Best regards, > -- > FAN YE <fy15309206903@gmail.com> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace 2026-08-22 8:01 ` Qu Wenruo @ 2026-08-22 9:37 ` old king 0 siblings, 0 replies; 5+ messages in thread From: old king @ 2026-08-22 9:37 UTC (permalink / raw) To: Qu Wenruo Cc: Nick Terrell, David Sterba, Chris Mason, linux-btrfs, linux-kernel Sorry, my two earlier replies went to you only and never reached linux-btrfs. > This is the typical LLM behavior, if your human brain can not examine > every possibly wrong conclusion/fix from LLM, LLM will only waste time > of everyone. Thanks for the review. The LLM told me the diagram is missing the line this patch adds. In your Thread B there is nothing between prepare_to_wait() and schedule(), which is the unpatched sequence. The patch puts the re-check there: Thread A (last ws holder) | Thread B (new ws user) -----------------------------------+--------------------------------- zstd_put_workspace() | ... | |- spin_lock_bh() | |- spin_unlock_bh() | |- cond_wake_up() | No one is waiting | |- prepare_to_wait() | | // THE CODE ADDED BY THIS PATCH: |- zstd_find_workspace() | WS IS FOUND HERE! | | // schedule() is NOT called |- finish_wait() The interleaving you describe is the one the patch handles: if cond_wake_up() sees an empty queue, thread A's list_add() has already completed, so the zstd_find_workspace() after prepare_to_wait() finds that workspace. The reverse order is covered by the barriers. prepare_to_wait() queues the waiter and then calls set_current_state(), which is a full barrier; cond_wake_up() -> wq_has_sleeper() has the matching smp_mb() before it reads the queue. Either A observes the waiter and wakes it, or B observes the workspace and does not sleep. That is the pattern wait_event() relies on. Measured, same kernel image for both rows, the wait order selected by a module parameter. QEMU/TCG, x86_64, 6 concurrent zstd:15 writers, failslab restricted to the zstd_alloc_workspace() call stack via CONFIG_FAULT_INJECTION_STACKTRACE_FILTER (probability=100, times=1, ignore-gfp-wait=0), and an mdelay(400) widening the window between the failed allocation and prepare_to_wait(). Two allocation failures are forced per run, so two tasks sit in that window. Instrumentation counts a max level put whose wakeup found an empty queue while a task was inside the window (dropped), and a re-check after prepare_to_wait() that returned a workspace (recovered): | dropped | recovered | result ------------------+---------+-----------+--------------------------------- unpatched | 136 | - | hang, kworker/u16:0:12 blocked | | | >122s in zstd_get_workspace() patched | 140 | 2 | no hang The wakeups are dropped in both rows, in the window your diagram describes. The difference is what happens next: in the patched row the two tasks that would have slept find a workspace in the re-check and never reach schedule(). One of them is the pid that hangs in the unpatched row. The LLM assistance is the one already declared in the Assisted-by tag. The reasoning above is checked against the code and against these numbers. On Sat, Aug 22, 2026 at 5:01 PM Qu Wenruo <wqu@suse.com> wrote: > > > > 在 2026/8/22 03:20, FAN YE via B4 Relay 写道: > > From: FAN YE <fy15309206903@gmail.com> > > > > A writer can sleep forever in zstd_get_workspace() even though a workspace > > is free. When zstd_alloc_workspace() fails, the task is queued on > > zwsm->wait and schedules unconditionally, never re-testing the pool. > > zstd_put_workspace() publishes the workspace and then calls cond_wake_up(), > > which only wakes when a sleeper is already visible, so a workspace returned > > between the failed allocation and prepare_to_wait() wakes nobody. The > > window is wide: zstd_alloc_workspace() goes through kvmalloc() and may > > enter reclaim. > > > > Only a max level workspace triggers the wakeup and one is deliberately kept > > allocated as the fallback every waiter waits for, so once its wakeup is > > lost the writer stays in TASK_UNINTERRUPTIBLE until some other task happens > > to return one. Re-check the pool after prepare_to_wait() has published the > > waiter, and use the workspace if one turned up. > > > > Fixes: 3f93aef535c8 ("btrfs: add zstd compression level support") > > Assisted-by: Claude:claude-opus-5 > > Signed-off-by: FAN YE <fy15309206903@gmail.com> > > --- > > Reproduced under QEMU/TCG: CONFIG_FAULT_INJECTION_STACKTRACE_FILTER forces > > zstd_alloc_workspace() to fail exactly once and widens the pre-wait window > > to 400ms while six concurrent zstd:15 writers race it. Unpatched, a > > btrfs-delalloc kworker hangs in zstd_get_workspace()'s schedule() (hung_task > > warning, >120s); the identical race against the patched code does not hang. > > Compile-tested (W=1, x86_64 defconfig + CONFIG_BTRFS_FS=y). > > --- > > fs/btrfs/zstd.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c > > index 86919293fd54..cb15cbd737c4 100644 > > --- a/fs/btrfs/zstd.c > > +++ b/fs/btrfs/zstd.c > > @@ -307,8 +307,17 @@ struct list_head *zstd_get_workspace(struct btrfs_fs_info *fs_info, int level) > > DEFINE_WAIT(wait); > > > > prepare_to_wait(&zwsm->wait, &wait, TASK_UNINTERRUPTIBLE); > > - schedule(); > > + /* > > + * Re-check after being queued: zstd_put_workspace() only > > + * wakes a queue that already has a sleeper, so a workspace > > + * returned since the failed allocation woke nobody. > > + */ > > + ws = zstd_find_workspace(fs_info, level); > > + if (!ws) > > + schedule(); > > finish_wait(&zwsm->wait, &wait); > > + if (ws) > > + return ws; > > This doesn't work. > > Thread A (last ws holder) | Thread B (new ws user) > -----------------------------------+-------------------------------- > zstd_put_workspace() | > | | zstd_get_workspace() > | | |- zstd_find_workspace() > | | | No WS found > | | | > | | |- zstd_alloc_workspace() > | | | Allocation failure > | | | > | | |- zstd_find_workspace() > | | | Still no WS found > |- spin_lock_bh() | | > |- spin_unlock_bh() | | > |- cond_wake_up() | | > No one is waiting | |- prepare_to_wait() > | |- schedule() > | |- finish_wait() > > This is the typical LLM behavior, if your human brain can not examine > every possibly wrong conclusion/fix from LLM, LLM will only waste time > of everyone. > > > > goto again; > > } > > > > --- > > base-commit: 531ed942bb0df04f6747983fecdedce76a22d07f > > change-id: 20260821-btrfs-zstd-lost-wakeup-0b0ee88ed52f > > > > Best regards, > > -- > > FAN YE <fy15309206903@gmail.com> > > > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace 2026-08-21 17:50 [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace FAN YE via B4 Relay 2026-08-21 22:12 ` Qu Wenruo 2026-08-22 8:01 ` Qu Wenruo @ 2026-08-27 8:40 ` Qu Wenruo 2 siblings, 0 replies; 5+ messages in thread From: Qu Wenruo @ 2026-08-27 8:40 UTC (permalink / raw) To: fy15309206903, Nick Terrell, David Sterba, Chris Mason Cc: linux-btrfs, linux-kernel 在 2026/8/22 03:20, FAN YE via B4 Relay 写道: > From: FAN YE <fy15309206903@gmail.com> > > A writer can sleep forever in zstd_get_workspace() even though a workspace > is free. When zstd_alloc_workspace() fails, the task is queued on > zwsm->wait and schedules unconditionally, never re-testing the pool. > zstd_put_workspace() publishes the workspace and then calls cond_wake_up(), > which only wakes when a sleeper is already visible, so a workspace returned > between the failed allocation and prepare_to_wait() wakes nobody. The > window is wide: zstd_alloc_workspace() goes through kvmalloc() and may > enter reclaim. > > Only a max level workspace triggers the wakeup and one is deliberately kept > allocated as the fallback every waiter waits for, so once its wakeup is > lost the writer stays in TASK_UNINTERRUPTIBLE until some other task happens > to return one. Re-check the pool after prepare_to_wait() has published the > waiter, and use the workspace if one turned up. > > Fixes: 3f93aef535c8 ("btrfs: add zstd compression level support") > Assisted-by: Claude:claude-opus-5 > Signed-off-by: FAN YE <fy15309206903@gmail.com> Reviewed-by: Qu Wenruo <wqu@suse.com> > --- > Reproduced under QEMU/TCG: CONFIG_FAULT_INJECTION_STACKTRACE_FILTER forces > zstd_alloc_workspace() to fail exactly once and widens the pre-wait window > to 400ms while six concurrent zstd:15 writers race it. Unpatched, a > btrfs-delalloc kworker hangs in zstd_get_workspace()'s schedule() (hung_task > warning, >120s); the identical race against the patched code does not hang. > Compile-tested (W=1, x86_64 defconfig + CONFIG_BTRFS_FS=y). > --- > fs/btrfs/zstd.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c > index 86919293fd54..cb15cbd737c4 100644 > --- a/fs/btrfs/zstd.c > +++ b/fs/btrfs/zstd.c > @@ -307,8 +307,17 @@ struct list_head *zstd_get_workspace(struct btrfs_fs_info *fs_info, int level) > DEFINE_WAIT(wait); > > prepare_to_wait(&zwsm->wait, &wait, TASK_UNINTERRUPTIBLE); > - schedule(); > + /* > + * Re-check after being queued: zstd_put_workspace() only > + * wakes a queue that already has a sleeper, so a workspace > + * returned since the failed allocation woke nobody. > + */ > + ws = zstd_find_workspace(fs_info, level); > + if (!ws) > + schedule(); > finish_wait(&zwsm->wait, &wait); > + if (ws) > + return ws; > > goto again; > } > > --- > base-commit: 531ed942bb0df04f6747983fecdedce76a22d07f > change-id: 20260821-btrfs-zstd-lost-wakeup-0b0ee88ed52f > > Best regards, > -- > FAN YE <fy15309206903@gmail.com> > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-27 8:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-21 17:50 [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace FAN YE via B4 Relay 2026-08-21 22:12 ` Qu Wenruo 2026-08-22 8:01 ` Qu Wenruo 2026-08-22 9:37 ` old king 2026-08-27 8:40 ` Qu Wenruo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox