Re: [Qemu-devel] [PATCH v2 1/3] cpu: Provide vcpu throttling interface

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

On 06/03/2015 02:11 PM, Jason J. Herne wrote:
> On 06/03/2015 02:03 PM, Dr. David Alan Gilbert wrote:
>> * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote:
>>> On 06/03/2015 03:56 AM, Juan Quintela wrote:
>>>> "Jason J. Herne" <jjherne@linux.vnet.ibm.com> wrote:
> ...
>>>> We are checking for throotling on each cpu each 10ms.
>>>> But on patch 2 we can see that we only change the throotling each
>>>> time that we call migration_bitmap_sync(), that only happens each round
>>>> through all the pages. Normally auto-converge only matters for machines
>>>> with lots of memory, so this is going to happen each more than 10ms (we
>>>> change it each 4 passes).  You changed it to each 2 passes, and you add
>>>> it a 0.2.  I think  that I would preffer to just have it each single
>>>> pass, but add a 0.1 each pass?  simpler and end result would be the
>>>> same?
>>>>
>>>>
>>>
>>> Well, we certainly could make it run every pass but I think it would get
>>> a little too aggressive then. The reason is, we do not increment the
>>> throttle
>>> rate by adding 0.2 each time. We increment it by multiplying the current
>>> rate
>>> by 2. So by doing that every pass we are doubling the exponential growth
>>> rate. I will admit the numbers I chose are hardly scientific... I
>>> chose them
>>> because they seemed to provide a decent balance of "throttling
>>> aggression"
>>> in
>>> my workloads.
>>
>> That's the advantage of making them parameters.
>
> I see your point. Expecting the user to configure these parameters
> seems a bit much. But I guess, in theory, it is better to have the
> ability to change them and not need it, than need it and not have it
> right?
>
> So, as you stated earlier these should hook into MigrationParams
> somehow? I'll admit this is the first I've seen this construct. If
> this is the optimal location for the two controls (x-throttle-initial,
> x-throttle-multiplier?) I can add them there. Will keep defaults of
> 0.2 for initial and 2.0 for multiplier(is there a better name?)?
>

So I'm attempting add the initial throttle value and the multiplier to 
MigrationParameters and I've come across a problem. 
hmp_migrate_set_parameter assumes all parameters are ints. Apparently 
floating point is not allowed...

     void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     {
         const char *param = qdict_get_str(qdict, "parameter");
         int value = qdict_get_int(qdict, "value");

Also from hmp-commands.hx

     {
         .name       = "migrate_set_parameter",
         .args_type  = "parameter:s,value:i",
         .params     = "parameter value",
         .help       = "Set the parameter for migration",
         .mhandler.cmd = hmp_migrate_set_parameter,
         .command_completion = migrate_set_parameter_completion,
     },

I'm hoping someone already has an idea for dealing with this problem? If 
not, I suppose this is a good add-on for Dave's discussion on 
redesigning MigrationParameters.

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