Re: [Xen-devel] [PATCH v2 1/2] xmalloc: remove struct xmem_pool init_region

[Jan Beulich <JBeulich@xxxxxxxx>]

On 05.07.2019 11:02, Paul Durrant wrote:
> This patch dispenses with the init_region. It's simply not necessary
> (pools will still happily grow and shrink on demand in its absence) and the
> code can be shortended by removing it. It also avoids the sole evaluation
> of ADD_REGION without holding the pool lock (which is unsafe).

Oh, so you've figured there can be even more code removed than
we first thought. Nice.

> @@ -352,13 +343,6 @@ void xmem_pool_destroy(struct xmem_pool *pool)
>       if ( pool == NULL )
>           return;
>   
> -    /* User is destroying without ever allocating from this pool */
> -    if ( xmem_pool_get_used_size(pool) == BHDR_OVERHEAD )
> -    {
> -        ASSERT(!pool->init_region);
> -        pool->used_size -= BHDR_OVERHEAD;
> -    }

I can see that the ASSERT() can (and needs to) go away. But I
don't think you've changed anything elsewhere that would also
allow the entire if() to go away.

> @@ -380,14 +364,6 @@ void *xmem_pool_alloc(unsigned long size, struct 
> xmem_pool *pool)
>       int fl, sl;
>       unsigned long tmp_size;
>   
> -    if ( pool->init_region == NULL )
> -    {
> -        if ( (region = pool->get_mem(pool->init_size)) == NULL )
> -            goto out;
> -        ADD_REGION(region, pool->init_size, pool);
> -        pool->init_region = region;
> -    }

I.e. the code further down in this function turned out to not
depend on there being at least one region in the pool, other
than I was afraid it would. Good - even more pretty a change.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel