Re: [PATCH v3 7/7] introduce simple linear scan rate limiting mechanism

[Peter Xu]

On Fri, Nov 20, 2020 at 03:06:56PM +0300, Andrey Gruzdev wrote:
> On 19.11.2020 23:02, Peter Xu wrote:
> > On Thu, Nov 19, 2020 at 03:59:40PM +0300, Andrey Gruzdev wrote:
> > > Since reading UFFD events and saving paged data are performed
> > > from the same thread, write fault latencies are sensitive to
> > > migration stream stalls. Limiting total page saving rate is a
> > > method to reduce amount of noticiable fault resolution latencies.
> > > 
> > > Migration bandwidth limiting is achieved via noticing cases of
> > > out-of-threshold write fault latencies and temporarily disabling
> > > (strictly speaking, severely throttling) saving non-faulting pages.
> > 
> > Just curious: have you measured aver/max latency of wr-protected page 
> > requests,
> > or better, even its distribution?
> > 
> > I believe it should also be relevant to where the snapshot is stored, say, 
> > the
> > backend disk of your tests.  Is that a file on some fs?
> > 
> > I would expect the latency should be still good if e.g. the throughput of 
> > the
> > backend file system is decent even without a patch like this, but I might 
> > have
> > missed something..
> > 
> > In all cases, it would be very nice if this patch can have the histogram or
> > aver or max latency measured and compared before/after this patch applied.
> > 
> > Thanks,
> > 
> So far I have no objective latency measurements, that really interesting.
> For testing I commonly used SATA HDD, not too fresh one, 1.5TB Seagate
> 7200.11 series, specially not to have large hardware cache or flash buffer.
> And yes, backend is a file on ext4.
> 
> I tried mostly with 'migrate exec:streamdump_utility', a very simple tool
> which writes stream to a file. It even doesn't use AIO - so reads from STDIN
> and file writes don't overlap. Made so intentionally to reduce throughput
> and give some random high-latency writes.
> 
> I think snapshotting performance may be severely degraded by I/O happening
> in parallel on the same storage media, that's the problem we need to
> consider.

Yeah this is a good point.  So ideally I think here we have similar requirement
with postcopy in that we really want the postcopy request stream to be separate
from the main migration stream, just like what we'd probably prefer here too
when vcpus writting to wr-protected pages, rather than missing ones.  So that
the urgent requests got handled faster than the background task.

For postcopy, we may still have some chance if someday we can separate the
migration channels, so that we can try to provide a standalone channel for
postcopy requests.  For example if tcp based, that can be a separate socket.
Then we can try to tune the priority of these sockets to make sure postcopy
stream were sent earlier.

So I guess it's not always easy to seperate these two channels - in your case
if the disk must be shared, seems not easy.  Maybe we can prioritize disk I/Os
somehow too just like tcp channels?  Not sure.  Hmm, it would be good if
QEMUFile could provide some mechanism to prioritize IOs as a common interface,
but that's probably too far beyond our discussion. :)

> 
> And yes, to take latency histogram before/after the patch is nice idea!
> Also I need to make stream dumping tool with AIO, to test with full storage
> throughput.

Yeah I would appreciate if you can provide those.

Note that I think it can even be some work after the functionality got merged
(Dave/Juan could correct me), because it can be some work on top.  Having them
together would be even nicer, of course.  Your call.  It's already much better
than stopping the whole vm, imho.

Thanks,

-- 
Peter Xu