From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: "Avihai Horon" <avihaih@nvidia.com>,
qemu-devel@nongnu.org, "Cédric Le Goater" <clg@redhat.com>,
"Juan Quintela" <quintela@redhat.com>,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Peter Xu" <peterx@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Eduardo Habkost" <eduardo@habkost.net>,
"David Hildenbrand" <david@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Yishai Hadas" <yishaih@nvidia.com>,
"Maor Gottlieb" <maorg@nvidia.com>,
"Kirti Wankhede" <kwankhede@nvidia.com>,
"Tarun Gupta" <targupta@nvidia.com>,
"Joao Martins" <joao.m.martins@oracle.com>
Subject: Re: [PATCH v2 03/20] vfio/migration: Add VFIO migration pre-copy support
Date: Mon, 6 Mar 2023 15:01:30 -0400 [thread overview]
Message-ID: <ZAY4irAjkZTaAh2R@nvidia.com> (raw)
In-Reply-To: <20230301153917.243792b8.alex.williamson@redhat.com>
On Wed, Mar 01, 2023 at 03:39:17PM -0700, Alex Williamson wrote:
> On Wed, 1 Mar 2023 17:12:51 -0400
> Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> > On Wed, Mar 01, 2023 at 12:55:59PM -0700, Alex Williamson wrote:
> >
> > > So it seems like what we need here is both a preface buffer size and a
> > > target device latency. The QEMU pre-copy algorithm should factor both
> > > the remaining data size and the device latency into deciding when to
> > > transition to stop-copy, thereby allowing the device to feed actually
> > > relevant data into the algorithm rather than dictate its behavior.
> >
> > I don't know that we can realistically estimate startup latency,
> > especially have the sender estimate latency on the receiver..
>
> Knowing that the target device is compatible with the source is a point
> towards making an educated guess.
>
> > I feel like trying to overlap the device start up with the STOP phase
> > is an unnecessary optimization? How do you see it benifits?
>
> If we can't guarantee that there's some time difference between sending
> initial bytes immediately at the end of pre-copy vs immediately at the
> beginning of stop-copy, does that mean any handling of initial bytes is
> an unnecessary optimization?
Sure if the device doesn't implement an initial_bytes startup phase
then it is all pointless, but probably those devices should return 0
for initial_bytes? If we see initial_bytes and assume it indicates a
startup phase, why not do it?
> I'm imagining that completing initial bytes triggers some
> initialization sequence in the target host driver which runs in
> parallel to the remaining data stream, so in practice, even if sent at
> the beginning of stop-copy, the target device gets a head start.
It isn't parallel in mlx5. The load operation of the initial bytes on
the receiver will execute the load command and that command will take
some amount of time sort of proportional to how much data is in the
device. IIRC the mlx5 VFIO driver will block read until this finishes.
It is convoluted but it ultimately is allocating (potentially alot)
pages in the hypervisor kernel so the time predictability is not very
good.
Other device types we are looking at might do network connections at
this step - eg a storage might open a network connection to its back
end. This could be unpredicatably long in degenerate cases.
> > I've been thinking of this from the perspective that we should always
> > ensure device startup is completed, it is time that has to be paid,
> > why pay it during STOP?
>
> Creating a policy for QEMU to send initial bytes in a given phase
> doesn't ensure startup is complete. There's no guaranteed time
> difference between sending that data and the beginning of stop-copy.
As I've said, to really do a good job here we want to have the sender
wait until the receiver completes startup, and not just treat it as a
unidirectional byte-stream. That isn't this patch..
> QEMU is trying to achieve a downtime goal, where it estimates network
> bandwidth to get a data size threshold, and then polls devices for
> remaining data. That downtime goal might exceed the startup latency of
> the target device anyway, where it's then the operators choice to pay
> that time in stop-copy, or stalled on the target.
If you are saying there should be a policy flag ('optimize for total
migration time' vs 'optimize for minimum downtime') that seems
reasonable, though I wonder who would pick the first option.
> But if we actually want to ensure startup of the target is complete,
> then drivers should be able to return both data size and estimated time
> for the target device to initialize. That time estimate should be
> updated by the driver based on if/when initial_bytes is drained. The
> decision whether to continue iterating pre-copy would then be based on
> both the maximum remaining device startup time and the calculated time
> based on remaining data size.
That seems complicated. Why not just wait for the other side to
acknowledge it has started the device? Then we aren't trying to guess.
AFAIK this sort of happens implicitly in this patch because once
initial bytes is pushed the next data that follows it will block on
the pending load and the single socket will backpressure until the
load is done. Horrible, yes, but it is where qemu is at. multi-fd is
really important :)
Jason
next prev parent reply other threads:[~2023-03-06 19:07 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-22 17:48 [PATCH v2 00/20] vfio: Add migration pre-copy support and device dirty tracking Avihai Horon
2023-02-22 17:48 ` [PATCH v2 01/20] migration: Pass threshold_size to .state_pending_{estimate, exact}() Avihai Horon via
2023-02-22 17:48 ` [PATCH v2 02/20] vfio/migration: Refactor vfio_save_block() to return saved data size Avihai Horon
2023-02-27 14:10 ` Cédric Le Goater
2023-02-22 17:48 ` [PATCH v2 03/20] vfio/migration: Add VFIO migration pre-copy support Avihai Horon
2023-02-22 20:58 ` Alex Williamson
2023-02-23 15:25 ` Avihai Horon
2023-02-23 21:16 ` Alex Williamson
2023-02-26 16:43 ` Avihai Horon
2023-02-27 16:14 ` Alex Williamson
2023-02-27 17:26 ` Jason Gunthorpe
2023-02-27 17:43 ` Alex Williamson
2023-03-01 18:49 ` Avihai Horon
2023-03-01 19:55 ` Alex Williamson
2023-03-01 21:12 ` Jason Gunthorpe
2023-03-01 22:39 ` Alex Williamson
2023-03-06 19:01 ` Jason Gunthorpe [this message]
2023-02-22 17:48 ` [PATCH v2 04/20] vfio/common: Fix error reporting in vfio_get_dirty_bitmap() Avihai Horon
2023-02-22 17:49 ` [PATCH v2 05/20] vfio/common: Fix wrong %m usages Avihai Horon
2023-02-22 17:49 ` [PATCH v2 06/20] vfio/common: Abort migration if dirty log start/stop/sync fails Avihai Horon
2023-02-22 17:49 ` [PATCH v2 07/20] vfio/common: Add VFIOBitmap and (de)alloc functions Avihai Horon
2023-02-22 21:40 ` Alex Williamson
2023-02-23 15:27 ` Avihai Horon
2023-02-27 14:09 ` Cédric Le Goater
2023-03-01 18:56 ` Avihai Horon
2023-03-02 13:24 ` Joao Martins
2023-03-02 14:52 ` Cédric Le Goater
2023-03-02 16:30 ` Joao Martins
2023-03-04 0:23 ` Joao Martins
2023-02-22 17:49 ` [PATCH v2 08/20] util: Add iova_tree_nnodes() Avihai Horon
2023-02-22 17:49 ` [PATCH v2 09/20] util: Extend iova_tree_foreach() to take data argument Avihai Horon
2023-02-22 17:49 ` [PATCH v2 10/20] vfio/common: Record DMA mapped IOVA ranges Avihai Horon
2023-02-22 22:10 ` Alex Williamson
2023-02-23 10:37 ` Joao Martins
2023-02-23 21:05 ` Alex Williamson
2023-02-23 21:19 ` Joao Martins
2023-02-23 21:50 ` Alex Williamson
2023-02-23 21:54 ` Joao Martins
2023-02-28 12:11 ` Joao Martins
2023-02-28 20:36 ` Alex Williamson
2023-03-02 0:07 ` Joao Martins
2023-03-02 0:13 ` Joao Martins
2023-03-02 18:42 ` Alex Williamson
2023-03-03 0:19 ` Joao Martins
2023-03-03 16:58 ` Joao Martins
2023-03-03 17:05 ` Alex Williamson
2023-03-03 19:14 ` Joao Martins
2023-03-03 19:40 ` Alex Williamson
2023-03-03 20:16 ` Joao Martins
2023-03-03 23:47 ` Alex Williamson
2023-03-03 23:57 ` Joao Martins
2023-03-04 0:21 ` Joao Martins
2023-02-22 17:49 ` [PATCH v2 11/20] vfio/common: Add device dirty page tracking start/stop Avihai Horon
2023-02-22 22:40 ` Alex Williamson
2023-02-23 2:02 ` Jason Gunthorpe
2023-02-23 19:27 ` Alex Williamson
2023-02-23 19:30 ` Jason Gunthorpe
2023-02-23 20:16 ` Alex Williamson
2023-02-23 20:54 ` Jason Gunthorpe
2023-02-26 16:54 ` Avihai Horon
2023-02-23 15:36 ` Avihai Horon
2023-02-22 17:49 ` [PATCH v2 12/20] vfio/common: Extract code from vfio_get_dirty_bitmap() to new function Avihai Horon
2023-02-22 17:49 ` [PATCH v2 13/20] vfio/common: Add device dirty page bitmap sync Avihai Horon
2023-02-22 17:49 ` [PATCH v2 14/20] vfio/common: Extract vIOMMU code from vfio_sync_dirty_bitmap() Avihai Horon
2023-02-22 17:49 ` [PATCH v2 15/20] memory/iommu: Add IOMMU_ATTR_MAX_IOVA attribute Avihai Horon
2023-02-22 17:49 ` [PATCH v2 16/20] intel-iommu: Implement get_attr() method Avihai Horon
2023-02-22 17:49 ` [PATCH v2 17/20] vfio/common: Support device dirty page tracking with vIOMMU Avihai Horon
2023-02-22 23:34 ` Alex Williamson
2023-02-23 2:08 ` Jason Gunthorpe
2023-02-23 20:06 ` Alex Williamson
2023-02-23 20:55 ` Jason Gunthorpe
2023-02-23 21:30 ` Joao Martins
2023-02-23 22:33 ` Alex Williamson
2023-02-23 23:26 ` Jason Gunthorpe
2023-02-24 11:25 ` Joao Martins
2023-02-24 12:53 ` Joao Martins
2023-02-24 15:47 ` Jason Gunthorpe
2023-02-24 15:56 ` Alex Williamson
2023-02-24 19:16 ` Joao Martins
2023-02-22 17:49 ` [PATCH v2 18/20] vfio/common: Optimize " Avihai Horon
2023-02-22 17:49 ` [PATCH v2 19/20] vfio/migration: Query device dirty page tracking support Avihai Horon
2023-02-22 17:49 ` [PATCH v2 20/20] docs/devel: Document VFIO device dirty page tracking Avihai Horon
2023-02-27 14:29 ` Cédric Le Goater
2023-02-22 18:00 ` [PATCH v2 00/20] vfio: Add migration pre-copy support and device dirty tracking Avihai Horon
2023-02-22 20:55 ` Alex Williamson
2023-02-23 10:05 ` Cédric Le Goater
2023-02-23 15:07 ` Avihai Horon
2023-02-27 10:24 ` Cédric Le Goater
2023-02-23 14:56 ` Avihai Horon
2023-02-24 19:26 ` Joao Martins
2023-02-26 17:00 ` Avihai Horon
2023-02-27 13:50 ` Cédric Le Goater
2023-03-01 19:04 ` Avihai Horon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZAY4irAjkZTaAh2R@nvidia.com \
--to=jgg@nvidia.com \
--cc=alex.williamson@redhat.com \
--cc=avihaih@nvidia.com \
--cc=clg@redhat.com \
--cc=david@redhat.com \
--cc=dgilbert@redhat.com \
--cc=eduardo@habkost.net \
--cc=jasowang@redhat.com \
--cc=joao.m.martins@oracle.com \
--cc=kwankhede@nvidia.com \
--cc=maorg@nvidia.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=richard.henderson@linaro.org \
--cc=targupta@nvidia.com \
--cc=yishaih@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.