Re: [PATCH 1/3] mm/memcontrol: fix reclaim_options leak in try_charge_memcg()

[Yosry Ahmed <yosry@kernel.org>]

On Tue, Mar 17, 2026 at 4:07 PM Bing Jiao <bingjiao@google.com> wrote:
>
> In try_charge_memcg(), the 'reclaim_options' variable is initialized
> once at the start of the function. However, the function contains a
> retry loop. If reclaim_options were modified during an iteration
> (e.g., by encountering a memsw limit), the modified state would
> persist into subsequent retries.
>
> This could lead to incorrect reclaim behavior, such as anon pages
> cannot be reclaimed if memsw has quotas after retries.
>
> Fix by moving the initialization of 'reclaim_options' inside the
> retry loop, ensuring a clean state for every reclaim attempt.
>
> Fixes: 73b73bac90d9 ("mm: vmpressure: don't count proactive reclaim in vmpressure")

Before this commit, we had the same logic with 'may_swap' being
initialized to true and set to false in the retry loop. Before that,
it was 'flags' and 'MEM_CGROUP_RECLAIM_NOSWAP'.

I think initializing whether to swap or not outside the retry loop
started by commit 6539cc053869 ("mm: memcontrol: fold
mem_cgroup_do_charge()") 12 years ago, so I don't think it's a problem
in practice.

Practically speaking, we clear MEMCG_RECLAIM_MAY_SWAP if we hit the
combined memcg->memsw limit. I guess it's theoretically possible (but
probably unlikely) that we try to charge memcg->memsw, fail, reclaim
and/or OOM, then try again, succeed in charging memcg->memsw, but fail
charging memcg->memory. In this case, we should indeed attempt to
swap.

All that being said, this looks correct with the right 'Fixes' tag:

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

> Signed-off-by: Bing Jiao <bingjiao@google.com>
> ---
>  mm/memcontrol.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a47fb68dd65f..303ac622d22d 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2558,7 +2558,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>         struct page_counter *counter;
>         unsigned long nr_reclaimed;
>         bool passed_oom = false;
> -       unsigned int reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
> +       unsigned int reclaim_options;
>         bool drained = false;
>         bool raised_max_event = false;
>         unsigned long pflags;
> @@ -2572,6 +2572,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>                 /* Avoid the refill and flush of the older stock */
>                 batch = nr_pages;
>
> +       reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
>         if (!do_memsw_account() ||
>             page_counter_try_charge(&memcg->memsw, batch, &counter)) {
>                 if (page_counter_try_charge(&memcg->memory, batch, &counter))
> --
> 2.53.0.851.ga537e3e6e9-goog
>