Message-ID: <7c4fe3af-a38b-4d40-9824-2935b46e1ecd@suse.cz>
Date: Wed, 23 Apr 2025 15:06:57 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Christoph Lameter <cl@...ux.com>, David Rientjes <rientjes@...gle.com>,
 Roman Gushchin <roman.gushchin@...ux.dev>, Harry Yoo <harry.yoo@...cle.com>,
 Uladzislau Rezki <urezki@...il.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, rcu@...r.kernel.org,
 maple-tree@...ts.infradead.org
Subject: Re: [PATCH RFC v3 4/8] slab: sheaf prefilling for guaranteed
 allocations

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)". 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?

>> +{
>> +       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
>>

Powered by blists - more mailing lists