diff mbox series

[v12,13/21] migration: Calculate transferred ram correctly

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

Commit Message

Juan Quintela April 25, 2018, 11:27 a.m. UTC
On multifd we send data from more places that main channel.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Dr. David Alan Gilbert May 2, 2018, 6:59 p.m. UTC | #1
* Juan Quintela (quintela@redhat.com) wrote:
> On multifd we send data from more places that main channel.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/migration.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 9b510a809a..75d30661e9 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2246,12 +2246,19 @@ static void migration_update_counters(MigrationState *s,
>  {
>      uint64_t transferred, time_spent;
>      double bandwidth;
> +    uint64_t now;
>  
>      if (current_time < s->iteration_start_time + BUFFER_DELAY) {
>          return;
>      }
>  
> -    transferred = qemu_ftell(s->to_dst_file) - s->iteration_initial_bytes;
> +    if (migrate_use_multifd()) {
> +        now = ram_counters.normal * qemu_target_page_size()
> +            + qemu_ftell(s->to_dst_file);

OK, can I just confirm, the 'multifd packets' go over the main fd, and
just the pags are going over the other fd's?  In which case this is
right; but if the headers are going over the other fd's as well then
this is wrong.

Dave

> +    } else {
> +        now = qemu_ftell(s->to_dst_file);
> +    }
> +    transferred = now - s->iteration_initial_bytes;
>      time_spent = current_time - s->iteration_start_time;
>      bandwidth = (double)transferred / time_spent;
>      s->threshold_size = bandwidth * s->parameters.downtime_limit;
> @@ -2271,7 +2278,7 @@ static void migration_update_counters(MigrationState *s,
>      qemu_file_reset_rate_limit(s->to_dst_file);
>  
>      s->iteration_start_time = current_time;
> -    s->iteration_initial_bytes = qemu_ftell(s->to_dst_file);
> +    s->iteration_initial_bytes = now;
>  
>      trace_migrate_transferred(transferred, time_spent,
>                                bandwidth, s->threshold_size);
> -- 
> 2.17.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela May 9, 2018, 11:14 a.m. UTC | #2
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> On multifd we send data from more places that main channel.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/migration.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 9b510a809a..75d30661e9 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2246,12 +2246,19 @@ static void migration_update_counters(MigrationState *s,
>>  {
>>      uint64_t transferred, time_spent;
>>      double bandwidth;
>> +    uint64_t now;
>>  
>>      if (current_time < s->iteration_start_time + BUFFER_DELAY) {
>>          return;
>>      }
>>  
>> -    transferred = qemu_ftell(s->to_dst_file) - s->iteration_initial_bytes;
>> +    if (migrate_use_multifd()) {
>> +        now = ram_counters.normal * qemu_target_page_size()
>> +            + qemu_ftell(s->to_dst_file);
>
> OK, can I just confirm, the 'multifd packets' go over the main fd, and
> just the pags are going over the other fd's?  In which case this is
> right; but if the headers are going over the other fd's as well then
> this is wrong.

We are not counting the headers on the multifd pages, you are right. fixing.

Thanks, Juan.
Juan Quintela May 9, 2018, 7:46 p.m. UTC | #3
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> On multifd we send data from more places that main channel.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/migration.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 9b510a809a..75d30661e9 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2246,12 +2246,19 @@ static void migration_update_counters(MigrationState *s,
>>  {
>>      uint64_t transferred, time_spent;
>>      double bandwidth;
>> +    uint64_t now;
>>  
>>      if (current_time < s->iteration_start_time + BUFFER_DELAY) {
>>          return;
>>      }
>>  
>> -    transferred = qemu_ftell(s->to_dst_file) - s->iteration_initial_bytes;
>> +    if (migrate_use_multifd()) {
>> +        now = ram_counters.normal * qemu_target_page_size()
>> +            + qemu_ftell(s->to_dst_file);
>
> OK, can I just confirm, the 'multifd packets' go over the main fd, and
> just the pags are going over the other fd's?

Nope.  We send (at the end) pages and metadata pages through the
multifd channels.  We don't send anything over the normal page for
multifd pages.  Multifd metadata is not taking into account.  It is
multifd_send_state->seq * sizeof(packet).  Fixing that.

> In which case this is
> right; but if the headers are going over the other fd's as well then
> this is wrong.

They are small, but still (aronud 64 offsets + something 20 bytes of
header), once every 64 pages, but as said, it is not difficult to do the
right thing.

Thanks, Juan.
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 9b510a809a..75d30661e9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2246,12 +2246,19 @@  static void migration_update_counters(MigrationState *s,
 {
     uint64_t transferred, time_spent;
     double bandwidth;
+    uint64_t now;
 
     if (current_time < s->iteration_start_time + BUFFER_DELAY) {
         return;
     }
 
-    transferred = qemu_ftell(s->to_dst_file) - s->iteration_initial_bytes;
+    if (migrate_use_multifd()) {
+        now = ram_counters.normal * qemu_target_page_size()
+            + qemu_ftell(s->to_dst_file);
+    } else {
+        now = qemu_ftell(s->to_dst_file);
+    }
+    transferred = now - s->iteration_initial_bytes;
     time_spent = current_time - s->iteration_start_time;
     bandwidth = (double)transferred / time_spent;
     s->threshold_size = bandwidth * s->parameters.downtime_limit;
@@ -2271,7 +2278,7 @@  static void migration_update_counters(MigrationState *s,
     qemu_file_reset_rate_limit(s->to_dst_file);
 
     s->iteration_start_time = current_time;
-    s->iteration_initial_bytes = qemu_ftell(s->to_dst_file);
+    s->iteration_initial_bytes = now;
 
     trace_migrate_transferred(transferred, time_spent,
                               bandwidth, s->threshold_size);