On Wed, Apr 23, 2025 at 6:06 AM Vlastimil Babka <vba...@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
> >>
>
