[PATCH v2 1/2] migration/rdma: honor blocking mode in QIOChannelRDMA readv

[PATCH v2 1/2] migration/rdma: honor blocking mode in QIOChannelRDMA readv

[Bin Guo]

QIOChannelRDMA's readv already blocks inside qemu_rdma_exchange_recv()
when waiting for the next RDMA SEND message.  What it did not do was
keep blocking when the bytes from a single receive were insufficient
to satisfy the full request -- it returned partial data (or EAGAIN)
instead of waiting for more.

Loop on qemu_rdma_exchange_recv() when the channel is blocking and
the receive buffer cannot satisfy the request.  This matches the
behaviour of other QIOChannel implementations, which block until
the full request is satisfied (or an error occurs).

Also remove the stale XXX comments about unimplemented blocking support.

Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/rdma.c | 46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 3e37a1d440..201cb9eb12 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -388,7 +388,7 @@ struct QIOChannelRDMA {
     QIOChannel parent;
     RDMAContext *rdmain;
     RDMAContext *rdmaout;
-    bool blocking; /* XXX we don't actually honour this yet */
+    bool blocking;
 };
 
 /*
@@ -2710,32 +2710,29 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
             break;
         }
 
-
-        /* We've got nothing at all, so lets wait for
-         * more to arrive
-         */
-        ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_QEMU_FILE,
-                                      errp);
-
-        if (ret < 0) {
-            rdma->errored = true;
-            return -1;
-        }
-
         /*
-         * SEND was received with new bytes, now try again.
+         * We've got nothing at all, so lets wait for
+         * more to arrive.
          */
-        len = qemu_rdma_fill(rdma, data, want, 0);
-        done += len;
-        want -= len;
-
-        /* Still didn't get enough, so lets just return */
-        if (want) {
-            if (done == 0) {
-                return QIO_CHANNEL_ERR_BLOCK;
-            } else {
-                break;
+        do {
+            ret = qemu_rdma_exchange_recv(rdma, &head,
+                                          RDMA_CONTROL_QEMU_FILE, errp);
+            if (ret < 0) {
+                rdma->errored = true;
+                return -1;
             }
+
+            /*
+             * SEND was received with new bytes, now try again.
+             */
+            len = qemu_rdma_fill(rdma, data, want, 0);
+            done += len;
+            want -= len;
+            data += len;
+        } while (want && rioc->blocking);
+
+        if (want && done == 0) {
+            return QIO_CHANNEL_ERR_BLOCK;
         }
     }
     return done;
@@ -2771,7 +2768,6 @@ static int qio_channel_rdma_set_blocking(QIOChannel *ioc,
                                          Error **errp)
 {
     QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
-    /* XXX we should make readv/writev actually honour this :-) */
     rioc->blocking = blocking;
     return 0;
 }
-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/2] migration/rdma: account transferred bytes for zero page compression

[Bin Guo]

RDMA zero page compression sends an RDMA_CONTROL_COMPRESS message
instead of an RDMA Write, but the transfer accounting
(mig_stats.rdma_bytes and ram_transferred_add) was not updated.
This caused migration progress and bandwidth to be undercounted.

Account the wire cost (RDMAControlHeader + RDMACompress payload),
matching how the non-RDMA path accounts for zero page markers.

Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 migration/rdma.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 201cb9eb12..f08468881f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1930,15 +1930,15 @@ retry:
                 }
 
                 /*
-                 * TODO: Here we are sending something, but we are not
-                 * accounting for anything transferred.  The following is wrong:
-                 *
-                 * stat64_add(&mig_stats.rdma_bytes, sge.length);
-                 *
-                 * because we are using some kind of compression.  I
-                 * would think that head.len would be the more similar
-                 * thing to a correct value.
+                 * Account for the control message we just sent.
+                 * The actual bytes on the wire are the control header
+                 * plus the RDMACompress payload, not the original page
+                 * size (which was never transferred).
                  */
+                qatomic_add(&mig_stats.rdma_bytes,
+                            sizeof(RDMAControlHeader) + sizeof(RDMACompress));
+                ram_transferred_add(sizeof(RDMAControlHeader) +
+                                    sizeof(RDMACompress));
                 qatomic_add(&mig_stats.zero_pages,
                             sge.length / qemu_target_page_size());
                 return 1;
-- 
2.50.1 (Apple Git-155)