Re: [PATCH v3 05/16] mm/page_alloc: unify __alloc_frozen_pages[_nolock]_noprof()

[Brendan Jackman]

On Mon, 29 Jun 2026 at 16:29, <sashiko-bot@xxxxxxxxxx> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Spurious allocation failures for ALLOC_NOLOCK on fragmented systems
> --
>
> commit e9001ddaf8c952ebe08e5042f720b09e0b4a5da8
> Author: Brendan Jackman <jackmanb@xxxxxxxxxx>
>
> mm/page_alloc: unify __alloc_frozen_pages[_nolock]_noprof()
>
> This commit unifies the core allocator entry points by pulling the ALLOC_NOLOCK
> control up into __alloc_frozen_pages_noprof(). It turns the nolock variant
> into a thin wrapper that just sets the ALLOC_NOLOCK flag and handles
> NUMA_NO_NODE.
>
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index a3ba63c7f9199..8d409d075e3e9 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
>
> [ ... ]
>
> > @@ -5310,9 +5384,9 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
> >       fastpath_alloc_flags |= alloc_flags_nofragment(zonelist_zone(ac.preferred_zoneref), gfp);
> >       fastpath_alloc_flags |= alloc_flags_nonblocking(gfp, order) & ALLOC_HIGHATOMIC;
> >
> > -     /* First allocation attempt */
> > +     /* First allocation attempt (or, for nolock, only attempt) */
> >       page = get_page_from_freelist(alloc_gfp, order, fastpath_alloc_flags, &ac);
> > -     if (likely(page))
> > +     if (likely(page) || (alloc_flags & ALLOC_NOLOCK))
> >               goto out;
>
> [Severity: High]
> Could this lead to spurious allocation failures for ALLOC_NOLOCK requests
> on fragmented systems?
>
> In the pre-patch code, alloc_frozen_pages_nolock_noprof() bypassed
> alloc_flags_nofragment(), allowing it to successfully steal pages from
> other migratetypes when its preferred migratetype was empty.

Oh, yep. Given "no functional change intended" this should not set
ALLOC_NOFRAGMENT when ac.alloc_flags has ALLOC_NOLOCK.

(Is this intentional? Do we really want to allow "opportunistic"
allocations, which are very much allowed to fail, to fragment memory?
It doesn't look like this was discussed at when the nolock
functionality was added).