On Thu, Oct 19, 2023 at 11:58:13AM -0300, Fabiano Rosas wrote:
> Juan Quintela <quint...@redhat.com> writes:
>
> > Fabiano Rosas <faro...@suse.de> wrote:
> >> We don't need to check p->quit in the multifd_send_thread() because it
> >> is shadowed by the 'exiting' flag. Ever since that flag was added
> >> p->quit became obsolete as a way to stop the thread.
> >>
> >> Since p->quit is set at multifd_send_terminate_threads() under the
> >> p->mutex lock, the thread will only see it once it loops, so 'exiting'
> >> will always be seen first.
> >>
> >> Note that setting p->quit at multifd_send_terminate_threads() still
> >> makes sense because we need a way to inform multifd_send_pages() that
> >> the channel has stopped.
> >>
> >> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> >
> > Reviewed-by: Juan Quintela <quint...@redhat.com>
> >
> > But then should we remove the quit altogether?
> >
>
> It still serves a purpose to allow multifd_send_pages() to see that the
> channel has exited. While that function does also check
> multifd_send_state->exiting, it could already be waiting at the mutex
> when the channel aborts. So we need to either check 'exiting' again or
> keep p->quit.
Sounds better to just move multifd_send_state->exiting check into the loop,
then drop the per-channel ->quit?
diff --git a/migration/multifd.c b/migration/multifd.c
index 1fe53d3b98..381a9a14e4 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -399,10 +399,6 @@ static int multifd_send_pages(QEMUFile *f)
MultiFDSendParams *p = NULL; /* make happy gcc */
MultiFDPages_t *pages = multifd_send_state->pages;
- if (qatomic_read(&multifd_send_state->exiting)) {
- return -1;
- }
-
qemu_sem_wait(&multifd_send_state->channels_ready);
/*
* next_channel can remain from a previous migration that was
@@ -413,12 +409,11 @@ static int multifd_send_pages(QEMUFile *f)
for (i = next_channel;; i = (i + 1) % migrate_multifd_channels()) {
p = &multifd_send_state->params[i];
- qemu_mutex_lock(&p->mutex);
- if (p->quit) {
- error_report("%s: channel %d has already quit!", __func__, i);
- qemu_mutex_unlock(&p->mutex);
+ if (qatomic_read(&multifd_send_state->exiting)) {
return -1;
}
+
+ qemu_mutex_lock(&p->mutex);
if (!p->pending_job) {
p->pending_job++;
next_channel = (i + 1) % migrate_multifd_channels();
Thanks,
--
Peter Xu
