| Message ID | 20241028105200.1205509-1-pierre.gondois@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: CPPC: Make rmw_lock a raw_spin_lock | expand |
On Mon, Oct 28, 2024 at 11:52 AM Pierre Gondois <pierre.gondois@arm.com> wrote: > > The following BUG was triggered. sugov_update_shared() locks a > raw_spinlock while cpc_write() locks a spinlock. To have a correct > wait-type order, update rmw_lock to a raw_spinlock. > > Also save irq state. > > ============================= > [ BUG: Invalid wait context ] > 6.12.0-rc2-XXX #406 Not tainted > ----------------------------- > kworker/1:1/62 is trying to lock: > ffffff8801593030 (&cpc_ptr->rmw_lock){+.+.}-{3:3}, at: cpc_write+0xcc/0x370 > other info that might help us debug this: > context-{5:5} > 2 locks held by kworker/1:1/62: > #0: ffffff897ef5ec98 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x2c/0x50 > #1: ffffff880154e238 (&sg_policy->update_lock){....}-{2:2}, at: sugov_update_shared+0x3c/0x280 > stack backtrace: > CPU: 1 UID: 0 PID: 62 Comm: kworker/1:1 Not tainted 6.12.0-rc2-g9654bd3e8806 #406 > Workqueue: 0x0 (events) > Call trace: > dump_backtrace+0xa4/0x130 > show_stack+0x20/0x38 > dump_stack_lvl+0x90/0xd0 > dump_stack+0x18/0x28 > __lock_acquire+0x480/0x1ad8 > lock_acquire+0x114/0x310 > _raw_spin_lock+0x50/0x70 > cpc_write+0xcc/0x370 > cppc_set_perf+0xa0/0x3a8 > cppc_cpufreq_fast_switch+0x40/0xc0 > cpufreq_driver_fast_switch+0x4c/0x218 > sugov_update_shared+0x234/0x280 > update_load_avg+0x6ec/0x7b8 > dequeue_entities+0x108/0x830 > dequeue_task_fair+0x58/0x408 > __schedule+0x4f0/0x1070 > schedule+0x54/0x130 > worker_thread+0xc0/0x2e8 > kthread+0x130/0x148 > ret_from_fork+0x10/0x20 > > Fixes: 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/acpi/cppc_acpi.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > index 1a40f0514eaa..e7e4bf932e28 100644 > --- a/drivers/acpi/cppc_acpi.c > +++ b/drivers/acpi/cppc_acpi.c > @@ -1127,7 +1127,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) > return -ENODEV; > } > > - raw_spin_lock_irqsave(&cpc_desc->rmw_lock, flags); > + raw_spin_lock(&cpc_desc->rmw_lock); This won't apply because the current code is spin_lock(&cpc_desc->rmw_lock); and analogously below. I think that it needs to be raw_spin_lock_irqsave(), though. Besides, a raw spin lock needs to be declared as such. > switch (size) { > case 8: > prev_val = readb_relaxed(vaddr); > @@ -1142,7 +1142,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) > prev_val = readq_relaxed(vaddr); > break; > default: > - raw_spin_unlock_irqrestore(&cpc_desc->rmw_lock, flags); > + raw_spin_unlock(&cpc_desc->rmw_lock); > return -EFAULT; > } > val = MASK_VAL_WRITE(reg, prev_val, val); > @@ -1175,7 +1175,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) > } > > if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > - raw_spin_unlock_irqrestore(&cpc_desc->rmw_lock, flags); > + raw_spin_unlock(&cpc_desc->rmw_lock); > > return ret_val; > } > -- > 2.25.1 >
On Mon, Oct 28, 2024 at 11:51:49AM +0100, Pierre Gondois wrote: > The following BUG was triggered. sugov_update_shared() locks a > raw_spinlock while cpc_write() locks a spinlock. To have a correct > wait-type order, update rmw_lock to a raw_spinlock. > > Also save irq state. > > ============================= > [ BUG: Invalid wait context ] > 6.12.0-rc2-XXX #406 Not tainted > ----------------------------- > kworker/1:1/62 is trying to lock: > ffffff8801593030 (&cpc_ptr->rmw_lock){+.+.}-{3:3}, at: cpc_write+0xcc/0x370 > other info that might help us debug this: > context-{5:5} > 2 locks held by kworker/1:1/62: > #0: ffffff897ef5ec98 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x2c/0x50 > #1: ffffff880154e238 (&sg_policy->update_lock){....}-{2:2}, at: sugov_update_shared+0x3c/0x280 > stack backtrace: > CPU: 1 UID: 0 PID: 62 Comm: kworker/1:1 Not tainted 6.12.0-rc2-g9654bd3e8806 #406 > Workqueue: 0x0 (events) > Call trace: > dump_backtrace+0xa4/0x130 > show_stack+0x20/0x38 > dump_stack_lvl+0x90/0xd0 > dump_stack+0x18/0x28 > __lock_acquire+0x480/0x1ad8 > lock_acquire+0x114/0x310 > _raw_spin_lock+0x50/0x70 > cpc_write+0xcc/0x370 > cppc_set_perf+0xa0/0x3a8 > cppc_cpufreq_fast_switch+0x40/0xc0 > cpufreq_driver_fast_switch+0x4c/0x218 > sugov_update_shared+0x234/0x280 > update_load_avg+0x6ec/0x7b8 > dequeue_entities+0x108/0x830 > dequeue_task_fair+0x58/0x408 > __schedule+0x4f0/0x1070 > schedule+0x54/0x130 > worker_thread+0xc0/0x2e8 > kthread+0x130/0x148 > ret_from_fork+0x10/0x20 > > Fixes: 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") > Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> > --- > drivers/acpi/cppc_acpi.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - You have marked a patch with a "Fixes:" tag for a commit that is in an older released kernel, yet you do not have a cc: stable line in the signed-off-by area at all, which means that the patch will not be applied to any older kernel releases. To properly fix this, please follow the documented rules in the Documentation/process/stable-kernel-rules.rst file for how to resolve this. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 1a40f0514eaa..e7e4bf932e28 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -1127,7 +1127,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) return -ENODEV; } - raw_spin_lock_irqsave(&cpc_desc->rmw_lock, flags); + raw_spin_lock(&cpc_desc->rmw_lock); switch (size) { case 8: prev_val = readb_relaxed(vaddr); @@ -1142,7 +1142,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) prev_val = readq_relaxed(vaddr); break; default: - raw_spin_unlock_irqrestore(&cpc_desc->rmw_lock, flags); + raw_spin_unlock(&cpc_desc->rmw_lock); return -EFAULT; } val = MASK_VAL_WRITE(reg, prev_val, val); @@ -1175,7 +1175,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) } if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) - raw_spin_unlock_irqrestore(&cpc_desc->rmw_lock, flags); + raw_spin_unlock(&cpc_desc->rmw_lock); return ret_val; }
The following BUG was triggered. sugov_update_shared() locks a raw_spinlock while cpc_write() locks a spinlock. To have a correct wait-type order, update rmw_lock to a raw_spinlock. Also save irq state. ============================= [ BUG: Invalid wait context ] 6.12.0-rc2-XXX #406 Not tainted ----------------------------- kworker/1:1/62 is trying to lock: ffffff8801593030 (&cpc_ptr->rmw_lock){+.+.}-{3:3}, at: cpc_write+0xcc/0x370 other info that might help us debug this: context-{5:5} 2 locks held by kworker/1:1/62: #0: ffffff897ef5ec98 (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x2c/0x50 #1: ffffff880154e238 (&sg_policy->update_lock){....}-{2:2}, at: sugov_update_shared+0x3c/0x280 stack backtrace: CPU: 1 UID: 0 PID: 62 Comm: kworker/1:1 Not tainted 6.12.0-rc2-g9654bd3e8806 #406 Workqueue: 0x0 (events) Call trace: dump_backtrace+0xa4/0x130 show_stack+0x20/0x38 dump_stack_lvl+0x90/0xd0 dump_stack+0x18/0x28 __lock_acquire+0x480/0x1ad8 lock_acquire+0x114/0x310 _raw_spin_lock+0x50/0x70 cpc_write+0xcc/0x370 cppc_set_perf+0xa0/0x3a8 cppc_cpufreq_fast_switch+0x40/0xc0 cpufreq_driver_fast_switch+0x4c/0x218 sugov_update_shared+0x234/0x280 update_load_avg+0x6ec/0x7b8 dequeue_entities+0x108/0x830 dequeue_task_fair+0x58/0x408 __schedule+0x4f0/0x1070 schedule+0x54/0x130 worker_thread+0xc0/0x2e8 kthread+0x130/0x148 ret_from_fork+0x10/0x20 Fixes: 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> --- drivers/acpi/cppc_acpi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)