On Mon, Oct 23, 2023 at 05:36:04PM -0300, Fabiano Rosas wrote:
> The new fixed-ram stream format uses a file transport and puts ram
> pages in the migration file at their respective offsets and can be
> done in parallel by using the pwritev system call which takes iovecs
> and an offset.
>
> Add support to enabling the new format along with multifd to make use
> of the threading and page handling already in place.
>
> This requires multifd to stop sending headers and leaving the stream
> format to the fixed-ram code. When it comes time to write the data, we
> need to call a version of qio_channel_write that can take an offset.
>
> Usage on HMP is:
>
> (qemu) stop
> (qemu) migrate_set_capability multifd on
> (qemu) migrate_set_capability fixed-ram on
> (qemu) migrate_set_parameter max-bandwidth 0
> (qemu) migrate_set_parameter multifd-channels 8
> (qemu) migrate file:migfile
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> include/qemu/bitops.h | 13 ++++++++++
> migration/multifd.c | 55 +++++++++++++++++++++++++++++++++++++++++--
> migration/options.c | 6 -----
> migration/ram.c | 2 +-
> 4 files changed, 67 insertions(+), 9 deletions(-)
>
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index cb3526d1f4..2c0a2fe751 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -67,6 +67,19 @@ static inline void clear_bit(long nr, unsigned long *addr)
> *p &= ~mask;
> }
>
> +/**
> + * clear_bit_atomic - Clears a bit in memory atomically
> + * @nr: Bit to clear
> + * @addr: Address to start counting from
> + */
> +static inline void clear_bit_atomic(long nr, unsigned long *addr)
> +{
> + unsigned long mask = BIT_MASK(nr);
> + unsigned long *p = addr + BIT_WORD(nr);
> +
> + return qatomic_and(p, ~mask);
> +}
> +
> /**
> * change_bit - Toggle a bit in memory
> * @nr: Bit to change
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 20e8635740..3f95a41ee9 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -260,6 +260,19 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
> g_free(pages);
> }
>
> +static void multifd_set_file_bitmap(MultiFDSendParams *p)
> +{
> + MultiFDPages_t *pages = p->pages;
> +
> + if (!pages->block) {
> + return;
> + }
> +
> + for (int i = 0; i < p->normal_num; i++) {
> + ramblock_set_shadow_bmap_atomic(pages->block, pages->offset[i]);
> + }
> +}
> +
> static void multifd_send_fill_packet(MultiFDSendParams *p)
> {
> MultiFDPacket_t *packet = p->packet;
> @@ -606,6 +619,29 @@ int multifd_send_sync_main(QEMUFile *f)
> }
> }
>
> + if (!migrate_multifd_packets()) {
> + /*
> + * There's no sync packet to send. Just make sure the sending
> + * above has finished.
> + */
> + for (i = 0; i < migrate_multifd_channels(); i++) {
> + qemu_sem_wait(&multifd_send_state->channels_ready);
> + }
> +
> + /* sanity check and release the channels */
> + for (i = 0; i < migrate_multifd_channels(); i++) {
> + MultiFDSendParams *p = &multifd_send_state->params[i];
> +
> + qemu_mutex_lock(&p->mutex);
> + assert(!p->pending_job || p->quit);
> + qemu_mutex_unlock(&p->mutex);
> +
> + qemu_sem_post(&p->sem);
> + }
> +
> + return 0;
> + }
> +
> /*
> * When using zero-copy, it's necessary to flush the pages before any of
> * the pages can be sent again, so we'll make sure the new version of the
> @@ -689,6 +725,8 @@ static void *multifd_send_thread(void *opaque)
>
> if (p->pending_job) {
> uint32_t flags;
> + uint64_t write_base;
> +
> p->normal_num = 0;
>
> if (!use_packets || use_zero_copy_send) {
> @@ -713,6 +751,16 @@ static void *multifd_send_thread(void *opaque)
> if (use_packets) {
> multifd_send_fill_packet(p);
> p->num_packets++;
> + write_base = 0;
> + } else {
> + multifd_set_file_bitmap(p);
> +
> + /*
> + * If we subtract the host page now, we don't need to
> + * pass it into qio_channel_write_full_all() below.
> + */
> + write_base = p->pages->block->pages_offset -
> + (uint64_t)p->pages->block->host;
> }
>
> flags = p->flags;
> @@ -738,8 +786,9 @@ static void *multifd_send_thread(void *opaque)
> p->iov[0].iov_base = p->packet;
> }
>
> - ret = qio_channel_writev_full_all(p->c, p->iov, p->iovs_num,
> NULL,
> - 0, p->write_flags, &local_err);
> + ret = qio_channel_write_full_all(p->c, p->iov, p->iovs_num,
> + write_base, NULL, 0,
> + p->write_flags, &local_err);
> if (ret != 0) {
> break;
> }
> @@ -969,6 +1018,8 @@ int multifd_save_setup(Error **errp)
>
> if (migrate_zero_copy_send()) {
> p->write_flags = QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> + } else if (!use_packets) {
> + p->write_flags |= QIO_CHANNEL_WRITE_FLAG_WITH_OFFSET;
> } else {
> p->write_flags = 0;
> }
Ah, so this is why you had the wierd overloaded design for
qio_channel_write_full_all in patch 22 that I queried. I'd
still prefer the simpler design at the QIO level, and just
calling the appropriate function above.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
