Re: [Qemu-devel] [PATCH v3 3/5] migration: Dynamic cpu throttling for auto-converge

["Jason J. Herne" <jjherne@linux.vnet.ibm.com>]

On 06/26/2015 01:54 PM, Dr. David Alan Gilbert wrote:
> * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote:
>> Remove traditional auto-converge static 30ms throttling code and replace it
>> with a dynamic throttling algorithm.
>>
>> Additionally, be more aggressive when deciding when to start throttling.
>> Previously we waited until four unproductive memory passes. Now we begin
>> throttling after only two unproductive memory passes. Four seemed quite
>> arbitrary and only waiting for two passes allows us to complete the migration
>> faster.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Reviewed-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> ---
>>   arch_init.c           | 93 +++++++++++++++++----------------------------------
>>   migration/migration.c |  4 +++
>>   2 files changed, 34 insertions(+), 63 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 23d3feb..d456527 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -111,9 +111,7 @@ int graphic_depth = 32;
>>   #endif
>>
>>   const uint32_t arch_type = QEMU_ARCH;
>> -static bool mig_throttle_on;
>>   static int dirty_rate_high_cnt;
>> -static void check_guest_throttling(void);
>>
>>   static uint64_t bitmap_sync_count;
>>
>> @@ -487,6 +485,29 @@ static size_t save_page_header(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
>>       return size;
>>   }
>>
>> +/* Reduce amount of guest cpu execution to hopefully slow down memory writes.
>> + * If guest dirty memory rate is reduced below the rate at which we can
>> + * transfer pages to the destination then we should be able to complete
>> + * migration. Some workloads dirty memory way too fast and will not effectively
>> + * converge, even with auto-converge.
>> + */
>> +static void mig_throttle_guest_down(void)
>> +{
>> +    MigrationState *s = migrate_get_current();
>> +    uint64_t pct_initial =
>> +            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INITIAL];
>> +    uint64_t pct_icrement =
>> +            s->parameters[MIGRATION_PARAMETER_X_CPU_THROTTLE_INCREMENT];
>> +
>> +    /* We have not started throttling yet. Let's start it. */
>> +    if (!cpu_throttle_active()) {
>> +        cpu_throttle_set(pct_initial);
>> +    } else {
>> +        /* Throttling already on, just increase the rate */
>> +        cpu_throttle_set(cpu_throttle_get_percentage() + pct_icrement);
>> +    }
>
> Shouldn't this cap it at 100% ?
>

The code in cpu_throttle_set() actually caps it at 99 percent.

...
>> @@ -1197,7 +1218,6 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>>       RAMBlock *block;
>>       int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
>>
>> -    mig_throttle_on = false;
>>       dirty_rate_high_cnt = 0;
>>       bitmap_sync_count = 0;
>>       migration_bitmap_sync_init();
>> @@ -1301,12 +1321,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
>>           }
>>           pages_sent += pages;
>>           acct_info.iterations++;
>> -        check_guest_throttling();
>> -        /* we want to check in the 1st loop, just in case it was the 1st time
>> -           and we had to sync the dirty bitmap.
>> -           qemu_get_clock_ns() is a bit expensive, so we only check each some
>> -           iterations
>> -        */
>> +
>
> Those comments are related to the code below aren't they, not the line you removed?
>

Yes, oversight on my part. I will re-add them for v4 :) Thanks!!

>>           if ((i & 63) == 0) {
>>               uint64_t t1 = (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - t0) / 1000000;
>>               if (t1 > MAX_WAIT) {
>> @@ -1913,51 +1928,3 @@ TargetInfo *qmp_query_target(Error **errp)
>>       return info;
>>   }
>>
...
-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)