Re: [PATCH v5 06/14] tools: Add sheaves support to testing infrastructure

[Vlastimil Babka <vbabka@suse.cz>]

On 8/22/25 18:56, Suren Baghdasaryan wrote:
>> @@ -270,6 +276,84 @@ __kmem_cache_create_args(const char *name, unsigned int size,
>>         return ret;
>>  }
>>
>> +struct slab_sheaf *
>> +kmem_cache_prefill_sheaf(struct kmem_cache *s, gfp_t gfp, unsigned int size)
>> +{
>> +       struct slab_sheaf *sheaf;
>> +       unsigned int capacity;
>> +
>> +       if (size > s->sheaf_capacity)
>> +               capacity = size;
>> +       else
>> +               capacity = s->sheaf_capacity;
> 
> nit:
> capacity = max(size, s->sheaf_capacity);

OK

>> +
>> +       sheaf = malloc(sizeof(*sheaf) + sizeof(void *) * s->sheaf_capacity * capacity);
> 
> Should this really be `sizeof(void *) * s->sheaf_capacity * capacity`
> or just `sizeof(void *) * capacity` ?

Right, so the whole thing should be:
sizeof(*sheaf) + sizeof(void *) * capacity

> 
>> +       if (!sheaf) {
>> +               return NULL;
>> +       }
>> +
>> +       memset(sheaf, 0, size);

This is also wrong, so I'm changing it to calloc(1, ...) to get the zeroing
there.

>> +       sheaf->cache = s;
>> +       sheaf->capacity = capacity;
>> +       sheaf->size = kmem_cache_alloc_bulk(s, gfp, size, sheaf->objects);
>> +       if (!sheaf->size) {
>> +               free(sheaf);
>> +               return NULL;
>> +       }
>> +
>> +       return sheaf;
>> +}
>> +
>> +int kmem_cache_refill_sheaf(struct kmem_cache *s, gfp_t gfp,
>> +                struct slab_sheaf **sheafp, unsigned int size)
>> +{
>> +       struct slab_sheaf *sheaf = *sheafp;
>> +       int refill;
>> +
>> +       if (sheaf->size >= size)
>> +               return 0;
>> +
>> +       if (size > sheaf->capacity) {
>> +               sheaf = kmem_cache_prefill_sheaf(s, gfp, size);
>> +               if (!sheaf)
>> +                       return -ENOMEM;
>> +
>> +               kmem_cache_return_sheaf(s, gfp, *sheafp);
>> +               *sheafp = sheaf;
>> +               return 0;
>> +       }
>> +
>> +       refill = kmem_cache_alloc_bulk(s, gfp, size - sheaf->size,
>> +                                      &sheaf->objects[sheaf->size]);
>> +       if (!refill)
>> +               return -ENOMEM;
>> +
>> +       sheaf->size += refill;
>> +       return 0;
>> +}
>> +
>> +void kmem_cache_return_sheaf(struct kmem_cache *s, gfp_t gfp,
>> +                struct slab_sheaf *sheaf)
>> +{
>> +       if (sheaf->size) {
>> +               //s->non_kernel += sheaf->size;
> 
> Above comment seems obsolete.

Ack.

> 
>> +               kmem_cache_free_bulk(s, sheaf->size, &sheaf->objects[0]);
>> +       }
>> +       free(sheaf);
>> +}
>> +
>> +void *
>> +kmem_cache_alloc_from_sheaf(struct kmem_cache *s, gfp_t gfp,
>> +               struct slab_sheaf *sheaf)
>> +{
>> +       if (sheaf->size == 0) {
>> +               printf("Nothing left in sheaf!\n");
>> +               return NULL;
>> +       }
>> +
> 
> Should we clear sheaf->objects[sheaf->size] for additional safety?

OK.
>> +       return sheaf->objects[--sheaf->size];
>> +}
>> +
>>  /*
>>   * Test the test infrastructure for kem_cache_alloc/free and bulk counterparts.
>>   */
>>
>> --
>> 2.50.1
>>