* Re: [PATCH] semaphore: Add might_sleep() to down_*() family [not found] <20210809021215.19991-1-nixiaoming@huawei.com> @ 2021-08-31 11:13 ` Guenter Roeck 2021-08-31 11:39 ` Hanjun Guo ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Guenter Roeck @ 2021-08-31 11:13 UTC (permalink / raw) To: Xiaoming Ni Cc: linux-kernel, peterz, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla Hi, On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: > Semaphore is sleeping lock. Add might_sleep() to down*() family > (with exception of down_trylock()) to detect atomic context sleep. > > Previously discussed with Peter Zijlstra, see link: > https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > Acked-by: Will Deacon <will@kernel.org> This patch results in the following traceback on all arm64 boots with EFI BIOS. The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. Guenter --- [ 14.048540] BUG: sleeping function called from invalid context at kernel/locking/semaphore.c:163 [ 14.048700] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 14, name: cpuhp/0 [ 14.048865] 2 locks held by cpuhp/0/14: [ 14.048943] #0: ffff8000125799b0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 [ 14.049320] #1: ffff8000125799d8 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 [ 14.049523] irq event stamp: 62 [ 14.049580] hardirqs last enabled at (61): [<ffff800010269690>] finish_task_switch.isra.0+0xd0/0x2f0 [ 14.049689] hardirqs last disabled at (62): [<ffff800010313ce8>] generic_exec_single+0x138/0x190 [ 14.049785] softirqs last enabled at (0): [<ffff8000102245d4>] copy_process+0x634/0x1af4 [ 14.049876] softirqs last disabled at (0): [<0000000000000000>] 0x0 [ 14.050299] CPU: 0 PID: 14 Comm: cpuhp/0 Not tainted 5.14.0-01100-gb91db6a0b52e #1 [ 14.050452] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 [ 14.050694] Call trace: [ 14.050753] dump_backtrace+0x0/0x19c [ 14.050839] show_stack+0x1c/0x30 [ 14.050892] dump_stack_lvl+0x9c/0xd8 [ 14.050949] dump_stack+0x1c/0x38 [ 14.050999] ___might_sleep+0x154/0x200 [ 14.051053] __might_sleep+0x54/0x90 [ 14.051106] down_timeout+0x34/0x90 [ 14.051159] acpi_os_wait_semaphore+0x68/0x9c [ 14.051218] acpi_ut_acquire_mutex+0x50/0xbc [ 14.051277] acpi_get_table+0x3c/0xc0 [ 14.051330] acpi_find_last_cache_level+0x44/0x12c [ 14.051391] _init_cache_level+0xd8/0xe4 [ 14.051446] generic_exec_single+0xf8/0x190 [ 14.051502] smp_call_function_single+0x174/0x1e0 [ 14.051561] init_cache_level+0x30/0x60 [ 14.051614] cacheinfo_cpu_online+0x28/0x840 [ 14.051675] cpuhp_invoke_callback+0x168/0x2ac [ 14.051751] cpuhp_thread_fun+0x198/0x254 [ 14.051810] smpboot_thread_fn+0x200/0x2c0 [ 14.051867] kthread+0x164/0x170 [ 14.051921] ret_from_fork+0x10/0x18 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 11:13 ` [PATCH] semaphore: Add might_sleep() to down_*() family Guenter Roeck @ 2021-08-31 11:39 ` Hanjun Guo 2021-08-31 12:20 ` Thomas Gleixner 2021-08-31 11:40 ` Peter Zijlstra 2021-08-31 12:13 ` Thomas Gleixner 2 siblings, 1 reply; 7+ messages in thread From: Hanjun Guo @ 2021-08-31 11:39 UTC (permalink / raw) To: Guenter Roeck, Xiaoming Ni Cc: linux-kernel, peterz, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Sudeep Holla, Xiongfeng Wang Hi Guenter, On 2021/8/31 19:13, Guenter Roeck wrote: > Hi, > > On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: >> Semaphore is sleeping lock. Add might_sleep() to down*() family >> (with exception of down_trylock()) to detect atomic context sleep. >> >> Previously discussed with Peter Zijlstra, see link: >> https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net >> >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> >> Acked-by: Will Deacon <will@kernel.org> > > This patch results in the following traceback on all arm64 boots with > EFI BIOS. > > The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. I Cced Xiongfeng, he sent a patch to fix this issue over a month ago: https://lore.kernel.org/linux-arm-kernel/20210720112635.38565-1-wangxiongfeng2@huawei.com/T/ and the calltrace is exactly the same as below. Sudeep, would you mind take a look again? > --- > [ 14.048540] BUG: sleeping function called from invalid context at kernel/locking/semaphore.c:163 > [ 14.048700] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 14, name: cpuhp/0 > [ 14.048865] 2 locks held by cpuhp/0/14: > [ 14.048943] #0: ffff8000125799b0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 > [ 14.049320] #1: ffff8000125799d8 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 > [ 14.049523] irq event stamp: 62 > [ 14.049580] hardirqs last enabled at (61): [<ffff800010269690>] finish_task_switch.isra.0+0xd0/0x2f0 > [ 14.049689] hardirqs last disabled at (62): [<ffff800010313ce8>] generic_exec_single+0x138/0x190 > [ 14.049785] softirqs last enabled at (0): [<ffff8000102245d4>] copy_process+0x634/0x1af4 > [ 14.049876] softirqs last disabled at (0): [<0000000000000000>] 0x0 > [ 14.050299] CPU: 0 PID: 14 Comm: cpuhp/0 Not tainted 5.14.0-01100-gb91db6a0b52e #1 > [ 14.050452] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 > [ 14.050694] Call trace: > [ 14.050753] dump_backtrace+0x0/0x19c > [ 14.050839] show_stack+0x1c/0x30 > [ 14.050892] dump_stack_lvl+0x9c/0xd8 > [ 14.050949] dump_stack+0x1c/0x38 > [ 14.050999] ___might_sleep+0x154/0x200 > [ 14.051053] __might_sleep+0x54/0x90 > [ 14.051106] down_timeout+0x34/0x90 > [ 14.051159] acpi_os_wait_semaphore+0x68/0x9c > [ 14.051218] acpi_ut_acquire_mutex+0x50/0xbc > [ 14.051277] acpi_get_table+0x3c/0xc0 > [ 14.051330] acpi_find_last_cache_level+0x44/0x12c > [ 14.051391] _init_cache_level+0xd8/0xe4 > [ 14.051446] generic_exec_single+0xf8/0x190 > [ 14.051502] smp_call_function_single+0x174/0x1e0 > [ 14.051561] init_cache_level+0x30/0x60 > [ 14.051614] cacheinfo_cpu_online+0x28/0x840 > [ 14.051675] cpuhp_invoke_callback+0x168/0x2ac > [ 14.051751] cpuhp_thread_fun+0x198/0x254 > [ 14.051810] smpboot_thread_fn+0x200/0x2c0 > [ 14.051867] kthread+0x164/0x170 > [ 14.051921] ret_from_fork+0x10/0x18 Thanks Hanjun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 11:39 ` Hanjun Guo @ 2021-08-31 12:20 ` Thomas Gleixner 0 siblings, 0 replies; 7+ messages in thread From: Thomas Gleixner @ 2021-08-31 12:20 UTC (permalink / raw) To: Hanjun Guo, Guenter Roeck, Xiaoming Ni Cc: linux-kernel, peterz, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Sudeep Holla, Xiongfeng Wang On Tue, Aug 31 2021 at 19:39, Hanjun Guo wrote: > On 2021/8/31 19:13, Guenter Roeck wrote: >> This patch results in the following traceback on all arm64 boots with >> EFI BIOS. >> >> The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. > > I Cced Xiongfeng, he sent a patch to fix this issue over a month ago: > > https://lore.kernel.org/linux-arm-kernel/20210720112635.38565-1-wangxiongfeng2@huawei.com/T/ OMG. None of this is necessary. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 11:13 ` [PATCH] semaphore: Add might_sleep() to down_*() family Guenter Roeck 2021-08-31 11:39 ` Hanjun Guo @ 2021-08-31 11:40 ` Peter Zijlstra 2021-08-31 12:13 ` Thomas Gleixner 2 siblings, 0 replies; 7+ messages in thread From: Peter Zijlstra @ 2021-08-31 11:40 UTC (permalink / raw) To: Guenter Roeck Cc: Xiaoming Ni, linux-kernel, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla On Tue, Aug 31, 2021 at 04:13:22AM -0700, Guenter Roeck wrote: > Hi, > > On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: > > Semaphore is sleeping lock. Add might_sleep() to down*() family > > (with exception of down_trylock()) to detect atomic context sleep. > > > > Previously discussed with Peter Zijlstra, see link: > > https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net > > > > Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > > Acked-by: Will Deacon <will@kernel.org> > > This patch results in the following traceback on all arm64 boots with > EFI BIOS. > > The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. > > Guenter > > --- > [ 14.048540] BUG: sleeping function called from invalid context at kernel/locking/semaphore.c:163 > [ 14.048700] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 14, name: cpuhp/0 > [ 14.048865] 2 locks held by cpuhp/0/14: > [ 14.048943] #0: ffff8000125799b0 (cpu_hotplug_lock){++++}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 > [ 14.049320] #1: ffff8000125799d8 (cpuhp_state-up){+.+.}-{0:0}, at: cpuhp_thread_fun+0x38/0x254 > [ 14.049523] irq event stamp: 62 > [ 14.049580] hardirqs last enabled at (61): [<ffff800010269690>] finish_task_switch.isra.0+0xd0/0x2f0 > [ 14.049689] hardirqs last disabled at (62): [<ffff800010313ce8>] generic_exec_single+0x138/0x190 > [ 14.049785] softirqs last enabled at (0): [<ffff8000102245d4>] copy_process+0x634/0x1af4 > [ 14.049876] softirqs last disabled at (0): [<0000000000000000>] 0x0 > [ 14.050299] CPU: 0 PID: 14 Comm: cpuhp/0 Not tainted 5.14.0-01100-gb91db6a0b52e #1 > [ 14.050452] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 > [ 14.050694] Call trace: > [ 14.050753] dump_backtrace+0x0/0x19c > [ 14.050839] show_stack+0x1c/0x30 > [ 14.050892] dump_stack_lvl+0x9c/0xd8 > [ 14.050949] dump_stack+0x1c/0x38 > [ 14.050999] ___might_sleep+0x154/0x200 > [ 14.051053] __might_sleep+0x54/0x90 > [ 14.051106] down_timeout+0x34/0x90 > [ 14.051159] acpi_os_wait_semaphore+0x68/0x9c > [ 14.051218] acpi_ut_acquire_mutex+0x50/0xbc > [ 14.051277] acpi_get_table+0x3c/0xc0 > [ 14.051330] acpi_find_last_cache_level+0x44/0x12c > [ 14.051391] _init_cache_level+0xd8/0xe4 > [ 14.051446] generic_exec_single+0xf8/0x190 > [ 14.051502] smp_call_function_single+0x174/0x1e0 This is the patch working as intended.. You simply cannot schedule with interrupts disabled as per the callchain. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 11:13 ` [PATCH] semaphore: Add might_sleep() to down_*() family Guenter Roeck 2021-08-31 11:39 ` Hanjun Guo 2021-08-31 11:40 ` Peter Zijlstra @ 2021-08-31 12:13 ` Thomas Gleixner 2021-08-31 12:34 ` Will Deacon 2021-08-31 17:42 ` Guenter Roeck 2 siblings, 2 replies; 7+ messages in thread From: Thomas Gleixner @ 2021-08-31 12:13 UTC (permalink / raw) To: Guenter Roeck, Xiaoming Ni Cc: linux-kernel, peterz, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Greg Kroah-Hartman On Tue, Aug 31 2021 at 04:13, Guenter Roeck wrote: > Hi, > > On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: >> Semaphore is sleeping lock. Add might_sleep() to down*() family >> (with exception of down_trylock()) to detect atomic context sleep. >> >> Previously discussed with Peter Zijlstra, see link: >> https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net >> >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> >> Acked-by: Will Deacon <will@kernel.org> > > This patch results in the following traceback on all arm64 boots with > EFI BIOS. That's what this change was supposed to catch :) > The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. The below should fix this. Thanks, tglx --- Subject: drivers: base: cacheinfo: Get rid of DEFINE_SMP_CALL_CACHE_FUNCTION() From: Thomas Gleixner <tglx@linutronix.de> Date: Tue, 31 Aug 2021 13:48:34 +0200 DEFINE_SMP_CALL_CACHE_FUNCTION() was usefel before the CPU hotplug rework to ensure that the cache related functions are called on the upcoming CPU because the notifier itself could run on any online CPU. The hotplug state machine guarantees that the callbacks are invoked on the upcoming CPU. So there is no need to have this SMP function call obfuscation. That indirection was missed when the hotplug notifiers were converted. This also solves the problem of ARM64 init_cache_level() invoking ACPI functions which take a semaphore in that context. That's invalid as SMP function calls run with interrupts disabled. Running it just from the callback in context of the CPU hotplug thread solves this. Reported-by: Guenter Roeck <linux@roeck-us.net> Fixes: 8571890e1513 ("arm64: Add support for ACPI based firmware tables") Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/arm64/kernel/cacheinfo.c | 7 ++----- arch/mips/kernel/cacheinfo.c | 7 ++----- arch/riscv/kernel/cacheinfo.c | 7 ++----- arch/x86/kernel/cpu/cacheinfo.c | 7 ++----- include/linux/cacheinfo.h | 18 ------------------ 5 files changed, 8 insertions(+), 38 deletions(-) --- a/arch/arm64/kernel/cacheinfo.c +++ b/arch/arm64/kernel/cacheinfo.c @@ -43,7 +43,7 @@ static void ci_leaf_init(struct cacheinf this_leaf->type = type; } -static int __init_cache_level(unsigned int cpu) +int init_cache_level(unsigned int cpu) { unsigned int ctype, level, leaves, fw_level; struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); @@ -78,7 +78,7 @@ static int __init_cache_level(unsigned i return 0; } -static int __populate_cache_leaves(unsigned int cpu) +int populate_cache_leaves(unsigned int cpu) { unsigned int level, idx; enum cache_type type; @@ -97,6 +97,3 @@ static int __populate_cache_leaves(unsig } return 0; } - -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) --- a/arch/mips/kernel/cacheinfo.c +++ b/arch/mips/kernel/cacheinfo.c @@ -17,7 +17,7 @@ do { \ leaf++; \ } while (0) -static int __init_cache_level(unsigned int cpu) +int init_cache_level(unsigned int cpu) { struct cpuinfo_mips *c = ¤t_cpu_data; struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); @@ -74,7 +74,7 @@ static void fill_cpumask_cluster(int cpu cpumask_set_cpu(cpu1, cpu_map); } -static int __populate_cache_leaves(unsigned int cpu) +int populate_cache_leaves(unsigned int cpu) { struct cpuinfo_mips *c = ¤t_cpu_data; struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); @@ -114,6 +114,3 @@ static int __populate_cache_leaves(unsig return 0; } - -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) --- a/arch/riscv/kernel/cacheinfo.c +++ b/arch/riscv/kernel/cacheinfo.c @@ -113,7 +113,7 @@ static void fill_cacheinfo(struct cachei } } -static int __init_cache_level(unsigned int cpu) +int init_cache_level(unsigned int cpu) { struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); struct device_node *np = of_cpu_device_node_get(cpu); @@ -155,7 +155,7 @@ static int __init_cache_level(unsigned i return 0; } -static int __populate_cache_leaves(unsigned int cpu) +int populate_cache_leaves(unsigned int cpu) { struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); struct cacheinfo *this_leaf = this_cpu_ci->info_list; @@ -187,6 +187,3 @@ static int __populate_cache_leaves(unsig return 0; } - -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) --- a/arch/x86/kernel/cpu/cacheinfo.c +++ b/arch/x86/kernel/cpu/cacheinfo.c @@ -985,7 +985,7 @@ static void ci_leaf_init(struct cacheinf this_leaf->priv = base->nb; } -static int __init_cache_level(unsigned int cpu) +int init_cache_level(unsigned int cpu) { struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); @@ -1014,7 +1014,7 @@ static void get_cache_id(int cpu, struct id4_regs->id = c->apicid >> index_msb; } -static int __populate_cache_leaves(unsigned int cpu) +int populate_cache_leaves(unsigned int cpu) { unsigned int idx, ret; struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); @@ -1033,6 +1033,3 @@ static int __populate_cache_leaves(unsig return 0; } - -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) --- a/include/linux/cacheinfo.h +++ b/include/linux/cacheinfo.h @@ -79,24 +79,6 @@ struct cpu_cacheinfo { bool cpu_map_populated; }; -/* - * Helpers to make sure "func" is executed on the cpu whose cache - * attributes are being detected - */ -#define DEFINE_SMP_CALL_CACHE_FUNCTION(func) \ -static inline void _##func(void *ret) \ -{ \ - int cpu = smp_processor_id(); \ - *(int *)ret = __##func(cpu); \ -} \ - \ -int func(unsigned int cpu) \ -{ \ - int ret; \ - smp_call_function_single(cpu, _##func, &ret, true); \ - return ret; \ -} - struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu); int init_cache_level(unsigned int cpu); int populate_cache_leaves(unsigned int cpu); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 12:13 ` Thomas Gleixner @ 2021-08-31 12:34 ` Will Deacon 2021-08-31 17:42 ` Guenter Roeck 1 sibling, 0 replies; 7+ messages in thread From: Will Deacon @ 2021-08-31 12:34 UTC (permalink / raw) To: Thomas Gleixner Cc: Guenter Roeck, Xiaoming Ni, linux-kernel, peterz, mingo, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Greg Kroah-Hartman On Tue, Aug 31, 2021 at 02:13:08PM +0200, Thomas Gleixner wrote: > On Tue, Aug 31 2021 at 04:13, Guenter Roeck wrote: > > On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: > >> Semaphore is sleeping lock. Add might_sleep() to down*() family > >> (with exception of down_trylock()) to detect atomic context sleep. > >> > >> Previously discussed with Peter Zijlstra, see link: > >> https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net > >> > >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > >> Acked-by: Will Deacon <will@kernel.org> > > > > This patch results in the following traceback on all arm64 boots with > > EFI BIOS. > > That's what this change was supposed to catch :) > > > The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. > > The below should fix this. > > Thanks, > > tglx > --- > Subject: drivers: base: cacheinfo: Get rid of DEFINE_SMP_CALL_CACHE_FUNCTION() > From: Thomas Gleixner <tglx@linutronix.de> > Date: Tue, 31 Aug 2021 13:48:34 +0200 > > DEFINE_SMP_CALL_CACHE_FUNCTION() was usefel before the CPU hotplug rework typo: "usefel" > to ensure that the cache related functions are called on the upcoming CPU > because the notifier itself could run on any online CPU. > > The hotplug state machine guarantees that the callbacks are invoked on the > upcoming CPU. So there is no need to have this SMP function call > obfuscation. That indirection was missed when the hotplug notifiers were > converted. > > This also solves the problem of ARM64 init_cache_level() invoking ACPI > functions which take a semaphore in that context. That's invalid as SMP > function calls run with interrupts disabled. Running it just from the > callback in context of the CPU hotplug thread solves this. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Fixes: 8571890e1513 ("arm64: Add support for ACPI based firmware tables") > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/arm64/kernel/cacheinfo.c | 7 ++----- > arch/mips/kernel/cacheinfo.c | 7 ++----- > arch/riscv/kernel/cacheinfo.c | 7 ++----- > arch/x86/kernel/cpu/cacheinfo.c | 7 ++----- > include/linux/cacheinfo.h | 18 ------------------ > 5 files changed, 8 insertions(+), 38 deletions(-) > > --- a/arch/arm64/kernel/cacheinfo.c > +++ b/arch/arm64/kernel/cacheinfo.c > @@ -43,7 +43,7 @@ static void ci_leaf_init(struct cacheinf > this_leaf->type = type; > } > > -static int __init_cache_level(unsigned int cpu) > +int init_cache_level(unsigned int cpu) > { > unsigned int ctype, level, leaves, fw_level; > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > @@ -78,7 +78,7 @@ static int __init_cache_level(unsigned i > return 0; > } > > -static int __populate_cache_leaves(unsigned int cpu) > +int populate_cache_leaves(unsigned int cpu) > { > unsigned int level, idx; > enum cache_type type; > @@ -97,6 +97,3 @@ static int __populate_cache_leaves(unsig > } > return 0; > } > - > -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) > -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) Glad to see the back of this: Acked-by: Will Deacon <will@kernel.org> Will ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] semaphore: Add might_sleep() to down_*() family 2021-08-31 12:13 ` Thomas Gleixner 2021-08-31 12:34 ` Will Deacon @ 2021-08-31 17:42 ` Guenter Roeck 1 sibling, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2021-08-31 17:42 UTC (permalink / raw) To: Thomas Gleixner Cc: Xiaoming Ni, linux-kernel, peterz, mingo, will, longman, boqun.feng, wangle6, xiaoqian9, shaolexi, linux-acpi, Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Greg Kroah-Hartman On Tue, Aug 31, 2021 at 02:13:08PM +0200, Thomas Gleixner wrote: > On Tue, Aug 31 2021 at 04:13, Guenter Roeck wrote: > > > Hi, > > > > On Mon, Aug 09, 2021 at 10:12:15AM +0800, Xiaoming Ni wrote: > >> Semaphore is sleeping lock. Add might_sleep() to down*() family > >> (with exception of down_trylock()) to detect atomic context sleep. > >> > >> Previously discussed with Peter Zijlstra, see link: > >> https://lore.kernel.org/lkml/20210806082320.GD22037@worktop.programming.kicks-ass.net > >> > >> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com> > >> Acked-by: Will Deacon <will@kernel.org> > > > > This patch results in the following traceback on all arm64 boots with > > EFI BIOS. > > That's what this change was supposed to catch :) > > > The problem is only seen with CONFIG_ACPI_PPTT=y, and thus only on arm64. > > The below should fix this. > > Thanks, > > tglx > --- > Subject: drivers: base: cacheinfo: Get rid of DEFINE_SMP_CALL_CACHE_FUNCTION() > From: Thomas Gleixner <tglx@linutronix.de> > Date: Tue, 31 Aug 2021 13:48:34 +0200 > > DEFINE_SMP_CALL_CACHE_FUNCTION() was usefel before the CPU hotplug rework > to ensure that the cache related functions are called on the upcoming CPU > because the notifier itself could run on any online CPU. > > The hotplug state machine guarantees that the callbacks are invoked on the > upcoming CPU. So there is no need to have this SMP function call > obfuscation. That indirection was missed when the hotplug notifiers were > converted. > > This also solves the problem of ARM64 init_cache_level() invoking ACPI > functions which take a semaphore in that context. That's invalid as SMP > function calls run with interrupts disabled. Running it just from the > callback in context of the CPU hotplug thread solves this. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Fixes: 8571890e1513 ("arm64: Add support for ACPI based firmware tables") > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> The warning is no longer seen with this patch applied on top of v5.14-1100-gb91db6a0b52e, and I don't see any new problems on riscv, x86/x86_64, or mips. Tested-by: Guenter Roeck <linux@roeck-us.net> Thanks, Guenter > --- > arch/arm64/kernel/cacheinfo.c | 7 ++----- > arch/mips/kernel/cacheinfo.c | 7 ++----- > arch/riscv/kernel/cacheinfo.c | 7 ++----- > arch/x86/kernel/cpu/cacheinfo.c | 7 ++----- > include/linux/cacheinfo.h | 18 ------------------ > 5 files changed, 8 insertions(+), 38 deletions(-) > > --- a/arch/arm64/kernel/cacheinfo.c > +++ b/arch/arm64/kernel/cacheinfo.c > @@ -43,7 +43,7 @@ static void ci_leaf_init(struct cacheinf > this_leaf->type = type; > } > > -static int __init_cache_level(unsigned int cpu) > +int init_cache_level(unsigned int cpu) > { > unsigned int ctype, level, leaves, fw_level; > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > @@ -78,7 +78,7 @@ static int __init_cache_level(unsigned i > return 0; > } > > -static int __populate_cache_leaves(unsigned int cpu) > +int populate_cache_leaves(unsigned int cpu) > { > unsigned int level, idx; > enum cache_type type; > @@ -97,6 +97,3 @@ static int __populate_cache_leaves(unsig > } > return 0; > } > - > -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) > -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) > --- a/arch/mips/kernel/cacheinfo.c > +++ b/arch/mips/kernel/cacheinfo.c > @@ -17,7 +17,7 @@ do { \ > leaf++; \ > } while (0) > > -static int __init_cache_level(unsigned int cpu) > +int init_cache_level(unsigned int cpu) > { > struct cpuinfo_mips *c = ¤t_cpu_data; > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > @@ -74,7 +74,7 @@ static void fill_cpumask_cluster(int cpu > cpumask_set_cpu(cpu1, cpu_map); > } > > -static int __populate_cache_leaves(unsigned int cpu) > +int populate_cache_leaves(unsigned int cpu) > { > struct cpuinfo_mips *c = ¤t_cpu_data; > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > @@ -114,6 +114,3 @@ static int __populate_cache_leaves(unsig > > return 0; > } > - > -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) > -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) > --- a/arch/riscv/kernel/cacheinfo.c > +++ b/arch/riscv/kernel/cacheinfo.c > @@ -113,7 +113,7 @@ static void fill_cacheinfo(struct cachei > } > } > > -static int __init_cache_level(unsigned int cpu) > +int init_cache_level(unsigned int cpu) > { > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > struct device_node *np = of_cpu_device_node_get(cpu); > @@ -155,7 +155,7 @@ static int __init_cache_level(unsigned i > return 0; > } > > -static int __populate_cache_leaves(unsigned int cpu) > +int populate_cache_leaves(unsigned int cpu) > { > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > struct cacheinfo *this_leaf = this_cpu_ci->info_list; > @@ -187,6 +187,3 @@ static int __populate_cache_leaves(unsig > > return 0; > } > - > -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) > -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) > --- a/arch/x86/kernel/cpu/cacheinfo.c > +++ b/arch/x86/kernel/cpu/cacheinfo.c > @@ -985,7 +985,7 @@ static void ci_leaf_init(struct cacheinf > this_leaf->priv = base->nb; > } > > -static int __init_cache_level(unsigned int cpu) > +int init_cache_level(unsigned int cpu) > { > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > > @@ -1014,7 +1014,7 @@ static void get_cache_id(int cpu, struct > id4_regs->id = c->apicid >> index_msb; > } > > -static int __populate_cache_leaves(unsigned int cpu) > +int populate_cache_leaves(unsigned int cpu) > { > unsigned int idx, ret; > struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); > @@ -1033,6 +1033,3 @@ static int __populate_cache_leaves(unsig > > return 0; > } > - > -DEFINE_SMP_CALL_CACHE_FUNCTION(init_cache_level) > -DEFINE_SMP_CALL_CACHE_FUNCTION(populate_cache_leaves) > --- a/include/linux/cacheinfo.h > +++ b/include/linux/cacheinfo.h > @@ -79,24 +79,6 @@ struct cpu_cacheinfo { > bool cpu_map_populated; > }; > > -/* > - * Helpers to make sure "func" is executed on the cpu whose cache > - * attributes are being detected > - */ > -#define DEFINE_SMP_CALL_CACHE_FUNCTION(func) \ > -static inline void _##func(void *ret) \ > -{ \ > - int cpu = smp_processor_id(); \ > - *(int *)ret = __##func(cpu); \ > -} \ > - \ > -int func(unsigned int cpu) \ > -{ \ > - int ret; \ > - smp_call_function_single(cpu, _##func, &ret, true); \ > - return ret; \ > -} > - > struct cpu_cacheinfo *get_cpu_cacheinfo(unsigned int cpu); > int init_cache_level(unsigned int cpu); > int populate_cache_leaves(unsigned int cpu); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-31 17:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210809021215.19991-1-nixiaoming@huawei.com>
2021-08-31 11:13 ` [PATCH] semaphore: Add might_sleep() to down_*() family Guenter Roeck
2021-08-31 11:39 ` Hanjun Guo
2021-08-31 12:20 ` Thomas Gleixner
2021-08-31 11:40 ` Peter Zijlstra
2021-08-31 12:13 ` Thomas Gleixner
2021-08-31 12:34 ` Will Deacon
2021-08-31 17:42 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox