From: Yu Kuai <yukuai@fygo.io>

blkcg_init_disk() currently waits for q->root_blkg to become NULL before
initializing blkcg state for a rebound disk. However, blkg_destroy_all()
clears q->root_blkg after calling blkg_destroy() for each blkg. At that
point the initial references have only been killed, and the blkgs remain
on q->blkg_list until the remaining references drain and
blkg_free_workfn() removes them.

A rebound disk can therefore install new blkcg state while old blkgs are
still attached to the request queue. Wait for q->blkg_list to become empty
instead, and wake the waiter when the final blkg is removed. This covers
the complete queue-side blkg lifetime without adding separate state.

Fixes: 3dbaacf6ab68 ("blk-cgroup: wait for blkcg cleanup before initializing new disk")
Signed-off-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-cgroup.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 2b5c29434e42..6e1e12841880 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -136,6 +136,8 @@ static void blkg_free_workfn(struct work_struct *work)
 		blkg_put(blkg->parent);
 	spin_lock_irq(&q->queue_lock);
 	list_del_init(&blkg->q_node);
+	if (list_empty(&q->blkg_list))
+		wake_up_var(&q->blkg_list);
 	spin_unlock_irq(&q->queue_lock);
 	mutex_unlock(&q->blkcg_mutex);
 
@@ -612,8 +614,6 @@ static void blkg_destroy_all(struct gendisk *disk)
 	q->root_blkg = NULL;
 	spin_unlock_irq(&q->queue_lock);
 	mutex_unlock(&q->blkcg_mutex);
-
-	wake_up_var(&q->root_blkg);
 }
 
 static void blkg_iostat_set(struct blkg_iostat *dst, struct blkg_iostat *src)
@@ -1472,14 +1472,10 @@ int blkcg_init_disk(struct gendisk *disk)
 	/*
 	 * If the queue is shared across disk rebind (e.g., SCSI), the
 	 * previous disk's blkcg state is cleaned up asynchronously via
-	 * disk_release() -> blkcg_exit_disk(). Wait for that cleanup to
-	 * finish (indicated by root_blkg becoming NULL) before setting up
-	 * new blkcg state. Otherwise, we may overwrite q->root_blkg while
-	 * the old one is still alive, and radix_tree_insert() in
-	 * blkg_create() will fail with -EEXIST because the old entries
-	 * still occupy the same queue id slot in blkcg->blkg_tree.
+	 * disk_release() -> blkcg_exit_disk(). Wait for all old blkgs to be
+	 * removed from the queue list before setting up new blkcg state.
 	 */
-	wait_var_event(&q->root_blkg, !READ_ONCE(q->root_blkg));
+	wait_var_event(&q->blkg_list, list_empty_careful(&q->blkg_list));
 
 	new_blkg = blkg_alloc(&blkcg_root, disk, GFP_KERNEL);
 	if (!new_blkg)
-- 
2.51.0

Hi, Jens

在 2026/8/18 14:58, Yu Kuai 写道:

> From: Yu Kuai <yukuai@fygo.io>
>
> blkcg_init_disk() currently waits for q->root_blkg to become NULL before
> initializing blkcg state for a rebound disk. However, blkg_destroy_all()
> clears q->root_blkg after calling blkg_destroy() for each blkg. At that
> point the initial references have only been killed, and the blkgs remain
> on q->blkg_list until the remaining references drain and
> blkg_free_workfn() removes them.
>
> A rebound disk can therefore install new blkcg state while old blkgs are
> still attached to the request queue. Wait for q->blkg_list to become empty
> instead, and wake the waiter when the final blkg is removed. This covers
> the complete queue-side blkg lifetime without adding separate state.
>
> Fixes: 3dbaacf6ab68 ("blk-cgroup: wait for blkcg cleanup before initializing new disk")
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/blk-cgroup.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)

Friendly ping ...

>
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index 2b5c29434e42..6e1e12841880 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -136,6 +136,8 @@ static void blkg_free_workfn(struct work_struct *work)
>   		blkg_put(blkg->parent);
>   	spin_lock_irq(&q->queue_lock);
>   	list_del_init(&blkg->q_node);
> +	if (list_empty(&q->blkg_list))
> +		wake_up_var(&q->blkg_list);
>   	spin_unlock_irq(&q->queue_lock);
>   	mutex_unlock(&q->blkcg_mutex);
>   
> @@ -612,8 +614,6 @@ static void blkg_destroy_all(struct gendisk *disk)
>   	q->root_blkg = NULL;
>   	spin_unlock_irq(&q->queue_lock);
>   	mutex_unlock(&q->blkcg_mutex);
> -
> -	wake_up_var(&q->root_blkg);
>   }
>   
>   static void blkg_iostat_set(struct blkg_iostat *dst, struct blkg_iostat *src)
> @@ -1472,14 +1472,10 @@ int blkcg_init_disk(struct gendisk *disk)
>   	/*
>   	 * If the queue is shared across disk rebind (e.g., SCSI), the
>   	 * previous disk's blkcg state is cleaned up asynchronously via
> -	 * disk_release() -> blkcg_exit_disk(). Wait for that cleanup to
> -	 * finish (indicated by root_blkg becoming NULL) before setting up
> -	 * new blkcg state. Otherwise, we may overwrite q->root_blkg while
> -	 * the old one is still alive, and radix_tree_insert() in
> -	 * blkg_create() will fail with -EEXIST because the old entries
> -	 * still occupy the same queue id slot in blkcg->blkg_tree.
> +	 * disk_release() -> blkcg_exit_disk(). Wait for all old blkgs to be
> +	 * removed from the queue list before setting up new blkcg state.
>   	 */
> -	wait_var_event(&q->root_blkg, !READ_ONCE(q->root_blkg));
> +	wait_var_event(&q->blkg_list, list_empty_careful(&q->blkg_list));
>   
>   	new_blkg = blkg_alloc(&blkcg_root, disk, GFP_KERNEL);
>   	if (!new_blkg)

-- 
Thanks,
Kuai