Re: [PATCH 3/3] migration/rdma: Retry control sends on full queue

[Peter Xu <peterx@redhat.com>]

On Thu, Aug 20, 2026 at 06:27:12PM +0800, Yanfei Xu wrote:
> Hi peter,
> 
> On 2026/8/20 03:18, Peter Xu wrote:
> > On Mon, Aug 17, 2026 at 06:51:17PM +0800, Yanfei Xu wrote:
> > > RAM writes and control messages share the send queue.  If
> > > outstanding writes fill it, RDMA writes drain a completion and retry,
> > > but control sends fail the migration.
> > > 
> > > Drain one outstanding write and retry the control send on ENOMEM.
> > > 
> > > Signed-off-by: Yanfei Xu <yanfei.xu@bytedance.com>
> > > ---
> > >   migration/rdma.c | 12 +++++++++++-
> > >   1 file changed, 11 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > index 6e8436ccc1..d1f44a5f55 100644
> > > --- a/migration/rdma.c
> > > +++ b/migration/rdma.c
> > > @@ -1572,9 +1572,19 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf,
> > >           memcpy(wr->control + sizeof(RDMAControlHeader), buf, head->len);
> > >       }
> > > -
> > > +retry:
> > >       ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr);
> > > +    if (ret == ENOMEM && rdma->nb_sent) {
> > > +        ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
> > > +        if (ret < 0) {
> > > +            error_setg(errp, "rdma migration: failed to make room for "
> > > +                       "control send");
> > > +            return -1;
> > > +        }
> > > +        goto retry;
> > > +    }
> > Looks also correct, but two questions:
> > 
> > - Should we provide a helper instead of duplicating the WRITE op handling?
> >    I believe only WRITE wrids can be on the fly.
> 
> Yes, control message is syncronized, only WRITE can be on the fly. A helper
> is a good suggestion. Will do.
> 
> > 
> > - Could ENOMEM be returned when nb_sent==0?  If that check applies to WRITE
> >    path too?
> 
> ENOMEM can be regarded as SQ is full to RDMA usage in qemu. Actually ENOMEM
> is determined by provider and could have other meaning like
> inline_data > qp->max_inline_data in mlx5. Based on qemu codes, I think it's
> fine without nb_sent==0

I'm not familiar with mlx5 impl that you're discussing here, but IIUC the
point is we should be able to capture all recoverable faults and retry,
meanwhile we should fail immediately on non-recoverable faults.

From the name of the errno (ENOMEM), I expect non-recoverable faults can
happen with it.. unless this is something special to libibverbs to
explicitly imply "queue full"..

I wonder if it means this nb_sent!=0 check should indeed make sense, but I
also wonder if we should add a number of retry so as to capture real ENOMEM
errors otherwise that is not recoverable?  As long as it won't keep
spinning with the same error then we should be good.

> 
> In addition, I encountered this bug when I attempt to send dirty pages
> belongs
> to same chunk in parallel. That could more efficiently utilizes throughput
> when
> many scattered page in one chunk, and quickly exhausts SQ's WRs. Will post a
> RFC with more data.

Sure, I hope that still makes sure different versions of a same page will
be still ordered.

Thanks,

-- 
Peter Xu