[Mesa-dev] [PATCH 3/7] util/slab: Add function to flush allocations from a child pool

[Haehnle, Nicolai]

On 21.11.18 12:08, Nicolai Hähnle wrote:
> On 21.11.18 01:39, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Ralloc has a feature that all allocations from a temporary memory
>> context can be whisked away in a single call without fear of leaks.  As
>> the slab allocator is designed for use in multhreaded scenarios with a
>> child pool per CPU, it lacks this feature.  However, many users will be
>> single threaded with a single child pool.  For these users, we can have
>> our cake and eat it too.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Nicolai Hähnle <nicolai.haehnle at amd.com>
>> ---
>>   src/util/slab.c | 21 +++++++++++++++++++++
>>   src/util/slab.h |  1 +
>>   2 files changed, 22 insertions(+)
>>
>> diff --git a/src/util/slab.c b/src/util/slab.c
>> index 5f048666b56..1bcc1db6e09 100644
>> --- a/src/util/slab.c
>> +++ b/src/util/slab.c
>> @@ -172,6 +172,27 @@ void slab_destroy_child(struct slab_child_pool 
>> *pool)
>>      pool->parent = NULL;
>>   }
>> +/**
>> + * Flush all allocations from a pool.  Single-threaded (no mutex).
>> + */
>> +void
>> +slab_flush_st(struct slab_mempool *parent)
> 
> The name of the function argument should be "pool" for consistency.
> 
>> +{
>> +   struct slab_child_pool *const pool = &parent->child;
>> +
>> +   assert(pool->migrated == NULL);
>> +   assert(pool->parent == parent);
> 
> I'm surprised this works. Why isn't pool->parent == &parent->parent?

Oh, I see, it might work (with a warning?) because &parent->parent is an 
alias for parent. But semantically (and with proper type checking) it 
needs to be changed.

Cheers,
Nicolai


> 
> Or, with a consistently named function argument, why isn't 
> pool->child.parent == &pool->parent?
> 
> The intention of the patch looks fine though.
> 
> Cheers,
> Nicolai
> 
> 
>> +
>> +   while (pool->pages) {
>> +      struct slab_page_header *page = pool->pages;
>> +      pool->pages = page->u.next;
>> +
>> +      free(page);
>> +   }
>> +
>> +   pool->free = NULL;
>> +}
>> +
>>   static bool
>>   slab_add_new_page(struct slab_child_pool *pool)
>>   {
>> diff --git a/src/util/slab.h b/src/util/slab.h
>> index e83f8ec1a0e..a4279d8e65b 100644
>> --- a/src/util/slab.h
>> +++ b/src/util/slab.h
>> @@ -90,5 +90,6 @@ void slab_create(struct slab_mempool *pool,
>>   void slab_destroy(struct slab_mempool *pool);
>>   void *slab_alloc_st(struct slab_mempool *pool);
>>   void slab_free_st(struct slab_mempool *pool, void *ptr);
>> +void slab_flush_st(struct slab_mempool *parent);
>>   #endif
>>