Re: [PATCH v4 22/33] vfio/migration: Convert bytes_transferred counter to atomic

[Maciej S. Szmigiero]

On 30.01.2025 22:35, Cédric Le Goater wrote:

On 1/30/25 11:08, Maciej S. Szmigiero wrote:

From: "Maciej S. Szmigiero" 

So it can be safety accessed from multiple threads.

Signed-off-by: Maciej S. Szmigiero 
---
  hw/vfio/migration.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index f5df5ef17080..cbb1e0b6f852 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -416,7 +416,7 @@ static ssize_t vfio_save_block(QEMUFile *f, VFIOMigration 
*migration)
  qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
  qemu_put_be64(f, data_size);
  qemu_put_buffer(f, migration->data_buffer, data_size);
-    bytes_transferred += data_size;
+    qatomic_add(&bytes_transferred, data_size);


bytes_transferred should of type : aligned_uint64_t


  trace_vfio_save_block(migration->vbasedev->name, data_size);
@@ -1038,12 +1038,12 @@ static int vfio_block_migration(VFIODevice *vbasedev, 
Error *err, Error **errp)
  int64_t vfio_mig_bytes_transferred(void)
  {
-    return bytes_transferred;
+    return qatomic_read(&bytes_transferred);


please use qatomic_read_u64()


  }
  void vfio_reset_bytes_transferred(void)
  {
-    bytes_transferred = 0;
+    qatomic_set(&bytes_transferred, 0);


and qatomic_set_u64().



Unfortunately, using aligned_uint64_t does not work since there's
no qatomic_add_u64().

I think best fix here is to use an "unsigned long" - it's 64 bit on
64-bit Unix systems and 32-bit on 32-bit ones.

On the other hand it's 32-bit on 64-bit Windows but Windows does not
use VFIO anyway.


No need to resend for that (yet) but it might explain the test issues.

Thanks,

C.


Thanks,
Maciej

Re: [PATCH v4 22/33] vfio/migration: Convert bytes_transferred counter to atomic

[Cédric Le Goater]

On 1/30/25 11:08, Maciej S. Szmigiero wrote:

From: "Maciej S. Szmigiero" 

So it can be safety accessed from multiple threads.

Signed-off-by: Maciej S. Szmigiero 
---
  hw/vfio/migration.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index f5df5ef17080..cbb1e0b6f852 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -416,7 +416,7 @@ static ssize_t vfio_save_block(QEMUFile *f, VFIOMigration 
*migration)
  qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
  qemu_put_be64(f, data_size);
  qemu_put_buffer(f, migration->data_buffer, data_size);
-bytes_transferred += data_size;
+qatomic_add(&bytes_transferred, data_size);


bytes_transferred should of type : aligned_uint64_t

  
  trace_vfio_save_block(migration->vbasedev->name, data_size);
  
@@ -1038,12 +1038,12 @@ static int vfio_block_migration(VFIODevice *vbasedev, Error *err, Error **errp)
  
  int64_t vfio_mig_bytes_transferred(void)

  {
-return bytes_transferred;
+return qatomic_read(&bytes_transferred);


please use qatomic_read_u64()


  }
  
  void vfio_reset_bytes_transferred(void)

  {
-bytes_transferred = 0;
+qatomic_set(&bytes_transferred, 0);


and qatomic_set_u64().

No need to resend for that (yet) but it might explain the test issues.

Thanks,

C.



  }
  
  /*

[PATCH v4 22/33] vfio/migration: Convert bytes_transferred counter to atomic

[Maciej S. Szmigiero]

From: "Maciej S. Szmigiero" 

So it can be safety accessed from multiple threads.

Signed-off-by: Maciej S. Szmigiero 
---
 hw/vfio/migration.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index f5df5ef17080..cbb1e0b6f852 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -416,7 +416,7 @@ static ssize_t vfio_save_block(QEMUFile *f, VFIOMigration 
*migration)
 qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
 qemu_put_be64(f, data_size);
 qemu_put_buffer(f, migration->data_buffer, data_size);
-bytes_transferred += data_size;
+qatomic_add(&bytes_transferred, data_size);
 
 trace_vfio_save_block(migration->vbasedev->name, data_size);
 
@@ -1038,12 +1038,12 @@ static int vfio_block_migration(VFIODevice *vbasedev, 
Error *err, Error **errp)
 
 int64_t vfio_mig_bytes_transferred(void)
 {
-return bytes_transferred;
+return qatomic_read(&bytes_transferred);
 }
 
 void vfio_reset_bytes_transferred(void)
 {
-bytes_transferred = 0;
+qatomic_set(&bytes_transferred, 0);
 }
 
 /*