Message ID | 1500378106-2620-11-git-send-email-vijay.kilari@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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. 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. Cheers,
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. The other solution is create a arch wrapper and call this from here. Regards Vijay > > Cheers, > > -- > Julien Grall
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,
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 ) return -1; if ( !arch_sanitize_nodes_memory() ) @@ -430,11 +430,9 @@ void __init numa_initmem_init(unsigned long start_pfn, unsigned long end_pfn) return; #endif -#ifdef CONFIG_ACPI_NUMA if ( !numa_off && !numa_scan_nodes(pfn_to_paddr(start_pfn), pfn_to_paddr(end_pfn)) ) return; -#endif printk(KERN_INFO "%s\n", numa_off ? "NUMA turned off" : "No NUMA configuration found");