From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:51490)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <nacc@linux.vnet.ibm.com>) id 1X0DwT-0002Ib-UJ
	for qemu-devel@nongnu.org; Thu, 26 Jun 2014 13:58:47 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <nacc@linux.vnet.ibm.com>) id 1X0DwK-0004Wy-8c
	for qemu-devel@nongnu.org; Thu, 26 Jun 2014 13:58:37 -0400
Received: from e33.co.us.ibm.com ([32.97.110.151]:34934)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <nacc@linux.vnet.ibm.com>) id 1X0DwI-0004Ud-Sq
	for qemu-devel@nongnu.org; Thu, 26 Jun 2014 13:58:28 -0400
Received: from /spool/local
	by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only!
	Violators will be prosecuted
	for <qemu-devel@nongnu.org> from <nacc@linux.vnet.ibm.com>;
	Thu, 26 Jun 2014 11:58:19 -0600
Received: from b03cxnp07029.gho.boulder.ibm.com
	(b03cxnp07029.gho.boulder.ibm.com [9.17.130.16])
	by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id A007D19D803E
	for <qemu-devel@nongnu.org>; Thu, 26 Jun 2014 11:58:07 -0600 (MDT)
Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168])
	by b03cxnp07029.gho.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with
	ESMTP id s5QFsivC11010470
	for <qemu-devel@nongnu.org>; Thu, 26 Jun 2014 17:54:44 +0200
Received: from d03av02.boulder.ibm.com (localhost [127.0.0.1])
	by d03av02.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP
	id s5QHwFHI023094
	for <qemu-devel@nongnu.org>; Thu, 26 Jun 2014 11:58:16 -0600
Date: Thu, 26 Jun 2014 10:58:07 -0700
From: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
Message-ID: <20140626175807.GM4323@linux.vnet.ibm.com>
References: <20140623193310.GD4323@linux.vnet.ibm.com>
	<20140624004825.GE4323@linux.vnet.ibm.com>
	<20140624174038.GK4323@linux.vnet.ibm.com>
	<20140625132134.24b70a65@nial.usersys.redhat.com>
	<20140625161359.GA8698@linux.vnet.ibm.com>
	<20140625165256.GD3222@otherpad.lan.raisama.net>
	<20140625182317.GG16636@redhat.com>
	<20140626090924.GA12168@G08FNSTD100614.fnst.cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20140626090924.GA12168@G08FNSTD100614.fnst.cn.fujitsu.com>
Subject: Re: [Qemu-devel] [RFC PATCH v3] numa: enable sparse node numbering
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Hu Tao <hutao@cn.fujitsu.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Alexey Kardashevskiy <aik@ozlabs.ru>, qemu-devel@nongnu.org, Anton Blanchard <anton@samba.org>, David Rientjes <rientjes@google.com>, Igor Mammedov <imammedo@redhat.com>

On 26.06.2014 [17:09:25 +0800], Hu Tao wrote:
> On Wed, Jun 25, 2014 at 09:23:17PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Jun 25, 2014 at 01:52:56PM -0300, Eduardo Habkost wrote:
> > > On Wed, Jun 25, 2014 at 09:13:59AM -0700, Nishanth Aravamudan wrote:
> > > > On 25.06.2014 [13:21:34 +0200], Igor Mammedov wrote:
> > > > > On Tue, 24 Jun 2014 10:40:38 -0700
> > > > > Nishanth Aravamudan <nacc@linux.vnet.ibm.com> wrote:
> > > > <snip>
> > > > > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > > > > index 277230d..b90bf66 100644
> > > > > > --- a/include/sysemu/sysemu.h
> > > > > > +++ b/include/sysemu/sysemu.h
> > > > > > @@ -145,11 +145,13 @@ extern int mem_prealloc;
> > > > > >   */
> > > > > >  #define MAX_CPUMASK_BITS 255
> > > > > >  
> > > > > > -extern int nb_numa_nodes;
> > > > > > +extern int nb_numa_nodes; /* Number of NUMA nodes */
> > > > > > +extern int max_numa_node; /* Highest specified NUMA node ID */
> > > > > >  typedef struct node_info {
> > > > > >      uint64_t node_mem;
> > > > > >      DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
> > > > > >      struct HostMemoryBackend *node_memdev;
> > > > > > +    bool present;
> > > > > How about dropping 'present' and replacing array with a list
> > > > > of only present nodes?
> > > > 
> > > > If that would be preferred, I can move to that. I assume a simple
> > > > linked-list is fine. Does qemu provide any infrastructure for defining
> > > > lists? I'll look through the source but any pointers would be helpful.
> > > > 
> > > > Generally speaking, sparse NUMA nodes aren't that common and when they
> > > > exist, the gaps aren't large. But it does seem to make sense if we have
> > > > sparse IDs at all, we might as well move to a list.
> > > > 
> > > > In any case, moving to the list means we'd have a nodeid as part of the
> > > > structure instead.
> > > > 
> > > > > That way it will be one more step closer to converting numa
> > > > > infrastructure to a set of QOM objects.
> > > > 
> > > > Sounds like a good idea to me. I'll respin the patch soon.
> > > 
> > > Having a list makes sense, the only difference is that keeping a sparse
> > > array sorted is much easier than making a sorted list (because the ACPI
> > > tables are nodeid-ordered). That's why I suggested keeping the array
> > > initially.
> > > 
> > > Adding a "present" field to the array is a trivial and easy-to-review
> > > change. Changing NodeInfo to use linked lists is a more complex change
> > > that I wouldn't want to include after soft freeze.
> > > 
> > > In other words:
> > >  * Having a list is better than a sparse array; but:
> > >  * Having a small sparse array with the "present" field is better
> > >    than broken sparse nodeid support (IMO).
> > 
> > I agree here. This patchset is still RFC but if that's
> > the only issue I might apply it.
> 
> I don't think it is ready for 2.1 since it lacks arch-specific changes.
> The patch itself only makes qemu be able to bring up guest with sparse
> numa nodes, without arch-specific changes, guest sees only one node(at
> least on x86).

Well, it does no harm or foul to have it, on it's own. In that, nothing
is worse or better than it was. Although I would argue the qemu core is
better than it was :) I have sent the powerpc enabling patch, but even
with that, we need Alexey's series of 6 patches (which do fix a
regression) to fully support the sparse numbering. Maybe that means I've
convinced myself it's best to wait until after 2.1.

I honestly am not sure how to "fix" the x86 code to support the sparse
numbering -- the APIC code seems to rely on it being continuous. From
some googling, it seems like the most common reason for non-continuous
numbering on x86 is faulty hardware. I will keep fiddling if no one else
steps up, but I'm not an x86 NUMA expert :)

Thanks,
Nish