| Message ID | 20241218134022.21264-1-avihaih@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | migration: Drop/unexport migration_is_device() and migration_is_active() | expand |
On 18/12/2024 13:40, Avihai Horon wrote: > Hello, > > This follows up on Peter's series [1] to simplify migration status API > to a single migration_is_running() function. > > Peter's series tried to drop migration_is_device() and > migration_is_active(), however VFIO used them to check if dirty page > tracking has been started in order to avoid errors in log sync, so they > couldn't simply be dropped without some preliminary cleanups. > > This series handles these preliminary cleanups and eventually drops > migration_is_device() and unexports migration_is_active(). > > The series has been migration tested with the following: > - VFIO device dirty tracking. > - Legacy VFIO iommu dirty tracking. > - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is > currently blocked, so I used a patched QEMU to allow it). > > I also tested calc-dirty-rate as now VFIO dirty pages should be included > in its report, and indeed they are. > > I didn't test it with iommu DPT as I don't have access to such HW. > It would be great if someone with the proper HW could test it. > FWIW tested iommufd DPT (migration and calc-dirty-rate) on said compatible hardware (Milan hardware). Things look to be working as expected. I deferred vIOMMU IOMMUFD DPT testing to my own follow-up once I am back from vacation. Thanks for fixing calc-dirty-rate along the way your cleanup! Joao
On 12/23/24 18:55, Joao Martins wrote: > On 18/12/2024 13:40, Avihai Horon wrote: >> Hello, >> >> This follows up on Peter's series [1] to simplify migration status API >> to a single migration_is_running() function. >> >> Peter's series tried to drop migration_is_device() and >> migration_is_active(), however VFIO used them to check if dirty page >> tracking has been started in order to avoid errors in log sync, so they >> couldn't simply be dropped without some preliminary cleanups. >> >> This series handles these preliminary cleanups and eventually drops >> migration_is_device() and unexports migration_is_active(). >> >> The series has been migration tested with the following: >> - VFIO device dirty tracking. >> - Legacy VFIO iommu dirty tracking. >> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >> currently blocked, so I used a patched QEMU to allow it). >> >> I also tested calc-dirty-rate as now VFIO dirty pages should be included >> in its report, and indeed they are. >> >> I didn't test it with iommu DPT as I don't have access to such HW. >> It would be great if someone with the proper HW could test it. >> > FWIW tested iommufd DPT (migration and calc-dirty-rate) on said compatible > hardware (Milan hardware). Things look to be working as expected. Could we have a Tested-by tag then ? > I deferred > vIOMMU IOMMUFD DPT testing to my own follow-up once I am back from vacation. > > Thanks for fixing calc-dirty-rate along the way your cleanup! Thanks, C.
On 23/12/2024 18:31, Cédric Le Goater wrote: > On 12/23/24 18:55, Joao Martins wrote: >> On 18/12/2024 13:40, Avihai Horon wrote: >>> Hello, >>> >>> This follows up on Peter's series [1] to simplify migration status API >>> to a single migration_is_running() function. >>> >>> Peter's series tried to drop migration_is_device() and >>> migration_is_active(), however VFIO used them to check if dirty page >>> tracking has been started in order to avoid errors in log sync, so they >>> couldn't simply be dropped without some preliminary cleanups. >>> >>> This series handles these preliminary cleanups and eventually drops >>> migration_is_device() and unexports migration_is_active(). >>> >>> The series has been migration tested with the following: >>> - VFIO device dirty tracking. >>> - Legacy VFIO iommu dirty tracking. >>> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >>> currently blocked, so I used a patched QEMU to allow it). >>> >>> I also tested calc-dirty-rate as now VFIO dirty pages should be included >>> in its report, and indeed they are. >>> >>> I didn't test it with iommu DPT as I don't have access to such HW. >>> It would be great if someone with the proper HW could test it. >>> >> FWIW tested iommufd DPT (migration and calc-dirty-rate) on said compatible >> hardware (Milan hardware). Things look to be working as expected. > > Could we have a Tested-by tag then ? > Yes: Tested-by: Joao Martins <joao.m.martins@oracle.com> >> I deferred >> vIOMMU IOMMUFD DPT testing to my own follow-up once I am back from vacation. >> >> Thanks for fixing calc-dirty-rate along the way your cleanup! > > > Thanks, > > C. > >
On 12/23/24 19:43, Joao Martins wrote: > On 23/12/2024 18:31, Cédric Le Goater wrote: >> On 12/23/24 18:55, Joao Martins wrote: >>> On 18/12/2024 13:40, Avihai Horon wrote: >>>> Hello, >>>> >>>> This follows up on Peter's series [1] to simplify migration status API >>>> to a single migration_is_running() function. >>>> >>>> Peter's series tried to drop migration_is_device() and >>>> migration_is_active(), however VFIO used them to check if dirty page >>>> tracking has been started in order to avoid errors in log sync, so they >>>> couldn't simply be dropped without some preliminary cleanups. >>>> >>>> This series handles these preliminary cleanups and eventually drops >>>> migration_is_device() and unexports migration_is_active(). >>>> >>>> The series has been migration tested with the following: >>>> - VFIO device dirty tracking. >>>> - Legacy VFIO iommu dirty tracking. >>>> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >>>> currently blocked, so I used a patched QEMU to allow it). >>>> >>>> I also tested calc-dirty-rate as now VFIO dirty pages should be included >>>> in its report, and indeed they are. >>>> >>>> I didn't test it with iommu DPT as I don't have access to such HW. >>>> It would be great if someone with the proper HW could test it. >>>> >>> FWIW tested iommufd DPT (migration and calc-dirty-rate) on said compatible >>> hardware (Milan hardware). Things look to be working as expected. >> >> Could we have a Tested-by tag then ? >> > > Yes: > > Tested-by: Joao Martins <joao.m.martins@oracle.com> > Applied to vfio-next. Thanks, C.
Fabiano, Peter, On 12/18/24 14:40, Avihai Horon wrote: > Hello, > > This follows up on Peter's series [1] to simplify migration status API > to a single migration_is_running() function. > > Peter's series tried to drop migration_is_device() and > migration_is_active(), however VFIO used them to check if dirty page > tracking has been started in order to avoid errors in log sync, so they > couldn't simply be dropped without some preliminary cleanups. > > This series handles these preliminary cleanups and eventually drops > migration_is_device() and unexports migration_is_active(). > > The series has been migration tested with the following: > - VFIO device dirty tracking. > - Legacy VFIO iommu dirty tracking. > - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is > currently blocked, so I used a patched QEMU to allow it). > > I also tested calc-dirty-rate as now VFIO dirty pages should be included > in its report, and indeed they are. > > I didn't test it with iommu DPT as I don't have access to such HW. > It would be great if someone with the proper HW could test it. > > Patch structure: > 1-4: Refactor and clean up VFIO dirty page tracking helpers. > 5: Refactor dirty limit code. > 6-7: Drop/unexport migration_is_device() and migration_is_running(). I can take these migration changes through the VFIO queue. Is that ok for you ? Thanks, C.
On Tue, Dec 24, 2024 at 09:20:08AM +0100, Cédric Le Goater wrote: > Fabiano, Peter, > > On 12/18/24 14:40, Avihai Horon wrote: > > Hello, > > > > This follows up on Peter's series [1] to simplify migration status API > > to a single migration_is_running() function. > > > > Peter's series tried to drop migration_is_device() and > > migration_is_active(), however VFIO used them to check if dirty page > > tracking has been started in order to avoid errors in log sync, so they > > couldn't simply be dropped without some preliminary cleanups. > > > > This series handles these preliminary cleanups and eventually drops > > migration_is_device() and unexports migration_is_active(). > > > > The series has been migration tested with the following: > > - VFIO device dirty tracking. > > - Legacy VFIO iommu dirty tracking. > > - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is > > currently blocked, so I used a patched QEMU to allow it). > > > > I also tested calc-dirty-rate as now VFIO dirty pages should be included > > in its report, and indeed they are. > > > > I didn't test it with iommu DPT as I don't have access to such HW. > > It would be great if someone with the proper HW could test it. > > > > Patch structure: > > 1-4: Refactor and clean up VFIO dirty page tracking helpers. > > 5: Refactor dirty limit code. > > 6-7: Drop/unexport migration_is_device() and migration_is_running(). > > I can take these migration changes through the VFIO queue. > Is that ok for you ? Fabiano is collecting things for this cycle, but just in case he's in the middle of a party - yes please, thanks!
On 12/24/24 16:12, Peter Xu wrote: > On Tue, Dec 24, 2024 at 09:20:08AM +0100, Cédric Le Goater wrote: >> Fabiano, Peter, >> >> On 12/18/24 14:40, Avihai Horon wrote: >>> Hello, >>> >>> This follows up on Peter's series [1] to simplify migration status API >>> to a single migration_is_running() function. >>> >>> Peter's series tried to drop migration_is_device() and >>> migration_is_active(), however VFIO used them to check if dirty page >>> tracking has been started in order to avoid errors in log sync, so they >>> couldn't simply be dropped without some preliminary cleanups. >>> >>> This series handles these preliminary cleanups and eventually drops >>> migration_is_device() and unexports migration_is_active(). >>> >>> The series has been migration tested with the following: >>> - VFIO device dirty tracking. >>> - Legacy VFIO iommu dirty tracking. >>> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >>> currently blocked, so I used a patched QEMU to allow it). >>> >>> I also tested calc-dirty-rate as now VFIO dirty pages should be included >>> in its report, and indeed they are. >>> >>> I didn't test it with iommu DPT as I don't have access to such HW. >>> It would be great if someone with the proper HW could test it. >>> >>> Patch structure: >>> 1-4: Refactor and clean up VFIO dirty page tracking helpers. >>> 5: Refactor dirty limit code. >>> 6-7: Drop/unexport migration_is_device() and migration_is_running(). >> >> I can take these migration changes through the VFIO queue. >> Is that ok for you ? > > Fabiano is collecting things for this cycle, but just in case he's in the > middle of a party - yes please, thanks! > Sending the PR then ! Thanks, C.
Hello, This follows up on Peter's series [1] to simplify migration status API to a single migration_is_running() function. Peter's series tried to drop migration_is_device() and migration_is_active(), however VFIO used them to check if dirty page tracking has been started in order to avoid errors in log sync, so they couldn't simply be dropped without some preliminary cleanups. This series handles these preliminary cleanups and eventually drops migration_is_device() and unexports migration_is_active(). The series has been migration tested with the following: - VFIO device dirty tracking. - Legacy VFIO iommu dirty tracking. - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is currently blocked, so I used a patched QEMU to allow it). I also tested calc-dirty-rate as now VFIO dirty pages should be included in its report, and indeed they are. I didn't test it with iommu DPT as I don't have access to such HW. It would be great if someone with the proper HW could test it. Patch structure: 1-4: Refactor and clean up VFIO dirty page tracking helpers. 5: Refactor dirty limit code. 6-7: Drop/unexport migration_is_device() and migration_is_running(). Changes from v1 [2]: * Bail out early in vfio_container_set_dirty_page_tracking() if dirty tracking has already been started/stopped. (Joao) * Untied the dirty tracking helpers from migration (i.e., removed migration_is_running() check) so VFIO dirty pages will be included in calc-dirty-rate. (Joao) * Added comment to VFIODevice->dirty_tracking that states it's protected by BQL. * Added R-b/A-b tags. Thanks. [1] https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/ [2] https://lore.kernel.org/qemu-devel/20241216094638.26406-1-avihaih@nvidia.com/ Avihai Horon (7): vfio/container: Add dirty tracking started flag vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic vfio/migration: Rename vfio_devices_all_dirty_tracking() system/dirtylimit: Don't use migration_is_active() migration: Drop migration_is_device() migration: Unexport migration_is_active() include/hw/vfio/vfio-common.h | 6 +-- include/hw/vfio/vfio-container-base.h | 1 + include/migration/misc.h | 2 - hw/vfio/common.c | 57 +++++++++++---------------- hw/vfio/container-base.c | 12 +++++- hw/vfio/container.c | 2 +- migration/migration.c | 23 ++++------- system/dirtylimit.c | 3 +- 8 files changed, 49 insertions(+), 57 deletions(-)