[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0g=Q0aYEZUdy+F-bmptOvLVRxAs5gzGpbgrbCoFxMpcZQ@mail.gmail.com>
Date: Mon, 28 Oct 2024 13:37:55 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Pierre Gondois <pierre.gondois@....com>
Cc: linux-kernel@...r.kernel.org, Dietmar Eggemann <dietmar.eggemann@....com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Sudeep Holla <sudeep.holla@....com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams <clrkwllms@...nel.org>,
Steven Rostedt <rostedt@...dmis.org>, Jeremy Linton <jeremy.linton@....com>,
Yunhui Cui <cuiyunhui@...edance.com>, Conor Dooley <conor.dooley@...rochip.com>,
Steffen Persvold <spersvold@...il.com>, Andrew Morton <akpm@...ux-foundation.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, "Borislav Petkov (AMD)" <bp@...en8.de>,
Reinette Chatre <reinette.chatre@...el.com>, Tony Luck <tony.luck@...el.com>,
Yury Norov <yury.norov@...il.com>, Clément Léger <cleger@...osinc.com>,
linux-arm-kernel@...ts.infradead.org, linux-riscv@...ts.infradead.org,
linux-acpi@...r.kernel.org, linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH] ACPI: CPPC: Make rmw_lock a raw_spin_lock
On Mon, Oct 28, 2024 at 11:52 AM Pierre Gondois <pierre.gondois@....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@....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
>
Powered by blists - more mailing lists