Re: [PATCH v2 2/3] Add zero-copy-copied migration stat

[Daniel P . Berrangé]

On Mon, Jul 04, 2022 at 02:04:41PM +0200, Markus Armbruster wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> >> On Mon, Jul 04, 2022 at 08:18:54AM +0200, Markus Armbruster wrote:
> >> > Leonardo Bras <leobras@redhat.com> writes:
> >> > 
> >> > > Signed-off-by: Leonardo Bras <leobras@redhat.com>
> >> > > ---
> >> > >  qapi/migration.json   | 5 ++++-
> >> > >  migration/migration.c | 1 +
> >> > >  monitor/hmp-cmds.c    | 4 ++++
> >> > >  3 files changed, 9 insertions(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/qapi/migration.json b/qapi/migration.json
> >> > > index 7102e474a6..925f009868 100644
> >> > > --- a/qapi/migration.json
> >> > > +++ b/qapi/migration.json
> >> > > @@ -55,6 +55,9 @@
> >> > >  # @postcopy-bytes: The number of bytes sent during the post-copy phase
> >> > >  #                  (since 7.0).
> >> > >  #
> >> > > +# @zero-copy-copied: The number of zero-copy flushes that reported 
> >> > > data sent
> >> > > +#                    using zero-copy that ended up being copied. 
> >> > > (since 7.2)
> 
> since 7.1, unless you're planning for really tortuous review :)
> 
> >> > 
> >> > The description feels awkward.  What's a "zero-copy flush", and why
> >> > should the user care?  I figure what users care about is the number of
> >> > all-zero pages we had to "copy", i.e. send the bulky way.  Is this what
> >> > @zero-copy-copied reports?
> >> 
> >> MigrationCapability field @zero-copy-send instructs QEMU to try to
> >> avoid copying data between userspace and kernel space when transmitting
> >> RAM region.
> >> 
> >> Even if the kernel supports zero copy, it is not guaranteed to happen,
> >> it is merely a request to try.
> >> 
> >> QEMU periodically (once per migration iteration) flushes outstanding
> >> zero-copy requests and gets an indication back of whether any copies
> >> took place or not.
> >> 
> >> So this counter is a reflection of how many iterations resulted  in
> >> zero-copy not being fully honoured.
> >> 
> >> IOW, ideally this counter will always be zero. If it is non-zero,
> >> then the magnitude gives a very very very rough guide to what's
> >> going on. If it is '1' then it was just a transient limitation.
> >> If it matches the number of migration iterations, then it is a
> >> more systemic limitation.
> >> 
> >> Incidentally, do we report the migration iteration count ? I
> >> thought we did, but i'm not finding it now that I look.
> >
> > Yes we do; it's dirty-sync-count
> 
> Please rephrase the documentation of @zero-copy-copied in terms of
> @dirty-sync-count.  Here's my attempt.
> 
> # @zero-copy-copied: Number of times dirty RAM synchronization could not
> #                    avoid copying zero pages.  This is between 0 and
> #                    @dirty-sync-count.  (since 7.1)

Any one have preferences on the name - i get slight put off by the
repeated word in the property name here.

   @zero-copy-rejects ?
   @zero-copy-misses ?
   @zero-copy-fails ?



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|