Re: [PATCH RFC 03/12] vfio/migration: Throttle vfio_save_block() on data size to read

[Avihai Horon <avihaih@nvidia.com>]

On 4/1/2026 11:36 PM, Peter Xu wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, Mar 25, 2026 at 04:10:14PM +0200, Avihai Horon wrote:
>> Hi Peter,
> Avihai,
>
>> Thanks for sending this series.
> Thanks for taking a look.
>
>> On 3/20/2026 1:12 AM, Peter Xu wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> During precopy phase, VFIO maintains two counters for init/dirty data
>>> tracking for query estimations.
>>>
>>> VFIO fetches data during precopy by reading from the VFIO fd, after
>>> fetching it'll deduct the read size.
>>>
>>> Here since the fd's size can dynamically change, I think it means VFIO may
>>> read more than what it "thought" were there for fetching.
>>>
>>> I highly suspect it's also relevant to a weird case in the function of
>>> vfio_update_estimated_pending_data(), where when VFIO reads 0 from the FD
>>> it will _reset_ the two counters, instead of asserting both of them being
>>> zeros, which looks pretty hackish.
>>>
>>> Just guarantee it from userspace level that VFIO won't read more than what
>>> it expects for now.
>> The VFIO_MIG_GET_PRECOPY_INFO ioctl returns an estimation of the data size
>> currently available for reading. So, even if the ioctl returns X bytes, it
>> may be that there are more than X bytes to read or less than X bytes.
>> The code was written in a flexible way to handle such discrepancies.
>>
>> Because we are dealing with an estimation, I don't think we can assert that
>> the counters are zero, and I don't think reading only up to the cached size
>> gives us any benefit:
>> If the estimation is lower than actual available data, we are just deferring
>> sending of the remaining data to a later stage.
> Since we'll introduce cached size, having the read() only happen with the
> size reported still makes sense to me.
>
> We're not deferring to later that much, when dirty data reaches zero, we'll
> re-sync with everything including VFIO's VFIO_MIG_GET_PRECOPY_INFO.  So
> it's just splitting one last-phase read() into two smaller read()s.  To me,
> it sounds still OK if with that we can make sure the counter won't overflow.

The counter doesn't overflow today as well (thanks to the MIN calculation).

>
>> If the estimation is higher than actual available data, we may still read()
>> zero when the cached values are not zero.
>>
>> I think we should keep the code as is.
>>
>> Does that make sense?
> I can understand what got reported in VFIO_MIG_GET_PRECOPY_INFO may not be
> the total size of dirty data, but what the userapp can read.  That part is
> fine.
>
> Now, do you mean the size reported could shrink as well?

Yes.

>    Could you explain
> why, and when, dirtied data size can shrink?

First of all, the sizes of VFIO_MIG_GET_PRECOPY_INFO are defined as an 
estimation, so anything can be reported there.

But specifically for mlx5, VFIO_MIG_GET_PRECOPY_INFO includes two steps 
- query and save: first, the driver queries the device for the 
*expected* amount of data (that's the returned init/dirty sizes) and 
then the driver async-ly saves the data.
The query returns the *expected* amount of data, but the actual data 
returned in the save may be smaller.

Regarding VFIO_DEVICE_FEATURE_MIG_DATA_SIZE , I don't have a concrete 
example of data "shrink".
But I can think of a device that all its data is precopy-able and 
VFIO_DEVICE_FEATURE_MIG_DATA_SIZE may return X data remaining while 
actual data is smaller than X, simply because the uAPI defines that it's 
an estimate.

What I am trying to say is that VFIO_MIG_GET_PRECOPY_INFO and 
VFIO_DEVICE_FEATURE_MIG_DATA_SIZE sizes are estimates which are good 
enough for telling QEMU how much data is remaining, but shouldn't be 
used to make precise calculations on how much to read(). At least I 
don't see the benefit from it.

Thanks.