Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932362AbcDLIEU (ORCPT ); Tue, 12 Apr 2016 04:04:20 -0400 Received: from cn.fujitsu.com ([59.151.112.132]:15926 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S932327AbcDLIEK (ORCPT ); Tue, 12 Apr 2016 04:04:10 -0400 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="5508998" Message-ID: <570CABDE.7030902@cn.fujitsu.com> Date: Tue, 12 Apr 2016 16:03:42 +0800 From: Zhu Guihua User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Lorenzo Pieralisi CC: , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH v6 5/5] x86, acpi, cpu-hotplug: Set persistent cpuid <-> nodeid mapping when booting. References: <0ecee1cba429e53220c7887c7a139ad598c5a4a2.1458177577.git.zhugh.fnst@cn.fujitsu.com> <20160406142916.GA1462@red-moon> In-Reply-To: <20160406142916.GA1462@red-moon> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.167.226.252] X-yoursite-MailScanner-ID: C2CD0408D281.ABAE9 X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: zhugh.fnst@cn.fujitsu.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6200 Lines: 186 Hi Lorenzo, Thanks for your test and detailed suggestions. I will update those later. Thanks, Zhu On 04/06/2016 10:29 PM, Lorenzo Pieralisi wrote: > [+Dennis since he reported ARM64 build breakage] > > On Thu, Mar 17, 2016 at 09:32:40AM +0800, Zhu Guihua wrote: >> From: Gu Zheng >> >> The whole patch-set aims at making cpuid <-> nodeid mapping persistent. So that, >> when node online/offline happens, cache based on cpuid <-> nodeid mapping such as >> wq_numa_possible_cpumask will not cause any problem. >> It contains 4 steps: >> 1. Enable apic registeration flow to handle both enabled and disabled cpus. >> 2. Introduce a new array storing all possible cpuid <-> apicid mapping. >> 3. Enable _MAT and MADT relative apis to return non-presnet or disabled cpus' apicid. >> 4. Establish all possible cpuid <-> nodeid mapping. >> >> This patch finishes step 4. > And it breaks the build on ARM64. > > drivers/acpi/processor_core.c: In function 'set_processor_node_mapping': > drivers/acpi/processor_core.c:316:2: error: implicit declaration of > function 'acpi_map_cpu2node' [-Werror=implicit-function-declaration] > > [...] > >> diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c >> index b1698bc..7db5563 100644 >> --- a/arch/ia64/kernel/acpi.c >> +++ b/arch/ia64/kernel/acpi.c >> @@ -796,7 +796,7 @@ int acpi_isa_irq_to_gsi(unsigned isa_irq, u32 *gsi) >> * ACPI based hotplug CPU support >> */ >> #ifdef CONFIG_ACPI_HOTPLUG_CPU >> -static int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) >> +int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) > Return value seems to be ignored on IA64 so you can get rid of it, > see below. > >> { >> #ifdef CONFIG_ACPI_NUMA >> /* >> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c >> index 0ce06ee..7d45261 100644 >> --- a/arch/x86/kernel/acpi/boot.c >> +++ b/arch/x86/kernel/acpi/boot.c >> @@ -696,7 +696,7 @@ static void __init acpi_set_irq_model_ioapic(void) >> #ifdef CONFIG_ACPI_HOTPLUG_CPU >> #include >> >> -static void acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) >> +void acpi_map_cpu2node(acpi_handle handle, int cpu, int physid) >> { >> #ifdef CONFIG_ACPI_NUMA >> int nid; >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >> index 0e85678..215177a 100644 >> --- a/drivers/acpi/bus.c >> +++ b/drivers/acpi/bus.c >> @@ -1110,6 +1110,9 @@ static int __init acpi_init(void) >> acpi_sleep_proc_init(); >> acpi_wakeup_device_init(); >> acpi_debugger_init(); >> +#ifdef CONFIG_ACPI_HOTPLUG_CPU >> + acpi_set_processor_mapping(); >> +#endif >> return 0; >> } >> >> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c >> index 824b98b..45580ff 100644 >> --- a/drivers/acpi/processor_core.c >> +++ b/drivers/acpi/processor_core.c >> @@ -261,6 +261,71 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id) >> } >> EXPORT_SYMBOL_GPL(acpi_get_cpuid); >> >> +#ifdef CONFIG_ACPI_HOTPLUG_CPU >> +static bool map_processor(acpi_handle handle, int *phys_id, int *cpuid) > phys_id size is 64 bits (phys_cpuid_t) on ARM64, (phys_cpuid_t *phys_id) is > what you have to have here. > >> +{ >> + int type; >> + u32 acpi_id; >> + acpi_status status; >> + acpi_object_type acpi_type; >> + unsigned long long tmp; >> + union acpi_object object = { 0 }; >> + struct acpi_buffer buffer = { sizeof(union acpi_object), &object }; >> + >> + status = acpi_get_type(handle, &acpi_type); >> + if (ACPI_FAILURE(status)) >> + return false; >> + >> + switch (acpi_type) { >> + case ACPI_TYPE_PROCESSOR: >> + status = acpi_evaluate_object(handle, NULL, NULL, &buffer); >> + if (ACPI_FAILURE(status)) >> + return false; >> + acpi_id = object.processor.proc_id; >> + break; >> + case ACPI_TYPE_DEVICE: >> + status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp); >> + if (ACPI_FAILURE(status)) >> + return false; >> + acpi_id = tmp; >> + break; >> + default: >> + return false; >> + } >> + >> + type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0; >> + >> + *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false); > Wrong on ARM64, see above. > >> + *cpuid = acpi_map_cpuid(*phys_id, acpi_id); >> + if (*cpuid == -1) >> + return false; >> + >> + return true; >> +} >> + >> +static acpi_status __init >> +set_processor_node_mapping(acpi_handle handle, u32 lvl, void *context, >> + void **rv) >> +{ >> + u32 apic_id; > - You can't use u32 here see above > - This is generic code and on ARM64 I have no idea what apic_id means, > choose another variable name please (phys_id ?) > >> + int cpu_id; >> + >> + if (!map_processor(handle, &apic_id, &cpu_id)) >> + return AE_ERROR; >> + >> + acpi_map_cpu2node(handle, cpu_id, apic_id); >> + return AE_OK; >> +} >> + >> +void __init acpi_set_processor_mapping(void) >> +{ >> + /* Set persistent cpu <-> node mapping for all processors. */ >> + acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, >> + ACPI_UINT32_MAX, set_processor_node_mapping, >> + NULL, NULL, NULL); >> +} >> +#endif >> + >> #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC >> static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base, >> u64 *phys_addr, int *ioapic_id) >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >> index 06ed7e5..ad9e7c7 100644 >> --- a/include/linux/acpi.h >> +++ b/include/linux/acpi.h >> @@ -265,6 +265,12 @@ static inline bool invalid_phys_cpuid(phys_cpuid_t phys_id) >> /* Arch dependent functions for cpu hotplug support */ >> int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu); >> int acpi_unmap_cpu(int cpu); >> +#if defined(CONFIG_X86) >> +void acpi_map_cpu2node(acpi_handle handle, int cpu, int physid); >> +#elif defined(CONFIG_IA64) >> +int acpi_map_cpu2node(acpi_handle handle, int cpu, int physid); >> +#endif > We do not need this per-arch ifdeffery unless I am missing something > obvious here, either you change the IA64 prototype or X86 one and > declare one prototype for all arches, either will do given that return > value is always ignored (I think changing x86 to return an int is > advisable). > > Lorenzo > > > . >