* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> > * Juan Quintela (quint...@redhat.com) wrote:
> >> Each time that we sync the bitmap, it is a possiblity that we receive
> >> a page that is being processed by a different thread. We fix this
> >> problem just making sure that we wait for all receiving threads to
> >> finish its work before we procedeed with the next stage.
> >>
> >> We are low on page flags, so we use a combination that is not valid to
> >> emit that message: MULTIFD_PAGE and COMPRESSED.
> >>
> >> I tried to make a migration command for it, but it don't work because
> >> we sync the bitmap sometimes when we have already sent the beggining
> >> of the section, so I just added a new page flag.
> >>
> >> Signed-off-by: Juan Quintela <quint...@redhat.com>
>
> >> +static int multifd_flush(void)
> >> +{
> >> + int i, thread_count;
> >> +
> >> + if (!migrate_use_multifd()) {
> >> + return 0;
> >> + }
> >> + thread_count = migrate_multifd_threads();
> >> + for (i = 0; i < thread_count; i++) {
> >> + MultiFDRecvParams *p = multifd_recv_state->params[i];
> >> +
> >> + qemu_mutex_lock(&p->mutex);
> >> + while (!p->done) {
> >> + p->sync = true;
> >> + qemu_cond_wait(&p->cond_sync, &p->mutex);
> >> + }
> >
> > I don't think I understand how that works in the case where the
> > recv_thread has already 'done' by the point you set sync=true; how does
> > it get back to the check and do the signal?
>
> We have two cases:
> * done = true
> * done = false
>
> if done is false, we need to wait until it is done. But if it is true,
> we don't have to wait. By definition, there is nothing on that thread
> that we need to wait for. It is not in the middle of receiving a page.
OK, and you've got the p->mutex, so done can't become true
between the check at the top of the while() and the p->sync = true
on the next line? OK.
Dave
>
>
> >
> >> + qemu_mutex_unlock(&p->mutex);
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> /**
> >> * save_page_header: write page header to wire
> >> *
> >> @@ -809,6 +847,12 @@ static size_t save_page_header(RAMState *rs, QEMUFile
> >> *f, RAMBlock *block,
> >> {
> >> size_t size, len;
> >>
> >> + if (rs->multifd_needs_flush &&
> >> + (offset & RAM_SAVE_FLAG_MULTIFD_PAGE)) {
> >> + offset |= RAM_SAVE_FLAG_ZERO;
> >
> > In the comment near the top you say RAM_SAVE_FLAG_COMPRESS_PAGE; it's
> > probably best to add an alias at the top to make it clear, e.g.
> > #define RAM_SAVE_FLAG_MULTIFD_SYNC RAM_SAVE_FLAG_ZERO
> >
> > or maybe (RAM_SAVE_FLAG_MULTIFD_PAGE | RAM_SAVE_FLAG_ZERO)
>
> done.
>
> But I only use it when we use the "or".
>
> Thanks, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
