Re: [PATCH v3 14/15] mm/slab: remove __GFP_NO_OBJ_EXT usage from alloc_slab_obj_exts()

[Suren Baghdasaryan]

On Wed, Jun 17, 2026 at 7:57 AM Vlastimil Babka (SUSE)
<vbabka@xxxxxxxxxx> wrote:
>
> On 6/17/26 16:36, Vlastimil Babka (SUSE) wrote:
> >
> >> With some comments below.
> >>
> >> I was worried that perhaps replacing SLAB_ALLOC_NO_RECURSE with
> >> __GFP_NO_OBJ_EXT will create a cycle of
> >>
> >> alloc_slab_obj_exts(SLAB_ALLOC_DEFAULT)
> >> -> kmalloc_flags(SLAB_ALLOC_NO_RECURSE)
> >> -> alloc_from_pcs(SLAB_ALLOC_NO_RECURSE)
> >> -> refill_objects(SLAB_ALLOC_DEFAULT)
> >> -> new_slab(SLAB_ALLOC_DEFAULT)
> >> -> account_slab(SLAB_ALLOC_DEFAULT)
> >> -> alloc_slab_obj_exts(SLAB_ALLOC_DEFAULT)
> >>
> >> with __GFP_NO_OBJ_EXT, it would have been passed to refill_objects(),
> >> but SLAB_ALLOC_NO_RECURSE is not. However this cycle does not exist
> >> because alloc_slab_obj_exts() clears __GFP_ACCOUNT (as part of
> >> OBJCG_CLEAR_MASK) and memory profiling itself does not invoke
> >> alloc_slab_obj_exts() when allocating new slabs if SLAB_ACCOUNT is not
> >> set (which is interesting, by the way).
> >
> > Hm yeah I think we should propagate alloc_flags to refill_objects() etc, to
> > avoid later surprise. But can be done as a later cleanup.
>
> It's also not a new hazard I think because while previously gfp flags with
> __GFP_NO_OBJ_EXT would could be propagated more thoroughly than alloc_flags
> for obj_exts only __alloc_tagging_slab_alloc_hook() looks at them, and
> alloc_slab_obj_exts() (from account_slab()) didn't either, so the amount of
> (finite) recursion is the same I think.

True but I think we should clean that up anyway after this change.

With the fixup you showed, LGTM.
Reviewed-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>

>
> >> Also alloc_slab_obj_exts() propagating SLAB_ALLOC_NEW_SLAB to
> >> kmalloc_flags() is little bit confusing because it does not have any
> >> effect due to SLAB_ALLOC_NO_RECURSE.
> >
> > OK let's address this one by this fixup:
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index fc5b8c85b690..dc4b4ae874ce 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -2164,6 +2164,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> >  {
> >       const bool allow_spin = alloc_flags_allow_spinning(alloc_flags);
> >       unsigned int objects = objs_per_slab(s, slab);
> > +     bool new_slab = alloc_flags & SLAB_ALLOC_NEW_SLAB;
> >       unsigned long new_exts;
> >       unsigned long old_exts;
> >       struct slabobj_ext *vec;
> > @@ -2173,6 +2174,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> >       /* Prevent recursive extension vector allocation */
> >       gfp |= __GFP_NO_OBJ_EXT;
> >       alloc_flags |= SLAB_ALLOC_NO_RECURSE;
> > +     alloc_flags &= ~SLAB_ALLOC_NEW_SLAB;
> >
> >       sz = obj_exts_alloc_size(s, slab, gfp);
> >
> > @@ -2203,7 +2205,7 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> >       old_exts = READ_ONCE(slab->obj_exts);
> >       handle_failed_objexts_alloc(old_exts, vec, objects);
> >
> > -     if (alloc_flags & SLAB_ALLOC_NEW_SLAB) {
> > +     if (new_slab) {
> >               /*
> >                * If the slab is brand new and nobody can yet access its
> >                * obj_exts, no synchronization is required and obj_exts can
> >
>