From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X250T-0005Sg-8M for qemu-devel@nongnu.org; Tue, 01 Jul 2014 16:50:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X250J-0000pb-W6 for qemu-devel@nongnu.org; Tue, 01 Jul 2014 16:50:24 -0400 Received: from e38.co.us.ibm.com ([32.97.110.159]:60721) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X250J-0000pU-PH for qemu-devel@nongnu.org; Tue, 01 Jul 2014 16:50:15 -0400 Received: from /spool/local by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 1 Jul 2014 14:50:13 -0600 Date: Tue, 1 Jul 2014 13:50:06 -0700 From: Nishanth Aravamudan Message-ID: <20140701205006.GA428@linux.vnet.ibm.com> References: <20140701201108.GA21770@linux.vnet.ibm.com> <20140701201328.GB21770@linux.vnet.ibm.com> <20140701203957.GU3222@otherpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140701203957.GU3222@otherpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH 2/2 v5] numa: enable sparse node numbering on ppc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: "Michael S. Tsirkin" , Alexey Kardashevskiy , Hu Tao , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Anton Blanchard , David Rientjes , Igor Mammedov On 01.07.2014 [17:39:57 -0300], Eduardo Habkost wrote: > On Tue, Jul 01, 2014 at 01:13:28PM -0700, Nishanth Aravamudan wrote: > [...] > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 12472c6..cdefafe 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1121,6 +1121,18 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size, > > guest_info->ram_size = below_4g_mem_size + above_4g_mem_size; > > guest_info->apic_id_limit = pc_apic_id_limit(max_cpus); > > guest_info->apic_xrupt_override = kvm_allows_irq0_override(); > > + /* No support for sparse NUMA node IDs yet: */ > > + for (i = max_numa_nodeid - 1; i >= 0; i--) { > > + /* Report large node IDs first, to make mistakes easier to spot */ > > + if (!numa_info[i].present) { > > + error_report("numa: Node ID missing: %d", i); > > + exit(EXIT_FAILURE); > > + } > > + } > > + > > + /* This must be always true if all nodes are present */ > > + assert(num_numa_nodes == max_numa_nodeid); > > + > > I wonder if there's a better place where we could put this check. Well, only i386 and ppc support NUMA, afaict. So I'm not sure where it makes sense to put it. I guess we could have a flag that the architectures set that indicates sparse NUMA support or not, and put this in the generic code. Or do you mean putting this check somewhere else in the PC init code? > > guest_info->numa_nodes = num_numa_nodes; > > guest_info->node_mem = g_malloc0(guest_info->numa_nodes * > > sizeof *guest_info->node_mem); > [...] > > diff --git a/numa.c b/numa.c > > index 5930df0..a689e52 100644 > > --- a/numa.c > > +++ b/numa.c > [...] > > @@ -225,9 +220,12 @@ void set_numa_nodes(void) > > * must cope with this anyway, because there are BIOSes out there in > > * real machines which also use this scheme. > > */ > > - if (i == num_numa_nodes) { > > - for (i = 0; i < max_cpus; i++) { > > - set_bit(i, numa_info[i % num_numa_nodes].node_cpu); > > + if (i == max_numa_nodeid) { > > + for (i = 0, j = 0; i < max_cpus; i++) { > > Doesn't j need to be initialized to -1, here? Arrgh, sorry had been messing with your suggestion to use a while loop. You're right, it needs to be -1 here. > Except for that, patch looks good to me. But I would be more comfortable > with it if we had automated tests to help ensure we are not breaking > compatibility of existing NUMA command-line conbinations with these > changes. Is that the test target in the qemu source? Are there examples of any such NUMA tests already? Thanks, Nish