Re: [PATCH] mm:vmscan: fix inaccurate reclaim during proactive reclaim

Johannes Weiner
2023-07-20T16:24:47Z

On Thu, Jul 20, 2023 at 03:27:08PM +0800, Efly Young wrote:
> Before commit f53af4285d77 ("mm: vmscan: fix extreme overreclaim and
> swap floods"), proactive reclaim will extreme overreclaim sometimes.
> But proactive reclaim still inaccurate and some extent overreclaim.
>
> Problematic case is easy to construct. Allocate lots of anonymous
> memory (e.g., 20G) in a memcg, then swapping by writing memory.recalim
> and there is a certain probability of overreclaim. For example, request
> 1G by writing memory.reclaim will eventually reclaim 1.7G or other
> values more than 1G.
>
> The reason is that reclaimer may have already reclaimed part of requested
> memory in one loop, but before adjust sc->nr_to_reclaim in outer loop,
> call shrink_lruvec() again will still follow the current sc->nr_to_reclaim
> to work. It will eventually lead to overreclaim. In theory, the amount
> of reclaimed would be in [request, 2 * request).
>
> Reclaimer usually tends to reclaim more than request. But either direct
> or kswapd reclaim have much smaller nr_to_reclaim targets, so it is
> less noticeable and not have much impact.
>
> Proactive reclaim can usually come in with a larger value, so the error
> is difficult to ignore. Considering proactive reclaim is usually low
> frequency, handle the batching into smaller chunks is a better approach.
>
> Signed-off-by: Efly Young <yangyifei03@kuaishou.com>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Hey, I didn't write the patch, you did :) Please change it to

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>

You can also add

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

[quoting remainder for new CCs]

> ---
> mm/memcontrol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 4b27e24..d36cf88 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -6741,8 +6741,8 @@ static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf,
> lru_add_drain_all();
>
> reclaimed = try_to_free_mem_cgroup_pages(memcg,
> - nr_to_reclaim - nr_reclaimed,
> - GFP_KERNEL, reclaim_options);
> + min(nr_to_reclaim - nr_reclaimed, SWAP_CLUSTER_MAX),
> + GFP_KERNEL, reclaim_options);
>
> if (!reclaimed && !nr_retries--)
> return -EAGAIN;
> --
> 1.8.3.1