Re: [PATCH v2] writeback: let foreign flushes reach dying cgwbs

[Tejun Heo]

Hello, Liz.

On Mon, Sep 28, 2026 at 10:12:59PM +0000, Liz Fong-Jones wrote:
> +	/* a newer wb may have taken the slot, see cgwb_create() */
> +	spin_lock_irq(&cgwb_lock);
> +	radix_tree_delete_item(&bdi->cgwb_tree, wb->memcg_css->id, wb);
> +	spin_unlock_irq(&cgwb_lock);

Maybe use scoped_guard() here and move list_del(&wb->offline_node) from
further down into the same block?

> +static bool cgwb_dying(struct bdi_writeback *wb)
> +{
> +	lockdep_assert_held(&cgwb_lock);
> +
> +	return percpu_ref_is_dying(&wb->refcnt);
> +}

Can you drop this and use wb_dying() instead? Requiring lockdep for testing
an atomic state is a bit odd.

> +/*
> + * A killed wb stays in bdi->cgwb_tree until it is released, so that foreign
> + * flushes can still find it through wb_get_lookup().  Inodes attached to it

"until it is released or replaced in cgwb_create()"?

>  	wb = radix_tree_lookup(&bdi->cgwb_tree, memcg_css->id);
> -	if (wb && wb->blkcg_css != blkcg_css) {
> +	if (wb && !cgwb_dying(wb) && wb->blkcg_css != blkcg_css)
>  		cgwb_kill(wb);
> +	if (wb && cgwb_dying(wb))
>  		wb = NULL;
> -	}

Maybe filter out dying wbs right after the lookup and leave the mismatch
block as-is?

	wb = radix_tree_lookup(&bdi->cgwb_tree, memcg_css->id);
	if (wb && wb_dying(wb))
		wb = NULL;
	if (wb && wb->blkcg_css != blkcg_css) {
		cgwb_kill(wb);
		wb = NULL;
	}

> +		} else if (cgwb_dying(radix_tree_deref_slot_protected(slot,
> +								      &cgwb_lock))) {
> +			radix_tree_replace_slot(&bdi->cgwb_tree, slot, wb);
> +			ret = 0;

After the takeover, the old wb is out of foreign flushes' reach while its
inodes may still be dirty. Kicking writeback on it would move them over to
the new wb as they get written back. Can you add that as a separate patch
when posting the next version?

>  	radix_tree_for_each_slot(slot, &bdi->cgwb_tree, &iter, 0)
> -		cgwb_kill(*slot);
> +		if (!cgwb_dying(*slot))
> +			cgwb_kill(*slot);

Can you add {} around the loop body?

Thanks.

-- 
tejun