| Message ID | 20250412103434.5321-1-xry111@xry111.site |
|---|---|
| State | New |
| Headers | show |
| Series | cpufreq: intel_pstate: Use raw_smp_processor_id() in hwp_get_cpu_scaling() | expand |
On Sat, 2025-04-12 at 18:34 +0800, Xi Ruoyao wrote: > Use raw_smp_processor_id() instead of plain smp_processor_id() in > hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo > Thinkpad > T14P Gen 2: > > BUG: using smp_processor_id() in preemptible [00000000] code: > swapper/0/1 > caller is hwp_get_cpu_scaling+0x7f/0xc0 > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > cpu-type") > Signed-off-by: Xi Ruoyao <xry111@xry111.site> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/intel_pstate.c > b/drivers/cpufreq/intel_pstate.c > index 4aad79d26c64..bfc20b978240 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -2209,7 +2209,7 @@ static int knl_get_turbo_pstate(int cpu) > static int hwp_get_cpu_scaling(int cpu) > { > if (hybrid_scaling_factor) { > - struct cpuinfo_x86 *c = > &cpu_data(smp_processor_id()); > + struct cpuinfo_x86 *c = > &cpu_data(raw_smp_processor_id()); > u8 cpu_type = c->topo.intel_type; > > /*
On Sunday, April 13, 2025 4:44:56 PM CEST srinivas pandruvada wrote: > On Sat, 2025-04-12 at 18:34 +0800, Xi Ruoyao wrote: > > Use raw_smp_processor_id() instead of plain smp_processor_id() in > > hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo > > Thinkpad > > T14P Gen 2: > > > > BUG: using smp_processor_id() in preemptible [00000000] code: > > swapper/0/1 > > caller is hwp_get_cpu_scaling+0x7f/0xc0 > > > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > > cpu-type") > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> It's still broken after this patch though because the function should use the cpu_data() of the target CPU and not of the CPU running the code. The patch below should fix it. === From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Subject: [PATCH v1] cpufreq: intel_pstate: Fix hwp_get_cpu_scaling() Commit b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get cpu-type") introduced two issues into hwp_get_cpu_scaling(). First, it made that function use the CPU type of the CPU running the code even though the target CPU is passed as the argument to it and second, it used smp_processor_id() for that even though hwp_get_cpu_scaling() runs in preemptible context. Fix both of these problems by simply passing "cpu" to cpu_data(). Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get cpu-type") Link: https://lore.kernel.org/linux-pm/20250412103434.5321-1-xry111@xry111.site/ Reported-by: Xi Ruoyao <xry111@xry111.site> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -2336,7 +2336,7 @@ static int hwp_get_cpu_scaling(int cpu) { if (hybrid_scaling_factor) { - struct cpuinfo_x86 *c = &cpu_data(smp_processor_id()); + struct cpuinfo_x86 *c = &cpu_data(cpu); u8 cpu_type = c->topo.intel_type; /*
On Mon, 2025-04-14 at 17:19 +0200, Rafael J. Wysocki wrote: > On Sunday, April 13, 2025 4:44:56 PM CEST srinivas pandruvada wrote: > > On Sat, 2025-04-12 at 18:34 +0800, Xi Ruoyao wrote: > > > Use raw_smp_processor_id() instead of plain smp_processor_id() in > > > hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo > > > Thinkpad > > > T14P Gen 2: > > > > > > BUG: using smp_processor_id() in preemptible [00000000] code: > > > swapper/0/1 > > > caller is hwp_get_cpu_scaling+0x7f/0xc0 > > > > > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to > > > get > > > cpu-type") > > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > It's still broken after this patch though because the function should > use the cpu_data() of the target CPU and not of the CPU running the > code. That is correct. The below patch should work. Thanks, Srinivas > > The patch below should fix it. > > === > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: [PATCH v1] cpufreq: intel_pstate: Fix hwp_get_cpu_scaling() > > Commit b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > cpu-type") introduced two issues into hwp_get_cpu_scaling(). First, > it made that function use the CPU type of the CPU running the code > even though the target CPU is passed as the argument to it and > second, > it used smp_processor_id() for that even though hwp_get_cpu_scaling() > runs in preemptible context. > > Fix both of these problems by simply passing "cpu" to cpu_data(). > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > cpu-type") > Link: > https://lore.kernel.org/linux-pm/20250412103434.5321-1-xry111@xry111.site/ > Reported-by: Xi Ruoyao <xry111@xry111.site> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/cpufreq/intel_pstate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -2336,7 +2336,7 @@ > static int hwp_get_cpu_scaling(int cpu) > { > if (hybrid_scaling_factor) { > - struct cpuinfo_x86 *c = > &cpu_data(smp_processor_id()); > + struct cpuinfo_x86 *c = &cpu_data(cpu); > u8 cpu_type = c->topo.intel_type; > > /* > > > >
On Mon, Apr 14, 2025 at 5:26 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > On Mon, 2025-04-14 at 17:19 +0200, Rafael J. Wysocki wrote: > > On Sunday, April 13, 2025 4:44:56 PM CEST srinivas pandruvada wrote: > > > On Sat, 2025-04-12 at 18:34 +0800, Xi Ruoyao wrote: > > > > Use raw_smp_processor_id() instead of plain smp_processor_id() in > > > > hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo > > > > Thinkpad > > > > T14P Gen 2: > > > > > > > > BUG: using smp_processor_id() in preemptible [00000000] code: > > > > swapper/0/1 > > > > caller is hwp_get_cpu_scaling+0x7f/0xc0 > > > > > > > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to > > > > get > > > > cpu-type") > > > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > > > It's still broken after this patch though because the function should > > use the cpu_data() of the target CPU and not of the CPU running the > > code. > > That is correct. The below patch should work. OK, I'll add an ACK from you to it then if you don't mind. > > > > The patch below should fix it. > > > > === > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Subject: [PATCH v1] cpufreq: intel_pstate: Fix hwp_get_cpu_scaling() > > > > Commit b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > > cpu-type") introduced two issues into hwp_get_cpu_scaling(). First, > > it made that function use the CPU type of the CPU running the code > > even though the target CPU is passed as the argument to it and > > second, > > it used smp_processor_id() for that even though hwp_get_cpu_scaling() > > runs in preemptible context. > > > > Fix both of these problems by simply passing "cpu" to cpu_data(). > > > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > > cpu-type") > > Link: > > https://lore.kernel.org/linux-pm/20250412103434.5321-1-xry111@xry111.site/ > > Reported-by: Xi Ruoyao <xry111@xry111.site> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/cpufreq/intel_pstate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- a/drivers/cpufreq/intel_pstate.c > > +++ b/drivers/cpufreq/intel_pstate.c > > @@ -2336,7 +2336,7 @@ > > static int hwp_get_cpu_scaling(int cpu) > > { > > if (hybrid_scaling_factor) { > > - struct cpuinfo_x86 *c = > > &cpu_data(smp_processor_id()); > > + struct cpuinfo_x86 *c = &cpu_data(cpu); > > u8 cpu_type = c->topo.intel_type; > > > > /* > > > > > > > > > >
On Mon, Apr 14, 2025 at 05:19:04PM +0200, Rafael J. Wysocki wrote: > On Sunday, April 13, 2025 4:44:56 PM CEST srinivas pandruvada wrote: > > On Sat, 2025-04-12 at 18:34 +0800, Xi Ruoyao wrote: > > > Use raw_smp_processor_id() instead of plain smp_processor_id() in > > > hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo > > > Thinkpad > > > T14P Gen 2: > > > > > > BUG: using smp_processor_id() in preemptible [00000000] code: > > > swapper/0/1 > > > caller is hwp_get_cpu_scaling+0x7f/0xc0 > > > > > > Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get > > > cpu-type") > > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > It's still broken after this patch though because the function should > use the cpu_data() of the target CPU and not of the CPU running the code. Sorry for missing that. I noticed that find_hybrid_pmu_for_cpu() doesn't take the cpu argument. Does it suffer from the same problem? init_hybrid_pmu(int cpu) { struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu); struct x86_hybrid_pmu *pmu = find_hybrid_pmu_for_cpu();
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 4aad79d26c64..bfc20b978240 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -2209,7 +2209,7 @@ static int knl_get_turbo_pstate(int cpu) static int hwp_get_cpu_scaling(int cpu) { if (hybrid_scaling_factor) { - struct cpuinfo_x86 *c = &cpu_data(smp_processor_id()); + struct cpuinfo_x86 *c = &cpu_data(raw_smp_processor_id()); u8 cpu_type = c->topo.intel_type; /*
Use raw_smp_processor_id() instead of plain smp_processor_id() in hwp_get_cpu_scaling(), otherwise we get some errors on a Lenovo Thinkpad T14P Gen 2: BUG: using smp_processor_id() in preemptible [00000000] code: swapper/0/1 caller is hwp_get_cpu_scaling+0x7f/0xc0 Fixes: b52aaeeadfac ("cpufreq: intel_pstate: Avoid SMP calls to get cpu-type") Signed-off-by: Xi Ruoyao <xry111@xry111.site> --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)