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

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

[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")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
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

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

[Yu Kuai]

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/20260811064744.1139446-2-yukuai@kernel.org/>

> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 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