From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2bdK-0006xp-NZ for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:45:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2bdH-0004WV-HK for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:45:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2bdH-0004W6-9F for qemu-devel@nongnu.org; Wed, 10 Jun 2015 04:45:11 -0400 Date: Wed, 10 Jun 2015 09:45:03 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150610084502.GC2135@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> <20150609080655.GB2135@work-vm> <557702EF.5070003@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <557702EF.5070003@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, armbru@redhat.com, qemu-devel@nongnu.org, borntraeger@de.ibm.com, amit.shah@redhat.com, afaerber@suse.de * Jason J. Herne (jjherne@linux.vnet.ibm.com) wrote: > On 06/09/2015 04:06 AM, Dr. David Alan Gilbert wrote: > >* 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. > > > > I could change it to a string, and then parse the data on a case-by-case > basis in the switch/case logic. I feel like this is making a bad situation > worse... But I don't see an easy way around it. I think the easiest 'solution' for this is to make the parameter an integer percentage rather than as a float. Not that pretty but easier than fighting the interface code. Dave > > -- > -- Jason J. Herne (jjherne@linux.vnet.ibm.com) > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK