Fabiano Rosas <faro...@suse.de> wrote:
> We shouldn't really be touching channel state from outside the
> channels except for the transfer of pages.
>
> Move the setting of p->quit into the channel. Keep posting the 'sem'
> from multifd_send_terminate_threads() so any channel waiting on the
> semaphore will unblock and see the 'exiting' flag and quit by itself.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> migration/multifd.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 4ffa67339c..b7ba3fe0e6 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -497,13 +497,11 @@ static void multifd_send_terminate_threads(Error *err)
> for (i = 0; i < migrate_multifd_channels(); i++) {
> MultiFDSendParams *p = &multifd_send_state->params[i];
>
> - qemu_mutex_lock(&p->mutex);
> - p->quit = true;
> + /* kick the channel if it was waiting for work */
> qemu_sem_post(&p->sem);
> if (p->c) {
> qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> }
> - qemu_mutex_unlock(&p->mutex);
Can we do two qio_channel_* operations at the same time on different
threads? mutex is also protecting that. This is a question, I don't
know the answer.
> }
> }
>
> @@ -690,6 +688,9 @@ static void *multifd_send_thread(void *opaque)
> qemu_sem_wait(&p->sem);
>
> if (qatomic_read(&multifd_send_state->exiting)) {
> + qemu_mutex_lock(&p->mutex);
> + p->quit = true;
> + qemu_mutex_unlock(&p->mutex);
> break;
> }
> qemu_mutex_lock(&p->mutex);
> @@ -765,6 +766,11 @@ static void *multifd_send_thread(void *opaque)
If we are going this route, we can just put it here, just in one place.
> out:
> if (local_err) {
> trace_multifd_send_error(p->id);
> +
> + qemu_mutex_lock(&p->mutex);
> + p->quit = true;
> + qemu_mutex_unlock(&p->mutex);
> +
> multifd_send_terminate_threads(local_err);
> error_free(local_err);
> }
But now ->quit has basically the same meaning that ->running, so we
could probably merge both.
Later, Juan.
