Hello Peter, On Thu, Jan 13, 2022 at 4:15 AM Peter Xu <pet...@redhat.com> wrote: > > On Thu, Jan 06, 2022 at 07:13:42PM -0300, Leonardo Bras wrote: > > Implement zero copy on nocomp_send_write(), by making use of QIOChannel > > writev + flags & flush interface. > > > > Change multifd_send_sync_main() so it can distinguish each iteration sync > > from > > the setup and the completion, so a flush_zero_copy() can be called > > after each iteration in order to make sure all dirty pages are sent > > before a new iteration is started. > > Leo - could you remind me (since I remembered we've discussed something > similar) on why we can't simply do the sync() unconditionally for zero copy?
On previous implementations, it would get stuck on the setup, since it was waiting for any movement on the error queue before even starting the sending process. At the time we would sync only at 'complete', so it would only need to run once. Running every iteration seemed a waste at the time. Then, after some talk with Juan, it was decided to sync after each migration, so on 'complete' it was unnecessary. But sure, now it would add just 2 syncs in the whole migration, and those should not even get to the syscall due to queued/sent counters. > > I remember why we need the sync(), but I can't remember what's the matter if > we > simply sync() too during setup and complete of migration. > > Another trivial nit here: > > > -void multifd_send_sync_main(QEMUFile *f) > > +int multifd_send_sync_main(QEMUFile *f, bool sync) > > I'd name it "bool full" or anything not called "sync", because the function > already has a name that contains "sync", then it's werid to sync(sync==false). > Yeah, I agree. But if we will flush every time, then there is no need for such parameter :). > The rest looks good to me. Thanks. > Thanks! > -- > Peter Xu > Best regards, Leo
