|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |