Re: [RFC PATCH v1 2/2] migration/rdma: Allow multiple in-flight writes per chunk

[Peter Xu <peterx@redhat.com>]

On Thu, Aug 20, 2026 at 08:58:33PM +0800, Yanfei Xu wrote:
> qemu_rdma_write_one() waits for an earlier write to the same
> registration chunk to complete.  This serializes disjoint dirty ranges
> in a chunk and leaves send queue capacity unused.
> 
> Remove the per-chunk wait and use the reference counts to track all
> outstanding writes.  The existing per-iteration drain remains the
> completion barrier.

I think this should work (by accident; will explain below), but I want to
raise the definition of iteration here, and it may or may not be what was
expected.

Migration core has this problem likely since 10+ years ago when it start to
have two definitions..

- Each time ram_save_iterate() is invoked: this is the "iteration" that
  RDMA is tracking, it does qemu_rdma_drain_cq() when finishing for each
  call (applies to complete() too)

- Each time migration RAM core syncs dirty info and re-scans the whole
  guest memories (all ramblocks)

For RDMA (and non-RDMA too), what matters is for each same page its new
version always lands *after* its old version.  IIUC, what it really needs
is defintion 2), not 1)..  See the call of multifd_ram_sync_per_round() of
find_dirty_block(), so it was called "a round" there, but I believe we
report such iteration count (in reality, "dirty-sync-count") in QMP
query-migrate with this concept.

I still think relying on the qemu_rdma_drain_cq() should be fine for now,
it's because currently we hold bitmap_mutex across the whole
ram_save_iterate() (NOTE: postcopy preempt may release it.. another thing
to discuss..), so bmap at least for precopy shouldn't be able to change, it
also means for each ram_save_iterate() we shouldn't be sending the same
page twice.  But I think it's risky relying on that fact, e.g. we already
have concurrent sync dirty bitmap, like cpu_throttle_dirty_sync_timer_tick,
so maybe it's better RDMA also flush its pages at definition 2) not 1).
IIUC, it'll also improve on performance because RDMA needs to flush less.

Maybe we should make it a generic API in find_dirty_block(), like
notifiers, so that multifd (and maybe RDMA too?) doesn't need to hard code
things like multifd_ram_flush_and_sync().

Thanks,

> 
> Signed-off-by: Yanfei Xu <yanfei.xu@bytedance.com>
> ---
>  migration/rdma.c       | 19 +------------------
>  migration/trace-events |  1 -
>  2 files changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 973a7a745a..63bc357657 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1869,7 +1869,7 @@ static int qemu_rdma_write_one(RDMAContext *rdma,
>      struct ibv_sge sge;
>      struct ibv_send_wr send_wr = { 0 };
>      struct ibv_send_wr *bad_wr;
> -    int reg_result_idx, ret, count = 0;
> +    int reg_result_idx, ret;
>      uint64_t chunk, chunks;
>      uint64_t chunk_size = migrate_rdma_chunk_size();
>      uint8_t *chunk_start, *chunk_end;
> @@ -1910,23 +1910,6 @@ retry:
>  
>      chunk_end = ram_chunk_end(block, chunk + chunks);
>  
> -
> -    while (qemu_rdma_chunk_in_transit(block, chunk)) {
> -        (void)count;
> -        trace_qemu_rdma_write_one_block(count++, current_index, chunk,
> -                sge.addr, length, rdma->nb_sent, block->nb_chunks);
> -
> -        ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
> -
> -        if (ret < 0) {
> -            error_setg(errp, "Failed to Wait for previous write to complete "
> -                    "block %d chunk %" PRIu64
> -                    " current %" PRIu64 " len %" PRIu64 " %d",
> -                    current_index, chunk, sge.addr, length, rdma->nb_sent);
> -            return -1;
> -        }
> -    }
> -
>      if (!rdma->pin_all || !block->is_ram_block) {
>          if (!block->remote_keys[chunk]) {
>              /*
> diff --git a/migration/trace-events b/migration/trace-events
> index 172761be78..253ff71891 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -249,7 +249,6 @@ qemu_rdma_unregister_waiting_proc(uint64_t chunk, int pos) "Processing unregiste
>  qemu_rdma_unregister_waiting_send(uint64_t chunk) "Sending unregister for chunk: %" PRIu64
>  qemu_rdma_unregister_waiting_complete(uint64_t chunk) "Unregister for chunk: %" PRIu64 " complete."
>  qemu_rdma_write_flush(int sent) "sent total: %d"
> -qemu_rdma_write_one_block(int count, int block, uint64_t chunk, uint64_t current, uint64_t len, int nb_sent, int nb_chunks) "(%d) Not clobbering: block: %d chunk %" PRIu64 " current %" PRIu64 " len %" PRIu64 " %d %d"
>  qemu_rdma_write_one_post(uint64_t chunk, long addr, long remote, uint32_t len) "Posting chunk: %" PRIu64 ", addr: 0x%lx remote: 0x%lx, bytes %" PRIu32
>  qemu_rdma_write_one_queue_full(void) ""
>  qemu_rdma_write_one_recvregres(int mykey, int theirkey, uint64_t chunk) "Received registration result: my key: 0x%x their key 0x%x, chunk %" PRIu64
> -- 
> 2.20.1
> 

-- 
Peter Xu