Re: [PATCH v2] memcg: Don't wait writeback completion when release memcg.

[Tejun Heo <tj@kernel.org>]

Hello,

On Tue, Aug 26, 2025 at 08:16:18PM +0800, Julian Sun wrote:
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 785173aa0739..f6dd771df369 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -157,11 +157,17 @@ struct mem_cgroup_thresholds {
>   */
>  #define MEMCG_CGWB_FRN_CNT	4
>  
> +struct cgwb_frn_wq_entry {
> +	struct wb_completion *done;
> +	struct wait_queue_entry wq_entry;
> +};

Why not embed wb_completion in the sturct? Also, can you name it
cgwb_frn_wait instead?

>  struct memcg_cgwb_frn {
>  	u64 bdi_id;			/* bdi->id of the foreign inode */
>  	int memcg_id;			/* memcg->css.id of foreign inode */
>  	u64 at;				/* jiffies_64 at the time of dirtying */
> -	struct wb_completion done;	/* tracks in-flight foreign writebacks */
> +	struct wb_completion *done;	/* tracks in-flight foreign writebacks */
> +	struct cgwb_frn_wq_entry *frn_wq; /* used to free resources when release memcg */

And the field just "wait". I know wq is used as an abbreviation for waitq
but it conflicts with workqueue and waitq / wait names seem clearer.

> +static int memcg_cgwb_waitq_callback_fn(struct wait_queue_entry *wq_entry, unsigned int mode,
> +					int flags, void *key)
> +{
> +	struct cgwb_frn_wq_entry *frn_wq_entry = container_of(wq_entry,
> +							struct cgwb_frn_wq_entry, wq_entry);
> +
> +	list_del_init_careful(&wq_entry->entry);

Why list_del_init_careful() instead of just list_del()?

> +	kfree(frn_wq_entry->done);
> +	kfree(frn_wq_entry);

If done is embedded, this will become one free, right?

Thanks.

-- 
tejun