Re: [Qemu-devel] [PATCH v12 15/21] migration: Add block where to send/receive packets

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> Once there add tracepoints.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/ram.c        | 49 +++++++++++++++++++++++++++++++++++++-----
>  migration/trace-events |  2 ++
>  2 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 21b448c4ed..c4c185cc4c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -455,6 +455,8 @@ typedef struct {
>      bool running;
>      /* should this thread finish */
>      bool quit;
> +    /* thread has work to do */
> +    int pending_job;
>      /* array of pages to sent */
>      MultiFDPages_t *pages;
>      /* packet allocated len */
> @@ -489,6 +491,8 @@ typedef struct {
>      bool running;
>      /* should this thread finish */
>      bool quit;
> +    /* thread has work to do */
> +    bool pending_job;
>      /* array of pages to receive */
>      MultiFDPages_t *pages;
>      /* packet allocated len */
> @@ -756,8 +760,28 @@ static void *multifd_send_thread(void *opaque)
>      while (true) {
>          qemu_sem_wait(&p->sem);
>          qemu_mutex_lock(&p->mutex);
> -        multifd_send_fill_packet(p);
> -        if (p->quit) {
> +
> +        if (p->pending_job) {
> +            uint32_t used = p->pages->used;
> +            uint32_t seq = p->seq;
> +            uint32_t flags = p->flags;
> +
> +            multifd_send_fill_packet(p);
> +            p->flags = 0;
> +            p->num_packets++;
> +            p->num_pages += used;
> +            p->pages->used = 0;
> +            qemu_mutex_unlock(&p->mutex);
> +
> +            trace_multifd_send(p->id, seq, used, flags);
> +
> +            /* ToDo: send packet here */
> +
> +            qemu_mutex_lock(&p->mutex);
> +            p->pending_job--;
> +            qemu_mutex_unlock(&p->mutex);
> +            continue;

OK, but this continue is an artifact of the warning about no command to
do that you later remove, so you could merge this down, but other than
that:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +        } else if (p->quit) {
>              qemu_mutex_unlock(&p->mutex);
>              break;
>          }
> @@ -823,6 +847,7 @@ int multifd_save_setup(void)
>          qemu_mutex_init(&p->mutex);
>          qemu_sem_init(&p->sem, 0);
>          p->quit = false;
> +        p->pending_job = 0;
>          p->id = i;
>          multifd_pages_init(&p->pages, page_count);
>          p->packet_len = sizeof(MultiFDPacket_t)
> @@ -910,14 +935,27 @@ static void *multifd_recv_thread(void *opaque)
>      while (true) {
>          qemu_sem_wait(&p->sem);
>          qemu_mutex_lock(&p->mutex);
> -        if (false)  {
> -            /* ToDo: Packet reception goes here */
> +        if (p->pending_job) {
> +            uint32_t used;
> +            uint32_t flags;
> +            qemu_mutex_unlock(&p->mutex);
>  
> +            /* ToDo: recv packet here */
> +
> +            qemu_mutex_lock(&p->mutex);
>              ret = multifd_recv_unfill_packet(p, &local_err);
> -            qemu_mutex_unlock(&p->mutex);
>              if (ret) {
> +                qemu_mutex_unlock(&p->mutex);
>                  break;
>              }
> +
> +            used = p->pages->used;
> +            flags = p->flags;
> +            trace_multifd_recv(p->id, p->seq, used, flags);
> +            p->pending_job = false;
> +            p->num_packets++;
> +            p->num_pages += used;
> +            qemu_mutex_unlock(&p->mutex);
>          } else if (p->quit) {
>              qemu_mutex_unlock(&p->mutex);
>              break;
> @@ -960,6 +998,7 @@ int multifd_load_setup(void)
>          qemu_mutex_init(&p->mutex);
>          qemu_sem_init(&p->sem, 0);
>          p->quit = false;
> +        p->pending_job = false;
>          p->id = i;
>          multifd_pages_init(&p->pages, page_count);
>          p->packet_len = sizeof(MultiFDPacket_t)
> diff --git a/migration/trace-events b/migration/trace-events
> index e480eb050e..9eee048287 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -81,6 +81,8 @@ multifd_send_thread_start(uint8_t id) "%d"
>  multifd_send_thread_end(uint8_t id, uint32_t packets, uint32_t pages) "channel %d packets %d pages %d"
>  multifd_recv_thread_start(uint8_t id) "%d"
>  multifd_recv_thread_end(uint8_t id, uint32_t packets, uint32_t pages) "channel %d packets %d pages %d"
> +multifd_send(uint8_t id, uint32_t seq, uint32_t used, uint32_t flags) "channel %d seq number %d pages %d flags 0x%x"
> +multifd_recv(uint8_t id, uint32_t seq, uint32_t used, uint32_t flags) "channel %d seq number %d pages %d flags 0x%x"
>  
>  # migration/migration.c
>  await_return_path_close_on_source_close(void) ""
> -- 
> 2.17.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK