Patchwork [3/4] migration: don't account sleep time for calculating bandwidth

login
register
mail settings
Submitter Juan Quintela
Date Feb. 1, 2013, 12:32 p.m.
Message ID <1359721976-19238-4-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/217456/
State New
Headers show

Comments

Juan Quintela - Feb. 1, 2013, 12:32 p.m.
While we are sleeping we are not sending, so we should not use that
time to estimate our bandwidth.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Orit Wasserman - Feb. 4, 2013, 12:50 p.m.
On 02/01/2013 02:32 PM, Juan Quintela wrote:
> While we are sleeping we are not sending, so we should not use that
> time to estimate our bandwidth.
Juan,
Maybe I missing something here but the sleep time is cause by the fact
we limit the migration bandwidth. So the available bandwidth should take
it into consideration? we still cape the bandwidth in the last stage.
Regards,
Orit
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/migration.c b/migration.c
> index 67abd12..64e75ca 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -666,6 +666,7 @@ static void *buffered_file_thread(void *opaque)
>  {
>      MigrationState *s = opaque;
>      int64_t initial_time = qemu_get_clock_ms(rt_clock);
> +    int64_t sleep_time = 0;
>      int64_t max_size = 0;
>      bool last_round = false;
>      int ret;
> @@ -738,7 +739,7 @@ static void *buffered_file_thread(void *opaque)
>          current_time = qemu_get_clock_ms(rt_clock);
>          if (current_time >= initial_time + BUFFER_DELAY) {
>              uint64_t transferred_bytes = s->bytes_xfer;
> -            uint64_t time_spent = current_time - initial_time;
> +            uint64_t time_spent = current_time - initial_time - sleep_time;
>              double bandwidth = transferred_bytes / time_spent;
>              max_size = bandwidth * migrate_max_downtime() / 1000000;
> 
> @@ -747,11 +748,13 @@ static void *buffered_file_thread(void *opaque)
>                      transferred_bytes, time_spent, bandwidth, max_size);
> 
>              s->bytes_xfer = 0;
> +            sleep_time = 0;
>              initial_time = current_time;
>          }
>          if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
>              /* usleep expects microseconds */
>              g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
> +            sleep_time += qemu_get_clock_ms(rt_clock) - current_time;
>          }
>          ret = buffered_flush(s);
>          if (ret < 0) {
>
Juan Quintela - Feb. 5, 2013, 2:55 p.m.
Orit Wasserman <owasserm@redhat.com> wrote:
> On 02/01/2013 02:32 PM, Juan Quintela wrote:
>> While we are sleeping we are not sending, so we should not use that
>> time to estimate our bandwidth.
> Juan,
> Maybe I missing something here but the sleep time is cause by the fact
> we limit the migration bandwidth. So the available bandwidth should take
> it into consideration? we still cape the bandwidth in the last stage.
> Regards,


No, in the last stage we sent as far as we can.  If last_round == true,
we dont' enter the if with the g_usleep().

Later, Juan.
Orit Wasserman - Feb. 5, 2013, 3:02 p.m.
On 02/05/2013 04:55 PM, Juan Quintela wrote:
> Orit Wasserman <owasserm@redhat.com> wrote:
>> On 02/01/2013 02:32 PM, Juan Quintela wrote:
>>> While we are sleeping we are not sending, so we should not use that
>>> time to estimate our bandwidth.
>> Juan,
>> Maybe I missing something here but the sleep time is cause by the fact
>> we limit the migration bandwidth. So the available bandwidth should take
>> it into consideration? we still cape the bandwidth in the last stage.
>> Regards,
> 
> 
> No, in the last stage we sent as far as we can.  If last_round == true,
> we dont' enter the if with the g_usleep().
Ok got it.

Reviewed-by: Orit Wasserman <owasserm@redhat.com>

> 
> Later, Juan.
>

Patch

diff --git a/migration.c b/migration.c
index 67abd12..64e75ca 100644
--- a/migration.c
+++ b/migration.c
@@ -666,6 +666,7 @@  static void *buffered_file_thread(void *opaque)
 {
     MigrationState *s = opaque;
     int64_t initial_time = qemu_get_clock_ms(rt_clock);
+    int64_t sleep_time = 0;
     int64_t max_size = 0;
     bool last_round = false;
     int ret;
@@ -738,7 +739,7 @@  static void *buffered_file_thread(void *opaque)
         current_time = qemu_get_clock_ms(rt_clock);
         if (current_time >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = s->bytes_xfer;
-            uint64_t time_spent = current_time - initial_time;
+            uint64_t time_spent = current_time - initial_time - sleep_time;
             double bandwidth = transferred_bytes / time_spent;
             max_size = bandwidth * migrate_max_downtime() / 1000000;

@@ -747,11 +748,13 @@  static void *buffered_file_thread(void *opaque)
                     transferred_bytes, time_spent, bandwidth, max_size);

             s->bytes_xfer = 0;
+            sleep_time = 0;
             initial_time = current_time;
         }
         if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
             /* usleep expects microseconds */
             g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
+            sleep_time += qemu_get_clock_ms(rt_clock) - current_time;
         }
         ret = buffered_flush(s);
         if (ret < 0) {