Re: [PATCH v2 2/4] blk-cgroup: fix race between policy activation and blkg destruction

[Nilay Shroff]

On 6/24/26 12:16 PM, Yu Kuai wrote:
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index 7baccfb690fe..f7e788a7fe95 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -1563,10 +1563,12 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
>   	if (WARN_ON_ONCE(!pol->pd_alloc_fn || !pol->pd_free_fn))
>   		return -EINVAL;
>   
>   	if (queue_is_mq(q))
>   		memflags = blk_mq_freeze_queue(q);
> +
> +	mutex_lock(&q->blkcg_mutex);
>   retry:
>   	spin_lock_irq(&q->queue_lock);
>   
>   	/* blkg_list is pushed at the head, reverse walk to initialize parents first */
>   	list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
> @@ -1625,10 +1627,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
>   	__set_bit(pol->plid, q->blkcg_pols);
>   	ret = 0;
>   
>   	spin_unlock_irq(&q->queue_lock);
>   out:
> +	mutex_unlock(&q->blkcg_mutex);
>   	if (queue_is_mq(q))
>   		blk_mq_unfreeze_queue(q, memflags);
>   	if (pinned_blkg)
>   		blkg_put(pinned_blkg);
>   	if (pd_prealloc)

If the policy allocation fails, we jump to the lable enomem: and teardown pds.
But I see this path still only acquires ->queue_lock. Don't we also need
to protect it with ->blkcg_mutex?

Moreover I still see race between blkg insertion in blkg_create() which
still doesn't use ->blkcg_mutex and so list traversal in bfq_end_wr_async()
may still race with blkg_create(), isn't it? I remember you once told
this will be handled in another series but I couldn't find that yet.

Thanks,
--Nilay