From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45932) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2EYs-0007UX-Bb for qemu-devel@nongnu.org; Tue, 09 Jun 2015 04:07:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2EYo-0002Fp-6G for qemu-devel@nongnu.org; Tue, 09 Jun 2015 04:07:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42006) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2EYn-0002FR-Ue for qemu-devel@nongnu.org; Tue, 09 Jun 2015 04:07:02 -0400 Date: Tue, 9 Jun 2015 09:06:56 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150609080655.GB2135@work-vm> References: <1433267209-9882-1-git-send-email-jjherne@linux.vnet.ibm.com> <1433267209-9882-2-git-send-email-jjherne@linux.vnet.ibm.com> <87pp5drzrl.fsf@neno.neno> <556F412C.9090106@linux.vnet.ibm.com> <20150603180356.GE2129@work-vm> <556F4343.6070801@linux.vnet.ibm.com> <5575E7E0.4050209@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5575E7E0.4050209@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 1/3] cpu: Provide vcpu throttling interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Jason J. Herne" Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, amit.shah@redhat.com, afaerber@suse.de * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote: > 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" 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. Oh, that's yet another problem; hadn't thought about this one. I don't think the suggestions I had in the previous mail would help that one either; It might work if you flipped the type to 's' and then parsed that in the hmp code. (cc'ing Markus in) Dave > > -- > -- Jason J. Herne (jjherne@linux.vnet.ibm.com) > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK