Re: [PATCH for-7.5 4/5] memcg: avoid queuing high_work from reclaim

[Michal Hocko]

On Sat 10-10-26 14:09:56, Shakeel Butt wrote:
> Kernel threads and remote chargers use high_work when memory.high is
> exceeded. Their allocations during reclaim can also charge memory,
> for example when zswap stores a page.
> 
> Without a check, the high_work worker can queue itself from its own
> reclaim. The pending bit is cleared before high_work runs, so each run
> can queue another run.

What does that mean really? Does this delays the actual run?
 
> Skip memory.high enforcement for charges from task context with
> PF_MEMALLOC set when they would use high_work. This covers kernel
> threads and remote charges. The memory is still charged. Interrupt
> charges keep their current path.
> 
> In a VM with incompressible zswap data, the worker queued itself 163 times
> without this check.
> 
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>

The patch itself looks OK. I am wondering whether we can unify the check
with the existing one which forces the allocation through in main
charging path.

> ---
>  mm/memcontrol.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3456960cbe68..d1e022e6572f 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2708,6 +2708,14 @@ static void memcg_enforce_high(struct mem_cgroup *memcg, unsigned int nr_pages,
>  	bool use_worker = !in_task() || (current->flags & PF_KTHREAD) ||
>  		!current->mm || !mm_match_cgroup(current->mm, memcg);
>  
> +	/*
> +	 * Reclaim can charge memory, for example when zswap stores a page.
> +	 * Don't queue high_work from a reclaiming task, or the
> +	 * high_work worker could keep queuing itself.
> +	 */
> +	if (in_task() && use_worker && (current->flags & PF_MEMALLOC))
> +		return;
> +
>  	/*
>  	 * If the hierarchy is above the normal consumption range, schedule
>  	 * reclaim on returning to userland.  We can perform reclaim here
> -- 
> 2.53.0-Meta

-- 
Michal Hocko
SUSE Labs