Re: [Xen-devel] [PATCH 2/3] xmalloc: don't evaluate ADD_REGION without holding the pool lock

[Jan Beulich <JBeulich@xxxxxxxx>]

On 02.07.2019 18:38, Paul Durrant wrote:
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -380,18 +380,22 @@ void *xmem_pool_alloc(unsigned long size, struct 
> xmem_pool *pool)
>      int fl, sl;
>      unsigned long tmp_size;
>   
> +    spin_lock(&pool->lock);
>      if ( pool->init_region == NULL )
>      {
> +       spin_unlock(&pool->lock);
>         if ( (region = pool->get_mem(pool->init_size)) == NULL )
>             goto out;
> +       spin_lock(&pool->lock);
>         ADD_REGION(region, pool->init_size, pool);
> -       pool->init_region = region;
> +       /* Re-check since the lock was dropped */
> +       if ( pool->init_region == NULL )
> +           pool->init_region = region;
>      }

Instead of this, how about deleting the init_region field?
It's not really used anywhere. I'm not going to exclude that
functions like FIND_SUITABLE_BLOCK() expect _some_ region to
be there in the pool, but that still wouldn't require
tracking which one was the first to get allocated. A check
like that in xmem_pool_destroy() would then do here to make
sure at least one region is there.

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