Re: [PATCH 2/2] multifd: cleanup the function multifd_send_thread

[Juan Quintela <quintela@redhat.com>]

Li Zhang <lizhang@suse.de> wrote:
> Cleanup multifd_send_thread
>
> Signed-off-by: Li Zhang <lizhang@suse.de>

Hi Zhang

First of all, sorry for the late review.

This other patch is wrong.

> ---
>  migration/multifd.c | 82 ++++++++++++++++++++++-----------------------
>  1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 4ec40739e0..7888d71bfe 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -649,58 +649,58 @@ static void *multifd_send_thread(void *opaque)
>              break;
>          }
>          qemu_mutex_lock(&p->mutex);
> -
> -        if (p->pending_job) {
> -            uint32_t used = p->pages->num;
> -            uint64_t packet_num = p->packet_num;
> -            uint32_t flags = p->flags;
> -
> -            if (used) {
> -                ret = multifd_send_state->ops->send_prepare(p, &local_err);
> -                if (ret != 0) {
> -                    qemu_mutex_unlock(&p->mutex);
> -                    break;
> -                }
> -            }
> -            multifd_send_fill_packet(p);
> -            p->flags = 0;
> -            p->num_packets++;
> -            p->num_pages += used;
> -            p->pages->num = 0;
> -            p->pages->block = NULL;
> +        if (!p->quit && !p->pending_job) {
> +            /* sometimes there are spurious wakeups */
> +            qemu_mutex_unlock(&p->mutex);
> +            continue;
> +        } else if (!p->pending_job) {

Here it should be
        } else if (p->quit) {

And in this case, I preffer the previous code, as the first case is the
common one.

I still have to see if we ever enter through the spurious case anymore.

Thanks, Juan.