| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <570CABDE.7030902@cn.fujitsu.com>
Date: Tue, 12 Apr 2016 16:03:42 +0800
From: Zhu Guihua <zhugh.fnst@...fujitsu.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC: <cl@...ux.com>, <tj@...nel.org>, <mika.j.penttila@...il.com>,
<mingo@...hat.com>, <akpm@...ux-foundation.org>,
<rjw@...ysocki.net>, <hpa@...or.com>, <yasu.isimatu@...il.com>,
<isimatu.yasuaki@...fujitsu.com>, <kamezawa.hiroyu@...fujitsu.com>,
<izumi.taku@...fujitsu.com>, <gongzhaogang@...pur.com>,
<len.brown@...el.com>, <lenb@...nel.org>, <tglx@...utronix.de>,
<chen.tang@...ystack.cn>, <x86@...nel.org>,
<linux-acpi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-mm@...ck.org>, <dennis.chen@....com>
Subject: Re: [PATCH v6 5/5] x86, acpi, cpu-hotplug: Set persistent cpuid <->
nodeid mapping when booting.
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 <guz.fnst@...fujitsu.com>
>>
>> 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 <acpi/processor.h>
>>
>> -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
>
>
> .
>
Powered by blists - more mailing lists