Re: [PATCH RFC v3 4/8] slab: sheaf prefilling for guaranteed allocations

[Suren Baghdasaryan <surenb@google.com>]

On Wed, Apr 23, 2025 at 6:06 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 4/10/25 22:47, Suren Baghdasaryan wrote:
> >> +/*
> >> + * refill a sheaf previously returned by kmem_cache_prefill_sheaf to at least
> >> + * the given size
> >> + *
> >> + * the sheaf might be replaced by a new one when requesting more than
> >> + * s->sheaf_capacity objects if such replacement is necessary, but the refill
> >> + * fails (returning -ENOMEM), the existing sheaf is left intact
> >> + *
> >> + * In practice we always refill to full sheaf's capacity.
> >> + */
> >> +int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,
> >> +                           struct slab_sheaf **sheafp, unsigned int size)
> >
> > nit: Would returning a refilled sheaf be a slightly better API than
> > passing pointer to a pointer?
>
> I'm not sure it would be simpler to use, since we need to be able to
> indicate -ENOMEM which would presumably become NULL, so the user would have
> to store the existing sheaf pointer and not just blindly do "sheaf =
> refill(sheaf)".

Ack.

> Or the semantics would have to be that in case of failure
> the existing sheaf is returned and caller is left with nothing. Liam, what
> do you think?

That sounds confusing. Compared to that alternative, I would prefer
keeping it the way it is now.

>
> >> +{
> >> +       struct slab_sheaf *sheaf;
> >> +
> >> +       /*
> >> +        * TODO: do we want to support *sheaf == NULL to be equivalent of
> >> +        * kmem_cache_prefill_sheaf() ?
> >> +        */
> >> +       if (!sheafp || !(*sheafp))
> >> +               return -EINVAL;
> >> +
> >> +       sheaf = *sheafp;
> >> +       if (sheaf->size >= size)
> >> +               return 0;
> >> +
> >> +       if (likely(sheaf->capacity >= size)) {
> >> +               if (likely(sheaf->capacity == s->sheaf_capacity))
> >> +                       return refill_sheaf(s, sheaf, gfp);
> >> +
> >> +               if (!__kmem_cache_alloc_bulk(s, gfp, sheaf->capacity - sheaf->size,
> >> +                                            &sheaf->objects[sheaf->size])) {
> >> +                       return -ENOMEM;
> >> +               }
> >> +               sheaf->size = sheaf->capacity;
> >> +
> >> +               return 0;
> >> +       }
> >> +
> >> +       /*
> >> +        * We had a regular sized sheaf and need an oversize one, or we had an
> >> +        * oversize one already but need a larger one now.
> >> +        * This should be a very rare path so let's not complicate it.
> >> +        */
> >> +       sheaf = kmem_cache_prefill_sheaf(s, gfp, size);
> >> +       if (!sheaf)
> >> +               return -ENOMEM;
> >> +
> >> +       kmem_cache_return_sheaf(s, gfp, *sheafp);
> >> +       *sheafp = sheaf;
> >> +       return 0;
> >> +}
> >> +
> >> +/*
> >> + * Allocate from a sheaf obtained by kmem_cache_prefill_sheaf()
> >> + *
> >> + * Guaranteed not to fail as many allocations as was the requested size.
> >> + * After the sheaf is emptied, it fails - no fallback to the slab cache itself.
> >> + *
> >> + * The gfp parameter is meant only to specify __GFP_ZERO or __GFP_ACCOUNT
> >> + * memcg charging is forced over limit if necessary, to avoid failure.
> >> + */
> >> +void *
> >> +kmem_cache_alloc_from_sheaf_noprof(struct kmem_cache *s, gfp_t gfp,
> >> +                                  struct slab_sheaf *sheaf)
> >> +{
> >> +       void *ret = NULL;
> >> +       bool init;
> >> +
> >> +       if (sheaf->size == 0)
> >> +               goto out;
> >> +
> >> +       ret = sheaf->objects[--sheaf->size];
> >> +
> >> +       init = slab_want_init_on_alloc(gfp, s);
> >> +
> >> +       /* add __GFP_NOFAIL to force successful memcg charging */
> >> +       slab_post_alloc_hook(s, NULL, gfp | __GFP_NOFAIL, 1, &ret, init, s->object_size);
> >> +out:
> >> +       trace_kmem_cache_alloc(_RET_IP_, ret, s, gfp, NUMA_NO_NODE);
> >> +
> >> +       return ret;
> >> +}
> >> +
> >> +unsigned int kmem_cache_sheaf_size(struct slab_sheaf *sheaf)
> >> +{
> >> +       return sheaf->size;
> >> +}
> >>  /*
> >>   * To avoid unnecessary overhead, we pass through large allocation requests
> >>   * directly to the page allocator. We use __GFP_COMP, because we will need to
> >>
> >> --
> >> 2.48.1
> >>
>