diff mbox series

[v12,12/21] migration: Add multifd traces for start/end thread

Message ID 20180425112723.1111-13-quintela@redhat.com
State New
Headers show
Series Multifd | expand

Commit Message

Juan Quintela April 25, 2018, 11:27 a.m. UTC
We want to know how many pages/packets each channel has sent.  Add
counters for those.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c        | 20 ++++++++++++++++++++
 migration/trace-events |  4 ++++
 2 files changed, 24 insertions(+)

Comments

Dr. David Alan Gilbert May 2, 2018, 6:35 p.m. UTC | #1
* Juan Quintela (quintela@redhat.com) wrote:
> We want to know how many pages/packets each channel has sent.  Add
> counters for those.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/ram.c        | 20 ++++++++++++++++++++
>  migration/trace-events |  4 ++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 804c83ed89..0f1340b4e3 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -465,6 +465,10 @@ typedef struct {
>      uint32_t flags;
>      /* global number of generated multifd packets */
>      uint32_t seq;
> +    /* packets sent through this channel */
> +    uint32_t num_packets;
> +    /* pages sent through this channel */
> +    uint32_t num_pages;
>  }  MultiFDSendParams;
>  
>  typedef struct {
> @@ -495,6 +499,10 @@ typedef struct {
>      uint32_t flags;
>      /* global number of generated multifd packets */
>      uint32_t seq;
> +    /* packets sent through this channel */
> +    uint32_t num_packets;
> +    /* pages sent through this channel */
> +    uint32_t num_pages;

Doesn't the comment for 'mutex' say 'protects the following parameters'
(added in 09/21) ?  In which case....

>  } MultiFDRecvParams;
>  
>  static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
> @@ -737,9 +745,13 @@ static void *multifd_send_thread(void *opaque)
>      MultiFDSendParams *p = opaque;
>      Error *local_err = NULL;
>  
> +    trace_multifd_send_thread_start(p->id);
> +
>      if (multifd_send_initial_packet(p, &local_err) < 0) {
>          goto out;
>      }
> +    /* initial packet */
> +    p->num_packets = 1;

That's writing to num_packets before the mutex is taken.

>  
>      while (true) {
>          qemu_mutex_lock(&p->mutex);
> @@ -761,6 +773,8 @@ out:
>      p->running = false;
>      qemu_mutex_unlock(&p->mutex);
>  
> +    trace_multifd_send_thread_end(p->id, p->num_packets, p->num_pages);
> +

and should this happen before they're released (OK, it's only a trace)

>      return NULL;
>  }
>  
> @@ -888,6 +902,8 @@ static void *multifd_recv_thread(void *opaque)
>      Error *local_err = NULL;
>      int ret;
>  
> +    trace_multifd_recv_thread_start(p->id);
> +
>      while (true) {
>          qemu_mutex_lock(&p->mutex);
>          if (false)  {
> @@ -910,6 +926,8 @@ static void *multifd_recv_thread(void *opaque)
>      p->running = false;
>      qemu_mutex_unlock(&p->mutex);
>  
> +    trace_multifd_recv_thread_end(p->id, p->num_packets, p->num_pages);
> +
>      return NULL;
>  }
>  
> @@ -975,6 +993,8 @@ void multifd_recv_new_channel(QIOChannel *ioc)
>      }
>      p->c = ioc;
>      object_ref(OBJECT(ioc));
> +    /* initial packet */
> +    p->num_packets = 1;

OK, the receive side here is different.

>      p->running = true;
>      qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
> diff --git a/migration/trace-events b/migration/trace-events
> index a180d7b008..e480eb050e 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -77,6 +77,10 @@ ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 " %x"
>  ram_postcopy_send_discard_bitmap(void) ""
>  ram_save_page(const char *rbname, uint64_t offset, void *host) "%s: offset: 0x%" PRIx64 " host: %p"
>  ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 0x%zx len: 0x%zx"
> +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"

Please put the m's before the r's

Dave

>  # migration/migration.c
>  await_return_path_close_on_source_close(void) ""
> -- 
> 2.17.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela May 9, 2018, 11:11 a.m. UTC | #2
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We want to know how many pages/packets each channel has sent.  Add
>> counters for those.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/ram.c        | 20 ++++++++++++++++++++
>>  migration/trace-events |  4 ++++
>>  2 files changed, 24 insertions(+)

>>  typedef struct {
>> @@ -495,6 +499,10 @@ typedef struct {
>>      uint32_t flags;
>>      /* global number of generated multifd packets */
>>      uint32_t seq;
>> +    /* packets sent through this channel */
>> +    uint32_t num_packets;
>> +    /* pages sent through this channel */
>> +    uint32_t num_pages;
>
> Doesn't the comment for 'mutex' say 'protects the following parameters'
> (added in 09/21) ?  In which case....

This two are really local.  Fixing comment.

Thanks, Juan.
diff mbox series

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 804c83ed89..0f1340b4e3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -465,6 +465,10 @@  typedef struct {
     uint32_t flags;
     /* global number of generated multifd packets */
     uint32_t seq;
+    /* packets sent through this channel */
+    uint32_t num_packets;
+    /* pages sent through this channel */
+    uint32_t num_pages;
 }  MultiFDSendParams;
 
 typedef struct {
@@ -495,6 +499,10 @@  typedef struct {
     uint32_t flags;
     /* global number of generated multifd packets */
     uint32_t seq;
+    /* packets sent through this channel */
+    uint32_t num_packets;
+    /* pages sent through this channel */
+    uint32_t num_pages;
 } MultiFDRecvParams;
 
 static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
@@ -737,9 +745,13 @@  static void *multifd_send_thread(void *opaque)
     MultiFDSendParams *p = opaque;
     Error *local_err = NULL;
 
+    trace_multifd_send_thread_start(p->id);
+
     if (multifd_send_initial_packet(p, &local_err) < 0) {
         goto out;
     }
+    /* initial packet */
+    p->num_packets = 1;
 
     while (true) {
         qemu_mutex_lock(&p->mutex);
@@ -761,6 +773,8 @@  out:
     p->running = false;
     qemu_mutex_unlock(&p->mutex);
 
+    trace_multifd_send_thread_end(p->id, p->num_packets, p->num_pages);
+
     return NULL;
 }
 
@@ -888,6 +902,8 @@  static void *multifd_recv_thread(void *opaque)
     Error *local_err = NULL;
     int ret;
 
+    trace_multifd_recv_thread_start(p->id);
+
     while (true) {
         qemu_mutex_lock(&p->mutex);
         if (false)  {
@@ -910,6 +926,8 @@  static void *multifd_recv_thread(void *opaque)
     p->running = false;
     qemu_mutex_unlock(&p->mutex);
 
+    trace_multifd_recv_thread_end(p->id, p->num_packets, p->num_pages);
+
     return NULL;
 }
 
@@ -975,6 +993,8 @@  void multifd_recv_new_channel(QIOChannel *ioc)
     }
     p->c = ioc;
     object_ref(OBJECT(ioc));
+    /* initial packet */
+    p->num_packets = 1;
 
     p->running = true;
     qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
diff --git a/migration/trace-events b/migration/trace-events
index a180d7b008..e480eb050e 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -77,6 +77,10 @@  ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 " %x"
 ram_postcopy_send_discard_bitmap(void) ""
 ram_save_page(const char *rbname, uint64_t offset, void *host) "%s: offset: 0x%" PRIx64 " host: %p"
 ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 0x%zx len: 0x%zx"
+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"
 
 # migration/migration.c
 await_return_path_close_on_source_close(void) ""