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

[Markus Armbruster <armbru@redhat.com>]

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?