linux-kernel - Re: [PATCH] mm/page_alloc.c: Avoid infinite retries caused by cpuset race

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJuCfpGEm04Eebt7FK=keu3ZF_34GTTmxMWVCiA1q3RQAVwAOA@mail.gmail.com>
Date: Mon, 21 Apr 2025 13:28:28 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Harry Yoo <harry.yoo@...cle.com>
Cc: Tianyang Zhang <zhangtianyang@...ngson.cn>, akpm@...ux-foundation.org, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	Vlastimil Babka <vbabka@...e.cz>, Michal Hocko <mhocko@...e.com>, Brendan Jackman <jackmanb@...gle.com>, 
	Johannes Weiner <hannes@...xchg.org>, Zi Yan <ziy@...dia.com>
Subject: Re: [PATCH] mm/page_alloc.c: Avoid infinite retries caused by cpuset race

On Mon, Apr 21, 2025 at 3:00 AM Harry Yoo <harry.yoo@...cle.com> wrote:
>
> On Wed, Apr 16, 2025 at 04:24:05PM +0800, Tianyang Zhang wrote:
> > __alloc_pages_slowpath has no change detection for ac->nodemask
> > in the part of retry path, while cpuset can modify it in parallel.
> > For some processes that set mempolicy as MPOL_BIND, this results
> > ac->nodemask changes, and then the should_reclaim_retry will
> > judge based on the latest nodemask and jump to retry, while the
> > get_page_from_freelist only traverses the zonelist from
> > ac->preferred_zoneref, which selected by a expired nodemask
> > and may cause infinite retries in some cases
> >
> > cpu 64:
> > __alloc_pages_slowpath {
> >         /* ..... */
> > retry:
> >         /* ac->nodemask = 0x1, ac->preferred->zone->nid = 1 */
> >         if (alloc_flags & ALLOC_KSWAPD)
> >                 wake_all_kswapds(order, gfp_mask, ac);
> >         /* cpu 1:
> >         cpuset_write_resmask
> >             update_nodemask
> >                 update_nodemasks_hier
> >                     update_tasks_nodemask
> >                         mpol_rebind_task
> >                          mpol_rebind_policy
> >                           mpol_rebind_nodemask
> >               // mempolicy->nodes has been modified,
> >               // which ac->nodemask point to
> >
> >         */
> >         /* ac->nodemask = 0x3, ac->preferred->zone->nid = 1 */
> >         if (should_reclaim_retry(gfp_mask, order, ac, alloc_flags,
> >                                  did_some_progress > 0, &no_progress_loops))
> >                 goto retry;
> > }
> >
> > Simultaneously starting multiple cpuset01 from LTP can quickly
> > reproduce this issue on a multi node server when the maximum
> > memory pressure is reached and the swap is enabled
> >
> > Signed-off-by: Tianyang Zhang <zhangtianyang@...ngson.cn>
> > ---
>
> What commit does it fix and should it be backported to -stable?

I think it fixes 902b62810a57 ("mm, page_alloc: fix more premature OOM
due to race with cpuset update").

>
> There's a new 'MEMORY MANAGEMENT - PAGE ALLOCATOR' entry (only in
> Andrew's mm.git repository now).
>
> Let's Cc the page allocator folks here!
>
> --
> Cheers,
> Harry / Hyeonggon
>
> >  mm/page_alloc.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index fd6b865cb1ab..1e82f5214a42 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -4530,6 +4530,14 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >       }
> >
> >  retry:
> > +     /*
> > +      * Deal with possible cpuset update races or zonelist updates to avoid
> > +      * infinite retries.
> > +      */
> > +     if (check_retry_cpuset(cpuset_mems_cookie, ac) ||
> > +         check_retry_zonelist(zonelist_iter_cookie))
> > +             goto restart;
> > +

We have this check later in this block:
https://elixir.bootlin.com/linux/v6.15-rc3/source/mm/page_alloc.c#L4652,
so IIUC you effectively are moving it to be called before
should_reclaim_retry(). If so, I think you should remove the old one
(the one I linked earlier) as it seems to be unnecessary duplication
at this point.


> >       /* Ensure kswapd doesn't accidentally go to sleep as long as we loop */
> >       if (alloc_flags & ALLOC_KSWAPD)
> >               wake_all_kswapds(order, gfp_mask, ac);
> > --
> > 2.20.1
> >
> >