Re: [PATCH v2 02/13] mm/page_alloc: some renames to clarify alloc_flags scopes

[Suren Baghdasaryan]

On Mon, Jun 22, 2026 at 3:01 AM Brendan Jackman <jackmanb@xxxxxxxxxx> wrote:
>
> It's pretty confusing that:
>
> - The slowpath and fastpath have a totally distinct set of alloc_flags.
>
> - gfp_to_alloc_flags() sounds generic but it only influences the
>   slowpath.
>
> - prepare_alloc_pages() is generic in that it sets up the
>   alloc_context, but the alloc_flags it generates are only used for the
>   fastpath.

I understand you want to clarify the usage but this particular point
seems to be an implementation detail. IOW, if tomorrow
__alloc_frozen_pages_noprof() is changed to use alloc_flags when
calling __alloc_pages_slowpath(), would we be renaming it back? So, I
would suggest keeping alloc_flags as is in prepare_alloc_pages() and
its callers. The rest LGTM.

>
> Rename some variables to highlight which alloc_flags are
> fastpath-specific. Rename gfp_to_alloc_flags() to highlight that it's
> slowpath-specific.
>
> gfp_to_alloc_flags_cma()'s current name is actually fine, but rename it
> anyway, just for consistency.
>
> Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>
> ---
>  mm/page_alloc.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6c4eb6908bd95..bc05d75a41627 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3771,8 +3771,8 @@ alloc_flags_nofragment(struct zone *zone, gfp_t gfp_mask)
>  }
>
>  /* Must be called after current_gfp_context() which can change gfp_mask */
> -static inline unsigned int gfp_to_alloc_flags_cma(gfp_t gfp_mask,
> -                                                 unsigned int alloc_flags)
> +static inline unsigned int cma_alloc_flags(gfp_t gfp_mask,
> +                                          unsigned int alloc_flags)
>  {
>  #ifdef CONFIG_CMA
>         if (gfp_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> @@ -4471,7 +4471,7 @@ static void wake_all_kswapds(unsigned int order, gfp_t gfp_mask,
>  }
>
>  static inline unsigned int
> -gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
> +slowpath_alloc_flags(gfp_t gfp_mask, unsigned int order)
>  {
>         unsigned int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET;
>
> @@ -4508,7 +4508,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
>         } else if (unlikely(rt_or_dl_task(current)) && in_task())
>                 alloc_flags |= ALLOC_MIN_RESERVE;
>
> -       alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, alloc_flags);
> +       alloc_flags = cma_alloc_flags(gfp_mask, alloc_flags);
>
>         if (defrag_mode)
>                 alloc_flags |= ALLOC_NOFRAGMENT;
> @@ -4774,7 +4774,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>          * kswapd needs to be woken up, and to avoid the cost of setting up
>          * alloc_flags precisely. So we do that now.
>          */
> -       alloc_flags = gfp_to_alloc_flags(gfp_mask, order);
> +       alloc_flags = slowpath_alloc_flags(gfp_mask, order);
>
>         /*
>          * We need to recalculate the starting point for the zonelist iterator
> @@ -4815,7 +4815,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>
>         reserve_flags = __gfp_pfmemalloc_flags(gfp_mask);
>         if (reserve_flags)
> -               alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, reserve_flags) |
> +               alloc_flags = cma_alloc_flags(gfp_mask, reserve_flags) |
>                                           (alloc_flags & ALLOC_KSWAPD);
>
>         /*
> @@ -5017,7 +5017,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order,
>                 int preferred_nid, nodemask_t *nodemask,
>                 struct alloc_context *ac, gfp_t *alloc_gfp,
> -               unsigned int *alloc_flags)
> +               unsigned int *fastpath_alloc_flags)
>  {
>         ac->highest_zoneidx = gfp_zone(gfp_mask);
>         ac->zonelist = node_zonelist(preferred_nid, gfp_mask);
> @@ -5033,7 +5033,7 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order,
>                 if (in_task() && !ac->nodemask)
>                         ac->nodemask = &cpuset_current_mems_allowed;
>                 else
> -                       *alloc_flags |= ALLOC_CPUSET;
> +                       *fastpath_alloc_flags |= ALLOC_CPUSET;
>         }
>
>         might_alloc(gfp_mask);
> @@ -5042,11 +5042,11 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order,
>          * Don't invoke should_fail logic, since it may call
>          * get_random_u32() and printk() which need to spin_lock.
>          */
> -       if (!(*alloc_flags & ALLOC_NOLOCK) &&
> +       if (!(*fastpath_alloc_flags & ALLOC_NOLOCK) &&
>             should_fail_alloc_page(gfp_mask, order))
>                 return false;
>
> -       *alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, *alloc_flags);
> +       *fastpath_alloc_flags = cma_alloc_flags(gfp_mask, *fastpath_alloc_flags);
>
>         /* Dirty zone balancing only done in the fast path */
>         ac->spread_dirty_pages = (gfp_mask & __GFP_WRITE);
> @@ -5260,7 +5260,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>                 int preferred_nid, nodemask_t *nodemask)
>  {
>         struct page *page;
> -       unsigned int alloc_flags = ALLOC_WMARK_LOW;
> +       unsigned int fastpath_alloc_flags = ALLOC_WMARK_LOW;
>         gfp_t alloc_gfp; /* The gfp_t that was actually used for allocation */
>         struct alloc_context ac = { };
>
> @@ -5282,17 +5282,17 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>         gfp = current_gfp_context(gfp);
>         alloc_gfp = gfp;
>         if (!prepare_alloc_pages(gfp, order, preferred_nid, nodemask, &ac,
> -                       &alloc_gfp, &alloc_flags))
> +                       &alloc_gfp, &fastpath_alloc_flags))
>                 return NULL;
>
>         /*
>          * Forbid the first pass from falling back to types that fragment
>          * memory until all local zones are considered.
>          */
> -       alloc_flags |= alloc_flags_nofragment(zonelist_zone(ac.preferred_zoneref), gfp);
> +       fastpath_alloc_flags |= alloc_flags_nofragment(zonelist_zone(ac.preferred_zoneref), gfp);
>
>         /* First allocation attempt */
> -       page = get_page_from_freelist(alloc_gfp, order, alloc_flags, &ac);
> +       page = get_page_from_freelist(alloc_gfp, order, fastpath_alloc_flags, &ac);
>         if (likely(page))
>                 goto out;
>
>
> --
> 2.54.0
>