Re: [PATCH v2] migration/rdma: add x-rdma-chunk-size parameter

[Markus Armbruster <armbru@redhat.com>]

"Zhang, GuoQing (Sam)" <guoqzhan@amd.com> writes:

> On 2026/3/26 14:57, Markus Armbruster wrote:
>> By convention, we don't post new patches in reply to old ones.  Next
>> time :)
>>
>> Samuel Zhang <guoqing.zhang@amd.com> writes:
>>
>>> The default 1MB RDMA chunk size causes slow live migration because
>>> each chunk triggers a write_flush (ibv_post_send). For 8GB RAM,
>>> 1MB chunk size produce ~15000 flushes vs ~3700 with 1024MB chunk size.
>>>
>>> Add x-rdma-chunk-size parameter to configure the RDMA chunk size for
>>> faster migration.
>>> Usage: `migrate_set_parameter x-rdma-chunk-size 1024M`
>>>
>>> Performance with RDMA live migration of 8GB RAM VM:
>>>
>>> | x-rdma-chunk-size (B) | time (s) | throughput (MB/s) |
>>> |-----------------------|----------|-------------------|
>>> | 1M (default)          | 37.915   |  1,007            |
>>> | 32M                   | 17.880   |  2,260            |
>>> | 1024M                 |  4.368   | 17,529            |
>>>
>>> Signed-off-by: Samuel Zhang <guoqing.zhang@amd.com>

[...]

>>> diff --git a/migration/rdma.c b/migration/rdma.c
>>> index 55ab85650a..3e37a1d440 100644
>>> --- a/migration/rdma.c
>>> +++ b/migration/rdma.c
>>> @@ -45,10 +45,12 @@
>>>   #define RDMA_RESOLVE_TIMEOUT_MS 10000
>>>
>>>   /* Do not merge data if larger than this. */
>>> -#define RDMA_MERGE_MAX (2 * 1024 * 1024)
>>> -#define RDMA_SIGNALED_SEND_MAX (RDMA_MERGE_MAX / 4096)
>>> +static inline uint64_t rdma_merge_max(void)
>>> +{
>>> +    return migrate_rdma_chunk_size() * 2;
>>> +}
>>>
>>> -#define RDMA_REG_CHUNK_SHIFT 20 /* 1 MB */
>>> +#define RDMA_SIGNALED_SEND_MAX 512
>>>
>>>   /*
>>>    * This is only for non-live state being migrated.
>>> @@ -527,21 +529,21 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head,
>>>   static inline uint64_t ram_chunk_index(const uint8_t *start,
>>>                                          const uint8_t *host)
>>>   {
>>> -    return ((uintptr_t) host - (uintptr_t) start) >> RDMA_REG_CHUNK_SHIFT;
>>> +    return ((uintptr_t) host - (uintptr_t) start) / migrate_rdma_chunk_size();
>> Double-checking: this function isn't speed-critical, correct?
>
>
> It's not. it is called once per chunk during live-migration. And no performance change is observed when comparing with original implementation.
>
> Should I change it to `>> ctz64(migrate_rdma_chunk_size())` ?

The code needs to divide by the chunk size in a couple of places.

Before the patch, the chunk size, a power of two, is a compile-time
constant.  It's given as a shift count, but that hardly matters;
compilers are good at optimizing division by constant power of two to a
right shift.

Afterwards, it's a function that returns a power of two.  I don't expect
compilers to optimize division by that to a right shift.

Integer division is expensive.  Matters only on hot paths.  I don't
understand the code nearly enough to judge, so I asked.

We can cache the value of the shift count, and use it to shift instead
of divide.  Complicates the code.  Worthwhile only if the speed gain
matters.  You tell me it doesn't.  I have no reason to doubt you :)

[...]