On Thu, Jul 07, 2022 at 04:59:22PM -0300, Leonardo Bras Soares Passos wrote:
> Hello Peter,
>
> On Thu, Jul 7, 2022 at 2:56 PM Peter Xu <pet...@redhat.com> wrote:
> >
> > On Mon, Jul 04, 2022 at 05:23:15PM -0300, Leonardo Bras wrote:
> > > Some errors, like the lack of Scatter-Gather support by the network
> > > interface(NETIF_F_SG) may cause sendmsg(...,MSG_ZEROCOPY) to fail on using
> > > zero-copy, which causes it to fall back to the default copying mechanism.
> > >
> > > After each full dirty-bitmap scan there should be a zero-copy flush
> > > happening, which checks for errors each of the previous calls to
> > > sendmsg(...,MSG_ZEROCOPY). If all of them failed to use zero-copy, then
> > > increment dirty_sync_missed_zero_copy migration stat to let the user know
> > > about it.
> > >
> > > Signed-off-by: Leonardo Bras <leob...@redhat.com>
> > > ---
> > > migration/ram.h | 2 ++
> > > migration/multifd.c | 2 ++
> > > migration/ram.c | 5 +++++
> > > 3 files changed, 9 insertions(+)
> > >
> > > diff --git a/migration/ram.h b/migration/ram.h
> > > index ded0a3a086..d3c7eb96f5 100644
> > > --- a/migration/ram.h
> > > +++ b/migration/ram.h
> > > @@ -87,4 +87,6 @@ void ram_write_tracking_prepare(void);
> > > int ram_write_tracking_start(void);
> > > void ram_write_tracking_stop(void);
> > >
> > > +void dirty_sync_missed_zero_copy(void);
> > > +
> > > #endif
> > > diff --git a/migration/multifd.c b/migration/multifd.c
> > > index 684c014c86..3909b34967 100644
> > > --- a/migration/multifd.c
> > > +++ b/migration/multifd.c
> > > @@ -624,6 +624,8 @@ int multifd_send_sync_main(QEMUFile *f)
> > > if (ret < 0) {
> > > error_report_err(err);
> > > return -1;
> > > + } else if (ret == 1) {
> > > + dirty_sync_missed_zero_copy();
> > > }
> > > }
> > > }
> >
> > I know that Juan is working on some patch to only do
> > multifd_send_sync_main() for each dirty sync, but that's not landed, right?
>
> That's correct, but I am hoping it should land before the release, so
> the numbers will match.
>
>
> >
> > Can we name it without "dirty-sync" at all (so it'll work before/after
> > Juan's patch will be applied)? Something like "zero-copy-send-fallbacks"?
>
> It initially was something like that, but on the v2 thread there was
> some discussion on
> the topic, and it was suggested the number would not mean much to the
> user, unless
> it was connected to something else.
>
> Markus suggested the connection to @dirty-sync-count right in the
> name, and Daniel suggested the above name, which sounds fine to me.
Ah okay.
But then I suggest we make sure it lands only after Juan's.. or it won't
really match. Also when Juan's patch ready, we'd also double check it will
be exactly called once per iteration, or we can get confusing numbers. I
assume Juan will take care of that then.
>
> >
> > The other thing is the subject may need to be touched up as right now with
> > the field we don't warn the user anymore on zero-copy-send fallbacks.
>
> Ok, Warning sounds misleading here.
> What do you think about 'report' instead?
Looks good. Thanks,
--
Peter Xu
