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

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

On 06/02/2015 09:58 AM, Dr. David Alan Gilbert wrote:
> * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote:
>> On 06/01/2015 11:32 AM, 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           | 95 +++++++++++++++++----------------------------------
>>>>   migration/migration.c |  9 +++++
>>>>   2 files changed, 41 insertions(+), 63 deletions(-)
>>>>
>>>> diff --git a/arch_init.c b/arch_init.c
>>>> index 23d3feb..73ae494 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,31 @@ 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. For these workloads we will continue to
>>>> + * increase throttling until the guest is paused long enough to complete the
>>>> + * migration. This essentially becomes a non-live migration.
>>>> + */
>>>> +static void mig_throttle_guest_down(void)
>>>> +{
>>>> +    CPUState *cpu;
>>>> +
>>>> +    CPU_FOREACH(cpu) {
>>>> +        /* We have not started throttling yet. Lets start it.*/
>>>> +        if (!cpu_throttle_active(cpu)) {
>>>> +            cpu_throttle_start(cpu, 0.2);
>>>> +        }
>>>> +
>>>> +        /* Throttling is already in place. Just increase the throttling rate */
>>>> +        else {
>>>> +            cpu_throttle_start(cpu, cpu_throttle_get_ratio(cpu) * 2);
>>>> +        }
>>>
>>> Now that migration has migrate_parameters, it would be best to replace
>>> the magic numbers (the 0.2, the *2 - anything else?)  by parameters that can
>>> change the starting throttling and increase rate.  It would probably also be
>>> good to make the current throttling rate visible in info somewhere; maybe
>>> info migrate?
>>>
>>
>> I did consider all of this. However, I don't think that the controls
>> this patch provides are an ideal throttling mechanism. I suspect someone
>> with
>> vcpu/scheduling experience could whip up something more user friendly and
>> cleaner.
>> I merely propose this because it seems better than what we have today for
>> auto-converge.
>>
>> Also, I'm not sure how useful the information really is to the user. The
>> fact that it is a ratio instead of a percentage might be confusing. Also,
>> I suspect it is not
>> truly very accurate. Again, I was going for "make it better", not "make it
>> perfect".
>>
>> Lastly, if we define this external interface we are kind of stuck with it,
>> yes?
>
> Well, one thing you can do is add a parameter with a name starting with x-
> which means it's not a fixed interface (so things like libvirt wont use it).
> And the reason I was interested in seeing the information was otherwise
> we don't really have any way of knowing how well the code is working;
> is it already throttling down more and more?
>

Fair point. Can we add a qmp/hmp command like "info x-cpu-throttle"? Or a
new line in "info migrate" output, "x-throttle-ration:  1.0" perhaps?
Would this mess up libvirt's parsing of the hmp/qmp data?


-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)