[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 06/16] libs/call: cache up to 4 pages in hypercall bounce buffers



On Fri, Jun 19, 2026 at 02:04:51PM +0100, Frediano Ziglio wrote:
> From: Edwin Török <edwin.torok@xxxxxxxxxx>
> 
> During migration there are a lot of mmap/munmap calls,
> because `xc_get_pfn_type_batch` exceeds the default hypercall bounce
> buffer cache size, and needs to allocate every time it is called.

I think xc_get_pfn_type_batch() would allocate a buffer of 2 page top,
in write_batch(), right ?

> 
> `munmap` is slow, especially in a PV Dom0 (takes an emulation fault),
> so is best avoided.
> 
> Eventually it'd be good if the memory pool from  xmalloc_tlsf.c
> was reused here, but for now make it handle the commonly encountered
> sizes (so far up to 4 pages).
> 
> Signed-off-by: Edwin Török <edwin.torok@xxxxxxxxxx>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> --
> Changes since v2:
> - change prefix in subject.
> 
> Changes since v4:
> - fix off-by-one bug.
> 
> Changes since v5:
> - avoids potential buffer underflow if nr_pages is 0 calling cache_alloc.
> ---
>  tools/libs/call/buffer.c  | 31 ++++++++++++++++++++-----------
>  tools/libs/call/core.c    |  3 ++-
>  tools/libs/call/private.h |  8 +++++---
>  3 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/libs/call/buffer.c b/tools/libs/call/buffer.c
> index 155e4f9d43..2f0515c273 100644
> --- a/tools/libs/call/buffer.c
> +++ b/tools/libs/call/buffer.c
> @@ -49,6 +49,9 @@ static void *cache_alloc(xencall_handle *xcall, size_t 
> nr_pages)
>  {
>      void *p = NULL;
>  
> +    if ( nr_pages == 0 )
> +        return NULL;

By doing that check here, we don't update the stat anymore. And it's
getting out-of-sync with the updates done in cache_free().

Before, we where returning a cache entry for that, and cache_hit++. I
think it's ok to return cache_miss++ instead.

>      cache_lock(xcall);
>  
>      xcall->buffer_total_allocations++;
> @@ -56,13 +59,13 @@ static void *cache_alloc(xencall_handle *xcall, size_t 
> nr_pages)
>      if ( xcall->buffer_current_allocations > 
> xcall->buffer_maximum_allocations )
>          xcall->buffer_maximum_allocations = 
> xcall->buffer_current_allocations;
>  
> -    if ( nr_pages > 1 )
> +    if ( nr_pages > ARRAY_SIZE(xcall->buffer_cache) )
>      {
>          xcall->buffer_cache_toobig++;
>      }
> -    else if ( xcall->buffer_cache_nr > 0 )
> +    else if ( xcall->buffer_cache_nr[nr_pages-1] > 0 )
>      {
> -        p = xcall->buffer_cache[--xcall->buffer_cache_nr];
> +        p = 
> xcall->buffer_cache[nr_pages-1][--xcall->buffer_cache_nr[nr_pages-1]];
>          xcall->buffer_cache_hits++;
>      }
>      else
> @@ -84,10 +87,10 @@ static int cache_free(xencall_handle *xcall, void *p, 
> size_t nr_pages)
>      xcall->buffer_total_releases++;
>      xcall->buffer_current_allocations--;
>  
> -    if ( nr_pages == 1 &&
> -         xcall->buffer_cache_nr < BUFFER_CACHE_SIZE )
> +    if ( nr_pages && nr_pages <= ARRAY_SIZE(xcall->buffer_cache) &&
> +         xcall->buffer_cache_nr[nr_pages-1] < BUFFER_CACHE_SIZE )
>      {
> -        xcall->buffer_cache[xcall->buffer_cache_nr++] = p;
> +        
> xcall->buffer_cache[nr_pages-1][xcall->buffer_cache_nr[nr_pages-1]++] = p;
>          rc = 1;
>      }
>  
> @@ -108,17 +111,23 @@ void buffer_release_cache(xencall_handle *xcall)
>      DBGPRINTF("current allocations:%d maximum allocations:%d",
>                xcall->buffer_current_allocations,
>                xcall->buffer_maximum_allocations);
> -    DBGPRINTF("cache current size:%d",
> -              xcall->buffer_cache_nr);
> +    for ( unsigned i = 0; i < ARRAY_SIZE(xcall->buffer_cache_nr); ++i )
> +    {
> +        DBGPRINTF("cache current size[%u pages]:%d", i+1,
> +                xcall->buffer_cache_nr[i]);
> +    }
>      DBGPRINTF("cache hits:%d misses:%d toobig:%d",
>                xcall->buffer_cache_hits,
>                xcall->buffer_cache_misses,
>                xcall->buffer_cache_toobig);
>  
> -    while ( xcall->buffer_cache_nr > 0 )
> +    for ( unsigned i = 0; i < ARRAY_SIZE(xcall->buffer_cache_nr); ++i )
>      {
> -        p = xcall->buffer_cache[--xcall->buffer_cache_nr];
> -        osdep_free_pages(xcall, p, 1);
> +        while ( xcall->buffer_cache_nr[i] > 0 )
> +        {
> +            p = xcall->buffer_cache[i][--xcall->buffer_cache_nr[i]];
> +            osdep_free_pages(xcall, p, i + 1);
> +        }
>      }
>  
>      cache_unlock(xcall);
> diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
> index 02c4f8e1ae..dd8877c1a0 100644
> --- a/tools/libs/call/core.c
> +++ b/tools/libs/call/core.c
> @@ -14,6 +14,7 @@
>   */
>  
>  #include <stdlib.h>
> +#include <string.h>
>  
>  #include "private.h"
>  
> @@ -44,7 +45,7 @@ xencall_handle *xencall_open(xentoollog_logger *logger, 
> unsigned open_flags)
>      xentoolcore__register_active_handle(&xcall->tc_ah);
>  
>      xcall->flags = open_flags;
> -    xcall->buffer_cache_nr = 0;
> +    memset(xcall->buffer_cache_nr, 0, sizeof(xcall->buffer_cache_nr));
>  
>      xcall->buffer_total_allocations = 0;
>      xcall->buffer_total_releases = 0;
> diff --git a/tools/libs/call/private.h b/tools/libs/call/private.h
> index 9c3aa432ef..8e6a208975 100644
> --- a/tools/libs/call/private.h
> +++ b/tools/libs/call/private.h
> @@ -31,13 +31,15 @@ struct xencall_handle {
>      Xentoolcore__Active_Handle tc_ah;
>  
>      /*
> -     * A simple cache of unused, single page, hypercall buffers
> +     * A simple cache of unused, small, hypercall buffers
> +     * buffer_cache[i]'s size is (i+1) pages
>       *
>       * Protected by a global lock.
>       */
>  #define BUFFER_CACHE_SIZE 4
> -    int buffer_cache_nr;
> -    void *buffer_cache[BUFFER_CACHE_SIZE];
> +#define BUFFER_CACHE_NRPAGES 4
> +    int buffer_cache_nr[BUFFER_CACHE_NRPAGES];
> +    void *buffer_cache[BUFFER_CACHE_NRPAGES][BUFFER_CACHE_SIZE];
>  
>      /*
>       * Hypercall buffer statistics. All protected by the global
> -- 
> 2.43.0
> 
> 


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.