Re: [PATCH RFC 08/12] vfio/migration: Fix incorrect reporting for VFIO pending data

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 25, 2026 at 07:32:12PM +0200, Avihai Horon wrote:
> 
> On 3/20/2026 1:12 AM, Peter Xu wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > VFIO used to report different things in its fast/slow version of query
> > pending results.  It was likely because it wants to make sure precopy data
> > can reach 0 hence trigger sync queries.
> 
> It was to guarantee precopy data can reach 0 and trigger dirty sync queries,
> since not all VFIO device data may be precopy-able.

Thanks for confirming.

> 
> > 
> > Now with stopcopy size reporting facility it doesn't need this hack
> > anymore.  Fix this.
> 
> Looks good, now the fast/slow path are consistent, plus we get the benefit
> that if VFIO device has much stopcopy data, migration will try to reduce
> RAM/other iterative devices to the minimum continuously instead of jumping
> from fast to slow path as it currently does.

Yep.

> 
> I still want to test this later with a few workloads, just to make sure we
> don't miss anything here.

Thanks a lot.  Let me know if there's any update.

> 
> > 
> > Copy stable might be too much; just skip it and skip the Fixes.
> > 
> > Cc: Avihai Horon <avihaih@nvidia.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   hw/vfio/migration.c | 11 +++++++----
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> > index c054c749b0..9dbe5ad9e9 100644
> > --- a/hw/vfio/migration.c
> > +++ b/hw/vfio/migration.c
> > @@ -591,6 +591,10 @@ static void vfio_state_pending_sync(VFIODevice *vbasedev)
> >                             __func__, migration->precopy_init_size,
> >                             migration->precopy_dirty_size,
> >                             migration->stopcopy_size);
> > +        migration->stopcopy_size = 0;
> > +    } else {
> > +        migration->stopcopy_size -=
> > +            (migration->precopy_init_size + migration->precopy_dirty_size);
> 
> Query of stopcopy and precopy are not atomic, so we better use MIN() here:
> 
> migration->stopcopy_size -= MIN(migration->stopcopy_size,
> migration->precopy_init_size + migration->precopy_dirty_size);

Yes.  I'll revisit all these places on atomicity and possible overflow
issues in this series after I get better understanding on the "data shrink"
possibilities.

> 
> >       }
> >   }
> > 
> > @@ -598,19 +602,18 @@ static void vfio_state_pending(void *opaque, MigPendingData *pending)
> >   {
> >       VFIODevice *vbasedev = opaque;
> >       VFIOMigration *migration = vbasedev->migration;
> > -    uint64_t remain;
> > 
> >       if (pending->fastpath) {
> >           if (!vfio_device_state_is_precopy(vbasedev)) {
> >               return;
> >           }
> > -        remain = migration->precopy_init_size + migration->precopy_dirty_size;
> >       } else {
> >           vfio_state_pending_sync(vbasedev);
> > -        remain = migration->stopcopy_size;
> >       }
> > 
> > -    pending->precopy_bytes += remain;
> > +    pending->precopy_bytes +=
> > +        migration->precopy_init_size + migration->precopy_dirty_size;
> > +    pending->stopcopy_bytes += migration->stopcopy_size;
> 
> Now that migration->stopcopy_size holds only the stopcopy size (and not
> stopcopy+precopy), we should remove these lines from
> vfio_update_estimated_pending_data():
> 
>     /* The total size remaining requires separate accounting */
>     migration->stopcopy_size -= data_size;

Good point, I missed this part when changing the definition, I'll fix.

Thanks,

> 
> Thanks.
> 
> > 
> >       trace_vfio_state_pending(vbasedev->name, migration->stopcopy_size,
> >                                migration->precopy_init_size,
> > --
> > 2.50.1
> > 
> 

-- 
Peter Xu