Re: [PATCH v4 0/7] Move memory listener register to vhost_vdpa_init

[Jonah Palmer <jonah.palmer@oracle.com>]

On 7/8/25 4:17 AM, Markus Armbruster wrote:
> Jonah Palmer <jonah.palmer@oracle.com> writes:
> 
>> On 7/4/25 11:00 AM, Markus Armbruster wrote:
>>> Jonah Palmer <jonah.palmer@oracle.com> writes:
> 
> [...]
> 
>>> So, total time increases: early pinning (before main loop) takes more
>>> time than we save pinning (in the main loop).  Correct?
>>
>> Correct. We only save ~0.07s from the pinning that happens in the main loop. But the extra 3s we now need to spend pinning before qemu_main_loop() overshadows it.
> 
> Got it.
> 
>>> We want this trade, because the time spent in the main loop is a
>>> problem: guest-visible downtime.  Correct?
>>> [...]
>>
>> Correct. Though whether or not we want this trade I suppose is subjective. But the 50-60% reduction in guest-visible downtime is pretty nice if we can stomach the initial startup costs.
> 
> I'll get back to this at the end.
> 
> [...]
> 
>>> Let me circle back to my question: Under what circumstances is QMP
>>> responsiveness affected?
>>>
>>> The answer seems to be "only when we're using a vhost-vDPA device".
>>> Correct?
>>
>> Correct, since using one of these guys causes us to do this memory pinning. If we're not using one, it's business as usual for Qemu.
> 
> Got it.
> 
>>> We're using one exactly when QEMU is running with one of its
>>> vhost-vdpa-device-pci* device models.  Correct?
>>
>> Yea, or something like:
>>
>> -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0,... \
>> -device virtio-net-pci,netdev=vhost-vdpa0,id=vdpa0,... \
> 
> I'll get back to this at the end.
> 
> [...]
> 
>>> Let me recap:
>>>
>>> * No change at all unless we're pinning memory early, and we're doing
>>>     that only when we're using a vhost-vDPA device.  Correct?
>>>
>>> * If we are using a vhost-vDPA device:
>>>     - Total startup time (until we're done pinning) increases.
>>
>> Correct.
>>
>>>     - QMP becomes available later.
>>
>> Correct.
>>
>>>     - Main loop behavior improves: less guest-visible downtime, QMP more
>>>       responsive (once it's available)
>>
>> Correct. Though the improvement is modest at best if we put aside the guest-visible downtime improvement.
>>
>>>     This is a tradeoff we want always.  There is no need to let users pick
>>>     "faster startup, worse main loop behavior."
>>>
>>
>> "Always" might be subjective here. For example, if there's no desire to perform live migration, then the user kinda just gets stuck with the cons.
>>
>> Whether or not we want to make this configurable though is another discussion.
>>
>>> Correct?
>>>
>>> [...]
> 
> I think I finally know enough to give you constructive feedback.
> 
> Your commit messages should answer the questions I had.  Specifically:
> 
> * Why are we doing this?  To shorten guest-visible downtime.
> 
> * How are we doing this?  We additionally pin memory before entering the
>    main loop.  This speeds up the pinning we still do in the main loop.
> 
> * Drawback: slower startup.  In particular, QMP becomes
>    available later.
> 
> * Secondary benefit: main loop responsiveness improves, in particular
>    QMP.
> 
> * What uses of QEMU are affected?  Only with vhost-vDPA.  Spell out all
>    the ways to get vhost-vDPA, please.
> 
> * There's a tradeoff.  Show your numbers.  Discuss whether this needs to
>    be configurable.
> 
> If you can make a case for pinning memory this way always, do so.  If
> you believe making it configurable would be a good idea, do so.  If
> you're not sure, say so in the cover letter, and add a suitable TODO
> comment.
> 
> Questions?
> 

No questions, understood.

As I was writing the responses to your questions I was thinking to 
myself that this stuff should've been in the cover letter / commit 
messages in the first place.

Definitely a learning moment for me. Thanks for your time on this Markus!

Jonah