On Thu, Aug 01, 2024 at 09:35:08AM -0300, Fabiano Rosas wrote:
> We're about to use MultiFDPages_t from inside the MultiFDSendData
> payload union, which means we cannot have pointers to allocated data
> inside the pages structure, otherwise we'd lose the reference to that
> memory once another payload type touches the union. Move the offset
> array into the end of the structure and turn it into a flexible array
> member, so it is allocated along with the rest of MultiFDSendData in
> the next patches.
>
> Note that other pointers, such as the ramblock pointer are still fine
> as long as the storage for them is not owned by the migration code and
> can be correctly released at some point.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
Reviewed-by: Peter Xu <pet...@redhat.com>
Two nits below..
> ---
> migration/multifd.c | 18 ++++++++++++------
> migration/multifd.h | 4 ++--
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 44d4c3ca11..64503604cf 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -98,6 +98,17 @@ struct {
> MultiFDMethods *ops;
> } *multifd_recv_state;
>
> +static size_t multifd_ram_payload_size(void)
> +{
> + uint32_t n = multifd_ram_page_count();
> +
> + /*
> + * We keep an array of page offsets at the end of MultiFDPages_t,
> + * add space for it in the allocation.
> + */
> + return sizeof(MultiFDPages_t) + n * sizeof(ram_addr_t);
> +}
> +
> static bool multifd_use_packets(void)
> {
> return !migrate_mapped_ram();
> @@ -394,9 +405,7 @@ static int multifd_recv_initial_packet(QIOChannel *c,
> Error **errp)
>
> static MultiFDPages_t *multifd_pages_init(uint32_t n)
> {
> - MultiFDPages_t *pages = g_new0(MultiFDPages_t, 1);
> -
> - pages->offset = g_new0(ram_addr_t, n);
> + MultiFDPages_t *pages = g_malloc0(multifd_ram_payload_size());
>
> return pages;
Can drop the temp var now.
> }
> @@ -404,8 +413,6 @@ static MultiFDPages_t *multifd_pages_init(uint32_t n)
> static void multifd_pages_clear(MultiFDPages_t *pages)
> {
> multifd_pages_reset(pages);
> - g_free(pages->offset);
> - pages->offset = NULL;
> g_free(pages);
> }
>
> @@ -1185,7 +1192,6 @@ bool multifd_send_setup(void)
> qemu_sem_init(&p->sem_sync, 0);
> p->id = i;
> p->pages = multifd_pages_init(page_count);
> -
Unneeded removal.
> if (use_packets) {
> p->packet_len = sizeof(MultiFDPacket_t)
> + sizeof(uint64_t) * page_count;
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 7bb4a2cbc4..a7fdd97f70 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -77,9 +77,9 @@ typedef struct {
> uint32_t num;
> /* number of normal pages */
> uint32_t normal_num;
> + RAMBlock *block;
> /* offset of each page */
> - ram_addr_t *offset;
> - RAMBlock *block;
> + ram_addr_t offset[];
> } MultiFDPages_t;
>
> struct MultiFDRecvData {
> --
> 2.35.3
>
--
Peter Xu
