From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 4 of 4] Support new xl command cpupool-numa-split Date: Wed, 08 Dec 2010 13:20:34 +0100 Message-ID: <4CFF7812.4050506@ts.fujitsu.com> References: <1291806978.13966.4529.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1291806978.13966.4529.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 12/08/10 12:16, Ian Campbell wrote: > On Fri, 2010-11-26 at 07:10 +0000, Juergen Gross wrote: >> + >> + if (libxl_cpumap_alloc(&ctx,&cpumap)) { >> + fprintf(stderr, "Failed to allocate cpumap\n"); >> + return -ERROR_FAIL; >> + } >> + >> + poolinfo = libxl_list_cpupool(&ctx,&n_pools); >> + if (!poolinfo) { >> + fprintf(stderr, "error getting cpupool info\n"); >> + return -ERROR_NOMEM; >> + } >> + poolid = poolinfo[0].poolid; >> + schedid = poolinfo[0].sched_id; >> + libxl_for_each_cpu(c, poolinfo[0].cpumap) >> + if (libxl_cpumap_test(&poolinfo[0].cpumap, c)) >> + libxl_cpumap_set(&cpumap, c); > > Can the libxl_cpumap_alloc and this loop be deferred until after the > check and bail for n_pools> 1? Sure. > > There seems to be no way to find out the number of pools without also > getting all the info about them, which is a shame. Taking a quick look I couldn't spot any way how to find out the number of domains without also getting all the info about them, too... > >> + /* Reset Pool-0 to 1st node */ >> + node = topology->nodemap.array[0]; >> + libxl_for_each_cpu(c, cpumap) { >> + if (!libxl_cpumap_test(&cpumap, c)&& (c< topology->nodemap.entries)&& >> + (topology->nodemap.array[c] == node)) { >> + ret = -libxl_cpupool_cpuadd(&ctx, poolid, c); >> + if (ret) { >> + fprintf(stderr, "error on adding cpu to Pool-0\n"); >> + goto out; >> + } >> + libxl_cpumap_reset(&freemap, c); > > (nt really related to this series but I wish this was called > libxl_cpumap_clear, I had to go check it wasn't resetting the whole map > or something...) Hmm, do you really think so? It would make me to check whether it is clearing the whole map :-) I think the second parameter is a strong hint :-) > >> + } >> + } >> + libxl_for_each_cpu(c, cpumap) { >> + if (libxl_cpumap_test(&cpumap, c)&& (c< topology->nodemap.entries)&& >> + (topology->nodemap.array[c] != node)) { >> + ret = -libxl_cpupool_cpuremove(&ctx, poolid, c); >> + if (ret) { >> + fprintf(stderr, "error on removing cpu from Pool-0\n"); >> + goto out; >> + } >> + libxl_cpumap_set(&freemap, c); >> + } >> + } > > Can this loop be merged with the preceding loop, with the body being the > else case of the if? No. I have to add new cpus first to avoid a cpupool without cpus in between. > >> + for (;;) { >> + node = -1; >> + libxl_for_each_cpu(c, freemap) { >> + if (libxl_cpumap_test(&freemap, c)&& (node == -1)) { >> + node = topology->nodemap.array[c]; >> + } >> + libxl_cpumap_reset(&cpumap, c); >> + if ((node>= 0)&& libxl_cpumap_test(&freemap, c)&& >> + (c< topology->nodemap.entries)&& (topology->nodemap.array[c] == node)) { >> + libxl_cpumap_reset(&freemap, c); >> + libxl_cpumap_set(&cpumap, c); >> + } >> + } >> + if (node == -1) >> + break; >> + >> + snprintf(name, 15, "Pool-node%d", node); > > Do we want to rename Pool-0 at some point too or do we rely on that name > elsewhere? Good question. There is a hard coded "Pool-0" reference in libxl, but this could easily be changed. I'm not sure about implications in xm/xend. I'll check this. Juergen -- Juergen Gross Principal Developer Operating Systems TSP ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html