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

[Peter Xu <peterx@redhat.com>]

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.

> 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?  Could you explain
why, and when, dirtied data size can shrink?

Thanks,

-- 
Peter Xu