diff mbox series

[v2,2/7] vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic

Message ID 20241218134022.21264-3-avihaih@nvidia.com (mailing list archive)
State New
Headers show
Series migration: Drop/unexport migration_is_device() and migration_is_active() | expand

Commit Message

Avihai Horon Dec. 18, 2024, 1:40 p.m. UTC
During dirty page log sync, vfio_devices_all_dirty_tracking() is used to
check if dirty tracking has been started in order to avoid errors. The
current logic checks if migration is in ACTIVE or DEVICE states to
ensure dirty tracking has been started.

However, recently there has been an effort to simplify the migration
status API and reduce it to a single migration_is_running() function.

To accommodate this, refactor vfio_devices_all_dirty_tracking() logic so
it won't use migration_is_active() and migration_is_device(). Instead,
use internal VFIO dirty tracking flags.

As a side effect, now that migration status is no longer used to detect
dirty tracking status, VFIO log syncs are untied from migration. This
will make calc-dirty-rate more accurate as now it will also include VFIO
dirty pages.

While at it, as VFIODevice->dirty_tracking is now used to detect dirty
tracking status, add a comment that states how it's protected.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/hw/vfio/vfio-common.h |  2 +-
 hw/vfio/common.c              | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Joao Martins Dec. 23, 2024, 5:42 p.m. UTC | #1
On 18/12/2024 13:40, Avihai Horon wrote:
> During dirty page log sync, vfio_devices_all_dirty_tracking() is used to
> check if dirty tracking has been started in order to avoid errors. The
> current logic checks if migration is in ACTIVE or DEVICE states to
> ensure dirty tracking has been started.
> 
> However, recently there has been an effort to simplify the migration
> status API and reduce it to a single migration_is_running() function.
> 
> To accommodate this, refactor vfio_devices_all_dirty_tracking() logic so
> it won't use migration_is_active() and migration_is_device(). Instead,
> use internal VFIO dirty tracking flags.
> 
> As a side effect, now that migration status is no longer used to detect
> dirty tracking status, VFIO log syncs are untied from migration. This
> will make calc-dirty-rate more accurate as now it will also include VFIO
> dirty pages.
> 
> While at it, as VFIODevice->dirty_tracking is now used to detect dirty
> tracking status, add a comment that states how it's protected.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  include/hw/vfio/vfio-common.h |  2 +-
>  hw/vfio/common.c              | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index e0ce6ec3a9..6c999be398 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -143,7 +143,7 @@ typedef struct VFIODevice {
>      OnOffAuto pre_copy_dirty_page_tracking;
>      OnOffAuto device_dirty_page_tracking;
>      bool dirty_pages_supported;
> -    bool dirty_tracking;
> +    bool dirty_tracking; /* Protected by BQL */
>      bool iommu_dirty_tracking;
>      HostIOMMUDevice *hiod;
>      int devid;
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index dcef44fe55..e032ce1b6f 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -170,11 +170,26 @@ bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
>             migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
>  }
>  
> +static bool vfio_devices_all_device_dirty_tracking_started(
> +    const VFIOContainerBase *bcontainer)
> +{
> +    VFIODevice *vbasedev;
> +
> +    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
> +        if (!vbasedev->dirty_tracking) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
>  {
>      VFIODevice *vbasedev;
>  
> -    if (!migration_is_active() && !migration_is_device()) {
> +    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
> +          bcontainer->dirty_pages_started)) {
>          return false;
>      }
>
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index e0ce6ec3a9..6c999be398 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -143,7 +143,7 @@  typedef struct VFIODevice {
     OnOffAuto pre_copy_dirty_page_tracking;
     OnOffAuto device_dirty_page_tracking;
     bool dirty_pages_supported;
-    bool dirty_tracking;
+    bool dirty_tracking; /* Protected by BQL */
     bool iommu_dirty_tracking;
     HostIOMMUDevice *hiod;
     int devid;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index dcef44fe55..e032ce1b6f 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -170,11 +170,26 @@  bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
            migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
 }
 
+static bool vfio_devices_all_device_dirty_tracking_started(
+    const VFIOContainerBase *bcontainer)
+{
+    VFIODevice *vbasedev;
+
+    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
+        if (!vbasedev->dirty_tracking) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
-    if (!migration_is_active() && !migration_is_device()) {
+    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+          bcontainer->dirty_pages_started)) {
         return false;
     }