From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35611) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2ej7-0004oj-MS for qemu-devel@nongnu.org; Wed, 10 Jun 2015 08:03:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z2ej3-00077G-9Y for qemu-devel@nongnu.org; Wed, 10 Jun 2015 08:03:25 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:51155) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z2ej3-00075R-3m for qemu-devel@nongnu.org; Wed, 10 Jun 2015 08:03:21 -0400 Received: from /spool/local by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 10 Jun 2015 06:03:19 -0600 Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 6974D1FF004B for ; Wed, 10 Jun 2015 05:54:26 -0600 (MDT) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by b03cxnp07028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t5AC0voP30670860 for ; Wed, 10 Jun 2015 05:00:57 -0700 Received: from d03av03.boulder.ibm.com (localhost [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t5AC3ENY014764 for ; Wed, 10 Jun 2015 06:03:14 -0600 Message-ID: <5578277C.7070201@linux.vnet.ibm.com> Date: Wed, 10 Jun 2015 08:03:08 -0400 From: "Jason J. Herne" MIME-Version: 1.0 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> <20150610084502.GC2135@work-vm> In-Reply-To: <20150610084502.GC2135@work-vm> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/3] cpu: Provide vcpu throttling interface Reply-To: jjherne@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: quintela@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, borntraeger@de.ibm.com, amit.shah@redhat.com, afaerber@suse.de On 06/10/2015 04:45 AM, Dr. David Alan Gilbert wrote: > * 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. > Yes, I'm starting to feel this way :). Another option I'd like to collect opinions on it to change hmp's migrate_set_parameter to accept argument type O. As per monitor.c: * 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. * Restriction: only lists with empty desc are supported This would allow arbitrary data types and allow several parameters to be set at once right? It looks like it would be a relatively straightforward change to make. Opinions? -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)