Re: [PATCH] btrfs: zstd: fix lost wakeup when waiting for a workspace

[Qu Wenruo <wqu@suse.com>]

在 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>
> 
> 
>