Re: [PATCH] blk-cgroup: fix lost wakeup in blkg_destroy_all()

Hi,

在 2026/8/11 19:04, Karl Mehltretter 写道:
> wake_up_var() requires a full barrier between making the wait condition
> true and its lockless waitqueue check. spin_unlock_irq() has only
> release semantics, so it does not order q->root_blkg = NULL against
> that check.
>
> During disk rebind, blkcg_init_disk() can therefore miss the wakeup and
> sleep indefinitely. Add the required smp_mb(). Also mark the store with
> WRITE_ONCE() since the waiter reads root_blkg locklessly with
> READ_ONCE(). An LKMM model permits the missed-wakeup outcome without
> the barrier and forbids it with the barrier.
>
> Fixes: 3dbaacf6ab68 ("blk-cgroup: wait for blkcg cleanup before initializing new disk")

Looks like this will be fixed by following patch as well:

[RFC PATCH v2 1/4] blk-cgroup: wait for old blkgs to leave queue before 
disk rebind - Yu Kuai 
<https://lore.kernel.org/all/[email protected]/>

> Cc: [email protected]
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <[email protected]>
> ---
> LKMM (herdtools7 7.58), with the store modeled by WRITE_ONCE(). flag
> is q->root_blkg (1 = set), lk the queue_lock, wq the var waitqueue
> occupancy. To model the fixed code, insert smp_mb() in P0 between
> smp_store_release(lk, 0) and READ_ONCE(*wq):
>
>    C blkcg-rebind-buggy
>    { flag=1; }
>    P0(int *flag, int *lk, int *wq)
>    {
>    	int r0;
>    	WRITE_ONCE(*flag, 0);
>    	smp_store_release(lk, 0);
>    	r0 = READ_ONCE(*wq);
>    }
>    P1(int *flag, int *lk, int *wq)
>    {
>    	int r1;
>    	WRITE_ONCE(*wq, 1);
>    	smp_mb();
>    	r1 = READ_ONCE(*flag);
>    }
>    exists (0:r0=0 /\ 1:r1=1)
>
>    herd7 -conf linux-kernel.cfg blkcg-rebind-buggy.litmus
>
>    without smp_mb(): Sometimes
>    with smp_mb():    Never
>
>   block/blk-cgroup.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index d9676126c5b5d..2b3cf9caeaa31 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -604,9 +604,11 @@ static void blkg_destroy_all(struct gendisk *disk)
>   			__clear_bit(pol->plid, q->blkcg_pols);
>   	}
>
> -	q->root_blkg = NULL;
> +	WRITE_ONCE(q->root_blkg, NULL);
>   	spin_unlock_irq(&q->queue_lock);
>
> +	/* Order q->root_blkg store before wake_up_var()'s waitqueue check */
> +	smp_mb();
>   	wake_up_var(&q->root_blkg);
>   }
>
> --
> 2.53.0
>
-- 
Thanks,
Kuai