Re: [PATCH v4 05/16] libs/guest: allocate various migration arrays just once

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Mon, 8 Jun 2026 at 16:37, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> On 03/06/2026 2:05 pm, Frediano Ziglio wrote:
> > From: Edwin Török <edwin.torok@xxxxxxxxxx>
> >
> > Allocate these array just once at the start of migration,
> > using the maximum batch size, and free them at the end.
> >
> > 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 v3:
> > - fix comment style
>
> These are intentionally freed/reallocated so valgrind can find overflows.
>
> ~Andrew

I understand the value of checking for possible overflow. But this
should not make the programs less efficient.

What about something like this (not tested, just to get the gist):

#if CONFIG_DEBUG && valgrind && whatever
#define MEM_NOACCESS_BUFFER(name, size) uint8_t name[size];
#define MEM_NOACCESS_INIT(field) VALGRIND_MAKE_MEM_NOACCESS(field,
sizeof(field))
#define MEM_UNDEFINED_INIT(field) VALGRIND_MAKE_MEM_UNDEFINED(field,
sizeof(field))
#else
#define MEM_NOACCESS_BUFFER(name, size)
#define MEM_NOACCESS_INIT(field) do {} while(0)
#define MEM_UNDEFINED_INIT(field) do {} while(0)
#endif
...

struct xc_sr_context_save_buffers
{
    MEM_NOACCESS_BUFFER(na0, 16)
    xen_pfn_t batch_pfns[MAX_BATCH_SIZE];
    MEM_NOACCESS_BUFFER(na1, 16)
    xen_pfn_t mfns[MAX_BATCH_SIZE];
    MEM_NOACCESS_BUFFER(na2, 16)
    xen_pfn_t types[MAX_BATCH_SIZE];
    MEM_NOACCESS_BUFFER(na3, 16)
    void *local_pages[MAX_BATCH_SIZE];
    MEM_NOACCESS_BUFFER(na4, 16)
    struct iovec iov[MAX_BATCH_SIZE + 2]; /* Headers + data. */
    MEM_NOACCESS_BUFFER(na5, 16)
    uint64_t rec_pfns[MAX_BATCH_SIZE];
    MEM_NOACCESS_BUFFER(na6, 16)
};
...

    ctx->save.buffers = calloc(1, sizeof(*ctx->save.buffers));

    if ( !dirty_bitmap || !ctx->save.deferred_pages ||
!ctx->save.buffers || !dest_buf )
    {
        ERROR("Unable to allocate memory for dirty bitmaps, deferred pages"
              " and various batch buffers");
        rc = -1;
        errno = ENOMEM;
        goto err;
    }
    ctx->save.batch_pfns = ctx->save.buffers->batch_pfns;
    MEM_NOACCESS_INIT(ctx->save.buffers->na0);
    MEM_NOACCESS_INIT(ctx->save.buffers->na1);
    MEM_NOACCESS_INIT(ctx->save.buffers->na2);
    MEM_NOACCESS_INIT(ctx->save.buffers->na3);
    MEM_NOACCESS_INIT(ctx->save.buffers->na4);
    MEM_NOACCESS_INIT(ctx->save.buffers->na5);
    MEM_NOACCESS_INIT(ctx->save.buffers->na6);
...

    /* Mfns of the batch pfns. */
    MEM_UNDEFINED_INIT(ctx->save.buffers->mfns);
    mfns = ctx->save.buffers->mfns;
    /* Types of the batch pfns. */
    MEM_UNDEFINED_INIT(ctx->save.buffers->types);
    types = ctx->save.buffers->types;
    /* Pointers to locally allocated pages.  Need freeing. */
    MEM_UNDEFINED_INIT(ctx->save.buffers->local_pages);
    local_pages = ctx->save.buffers->local_pages;
    memset(local_pages, 0, sizeof(*local_pages) * nr_pfns);
...

Frediano