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

[Tejun Heo <tj@kernel.org>]

Hello,

A couple minor points:

> +struct cgwb_frn_wait {
> +	struct wb_completion done;
> +	struct wait_queue_entry wq_entry;
> +};

Can you add a comment on top of cgwb_frn_wait to explain how it's used and
why?

> @@ -3912,8 +3940,21 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
>  	int __maybe_unused i;
>  
>  #ifdef CONFIG_CGROUP_WRITEBACK
> -	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++)
> -		wb_wait_for_completion(&memcg->cgwb_frn[i].done);
> +	spin_lock(&memcg_cgwb_frn_waitq.lock);
> +	for (i = 0; i < MEMCG_CGWB_FRN_CNT; i++) {
> +		struct cgwb_frn_wait *wait = memcg->cgwb_frn[i].wait;
> +
> +		/*
> +		 * Not necessary to wait for wb completion which might cause task hung,
> +		 * only used to free resources. See memcg_cgwb_waitq_callback_fn().
> +		 */
> +		__add_wait_queue_entry_tail(wait->done.waitq, &wait->wq_entry);
> +		if (atomic_dec_and_test(&wait->done.cnt)) {
> +			list_del(&wait->wq_entry.entry);

__remove_wait_queue()?

Looks good to me otherwise. Can you please cc Andrew Morton and memcg
maintainers on the next post?

Thanks.

-- 
tejun