Re: [PATCH for-7.5 2/5] memcg: use high_work for kernel threads

[Michal Hocko]

On Sat 10-10-26 14:09:54, Shakeel Butt wrote:
> Kernel threads never return to userspace, so deferred memory.high
> reclaim does not run. Reclaim and throttling during a charge can also
> delay work they do for other tasks.
> 
> Treat kernel threads like interrupts. Queue high_work for the first
> cgroup over memory.high and skip reclaim and throttling in the charging
> thread.
> 
> User workers created by create_io_thread() and vhost_task_create() keep
> their current behavior. Vhost kernel threads use the new path, even when
> they borrow the owner's mm.
> 
> The worker requests a fixed batch of pages each time. A fast allocator
> can keep usage well above memory.high. Kernel threads also skip
> memory.swap.high throttling and its high-event reporting. The memory.max
> and memory.swap.max checks stay the same.
> 
> Suggested-by: Tejun Heo <tj@xxxxxxxxxx>
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>

yes this makes sense as a general improvemnt.

Acked-by: Michal Hocko <mhocko@xxxxxxxx>

Thanks!
> ---
>  mm/memcontrol.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 20c5dfe1a894..8116fce6ab96 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2705,6 +2705,7 @@ static void memcg_enforce_high(struct mem_cgroup *memcg, unsigned int nr_pages,
>  			       gfp_t gfp_mask)
>  {
>  	bool allow_spinning = gfpflags_allow_spinning(gfp_mask);
> +	bool use_worker = !in_task() || (current->flags & PF_KTHREAD);
>  
>  	/*
>  	 * If the hierarchy is above the normal consumption range, schedule
> @@ -2723,8 +2724,12 @@ static void memcg_enforce_high(struct mem_cgroup *memcg, unsigned int nr_pages,
>  		swap_high = page_counter_read(&memcg->swap) >
>  			READ_ONCE(memcg->swap.high);
>  
> -		/* Don't bother a random interrupted task */
> -		if (!in_task()) {
> +		/*
> +		 * Don't make an unrelated interrupted task handle this charge.
> +		 * Kernel threads may be working for other tasks, so let a worker
> +		 * reclaim instead.
> +		 */
> +		if (use_worker) {
>  			if (mem_high) {
>  				if (allow_spinning)
>  					schedule_work(&memcg->high_work);
> @@ -2751,6 +2756,10 @@ static void memcg_enforce_high(struct mem_cgroup *memcg, unsigned int nr_pages,
>  		}
>  	} while ((memcg = parent_mem_cgroup(memcg)));
>  
> +	/* Interrupts and kernel threads stop here. */
> +	if (use_worker)
> +		return;
> +
>  	/*
>  	 * Reclaim is set up above to be called from the userland
>  	 * return path. But also attempt synchronous reclaim to avoid
> -- 
> 2.53.0-Meta
> 

-- 
Michal Hocko
SUSE Labs