| Message ID | 20240801123516.4498-7-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/multifd: Remove multifd_send_state->pages | expand |
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 <farosas@suse.de> Reviewed-by: Peter Xu <peterx@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 >
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; } @@ -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); - 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 {
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 <farosas@suse.de> --- migration/multifd.c | 18 ++++++++++++------ migration/multifd.h | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-)