Re: [RFC PATCH v3 10/24] NUMA: Allow numa initialization with DT

From: Julien Grall <julien.grall@arm.com>
To: Vijay Kilari <vijay.kilari@gmail.com>
Cc: Tim Deegan <tim@xen.org>,
	kevin.tian@intel.com, Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Dario Faggioli <dario.faggioli@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	Vijaya Kumar K <Vijaya.Kumar@cavium.com>
Subject: Re: [RFC PATCH v3 10/24] NUMA: Allow numa initialization with DT
Date: Thu, 20 Jul 2017 12:20:02 +0100	[thread overview]
Message-ID: <ae8b752d-fb77-bb1e-a829-2e1f6eeda2a2@arm.com> (raw)
In-Reply-To: <CALicx6t1kHhd2S_9=ehtLkrL1C7ThnDURxzGPVUDkrPnPo0obQ@mail.gmail.com>

Hi Vijay,

On 20/07/17 11:28, Vijay Kilari wrote:
> On Wed, Jul 19, 2017 at 11:28 PM, Julien Grall <julien.grall@arm.com> wrote:
>> Hi Vijay,
>>
>> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>>
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> The common code allows numa initialization only when
>>> ACPI_NUMA config is enabled. Allow initialization when
>>> NUMA config is enabled for DT.
>>>
>>> In this patch, along with acpi_numa, check for acpi_disabled
>>> is added.
>>>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> ---
>>>  xen/common/numa.c | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/xen/common/numa.c b/xen/common/numa.c
>>> index 74c4697..5e985d2 100644
>>> --- a/xen/common/numa.c
>>> +++ b/xen/common/numa.c
>>> @@ -324,7 +324,7 @@ static int __init numa_scan_nodes(paddr_t start,
>>> paddr_t end)
>>>      for ( i = 0; i < MAX_NUMNODES; i++ )
>>>          cutoff_node(i, start, end);
>>>
>>> -    if ( acpi_numa <= 0 )
>>> +    if ( !acpi_disabled && acpi_numa <= 0 )
>>
>>
>> I am struggling to understand this change. Likely you want to similar
>> variable for DT to say NUMA is available or this has failed.
>
> Yes, without this check for acpi_disabled, when booting with DT, the check
> acpi_numa <= 0 is true and does not allow numa initialization.
>
>>
>> This also change quite a bit the semantic for x86 because, you will now
>> continue if acpi_disabled and acpi_numa = 0. The code seems to allow it, but
>> I don't know if we support it.
>
> Yes, but prior to this patch, x86 is assuming that acpi_disabled is
> false by checking
> only for acpi_numa <=0.

101 of the contributor: make sure the commit message is meaningful to 
understand your changes.

>
> The other solution is create a arch wrapper and call this from here.

I suggested another solution but you seem to have ignored it... You can 
rename acpi_numa into something more generic and use it also for DT to 
detect whether it is possible to use NUMA.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel