[PATCH] cpufreq: acpi-cpufreq: Optimize SMP calls in drv_write() function

[PATCH] cpufreq: acpi-cpufreq: Optimize SMP calls in drv_write() function

[Kaushlendra Kumar]

The current drv_write() implementation performs redundant work by calling
do_drv_write() on the current CPU and then issuing smp_call_function_many()
to all CPUs in the mask, including the current CPU again. This results in
duplicate MSR writes on the current CPU and unnecessary IPI overhead.

Optimize the function by:
- Executing do_drv_write() locally when current CPU is
  in the target mask
- Use a temporary mask to exclude the current CPU from
  smp_call_function_many.
- Falling back to smp_call_function_many() for the target
  CPUs when current CPU is not in the target mask

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 drivers/cpufreq/acpi-cpufreq.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 4f7f9201598d..081d2d8a13da 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -330,6 +330,7 @@ static void drv_write(struct acpi_cpufreq_data *data,
 		      const struct cpumask *mask, u32 val)
 {
 	struct acpi_processor_performance *perf = to_perf_data(data);
+	struct cpumask tmp_mask;
 	struct drv_cmd cmd = {
 		.reg = &perf->control_register,
 		.val = val,
@@ -338,10 +339,14 @@ static void drv_write(struct acpi_cpufreq_data *data,
 	int this_cpu;
 
 	this_cpu = get_cpu();
-	if (cpumask_test_cpu(this_cpu, mask))
+	if (cpumask_test_cpu(this_cpu, mask)) {
 		do_drv_write(&cmd);
+		cpumask_andnot(&tmp_mask, mask, cpumask_of(this_cpu));
+		smp_call_function_many(&tmp_mask, do_drv_write, &cmd, 1);
+	} else {
+		smp_call_function_many(mask, do_drv_write, &cmd, 1);
+	}
 
-	smp_call_function_many(mask, do_drv_write, &cmd, 1);
 	put_cpu();
 }
 
-- 
2.34.1

Re: [PATCH] cpufreq: acpi-cpufreq: Optimize SMP calls in drv_write() function

[Rafael J. Wysocki]

On Thu, Sep 4, 2025 at 10:17 AM Kaushlendra Kumar
<kaushlendra.kumar@intel.com> wrote:
>
> The current drv_write() implementation performs redundant work by calling
> do_drv_write() on the current CPU and then issuing smp_call_function_many()
> to all CPUs in the mask, including the current CPU again. This results in
> duplicate MSR writes on the current CPU and unnecessary IPI overhead.

But smp_call_function_many() doesn't run func() on the local CPU, does it?

So what do you really do in this patch?

> Optimize the function by:
> - Executing do_drv_write() locally when current CPU is
>   in the target mask
> - Use a temporary mask to exclude the current CPU from
>   smp_call_function_many.
> - Falling back to smp_call_function_many() for the target
>   CPUs when current CPU is not in the target mask
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  drivers/cpufreq/acpi-cpufreq.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 4f7f9201598d..081d2d8a13da 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -330,6 +330,7 @@ static void drv_write(struct acpi_cpufreq_data *data,
>                       const struct cpumask *mask, u32 val)
>  {
>         struct acpi_processor_performance *perf = to_perf_data(data);
> +       struct cpumask tmp_mask;
>         struct drv_cmd cmd = {
>                 .reg = &perf->control_register,
>                 .val = val,
> @@ -338,10 +339,14 @@ static void drv_write(struct acpi_cpufreq_data *data,
>         int this_cpu;
>
>         this_cpu = get_cpu();
> -       if (cpumask_test_cpu(this_cpu, mask))
> +       if (cpumask_test_cpu(this_cpu, mask)) {
>                 do_drv_write(&cmd);
> +               cpumask_andnot(&tmp_mask, mask, cpumask_of(this_cpu));
> +               smp_call_function_many(&tmp_mask, do_drv_write, &cmd, 1);
> +       } else {
> +               smp_call_function_many(mask, do_drv_write, &cmd, 1);
> +       }
>
> -       smp_call_function_many(mask, do_drv_write, &cmd, 1);
>         put_cpu();
>  }
>
> --

RE: [PATCH] cpufreq: acpi-cpufreq: Optimize SMP calls in drv_write() function

[Kumar, Kaushlendra]

> But smp_call_function_many() doesn't run func() on the local CPU, does it?
> 
> So what do you really do in this patch?

Thanks for the review feedback. I've examined the function documentation for smp_call_function_many():

/**
 * smp_call_function_many(): Run a function on a set of CPUs.
 * @mask: The set of cpus to run on (only runs on online subset).
 * @func: The function to run. This must be fast and non-blocking.
 * @info: An arbitrary pointer to pass to the function.
 * @wait: Bitmask that controls the operation. If %SCF_WAIT is set, wait
 *        (atomically) until function has completed on other CPUs. If
 *        %SCF_RUN_LOCAL is set, the function will also be run locally
 *        if the local CPU is set in the @cpumask.
 *
 * If @wait is true, then returns once @func has returned.
 *
 * You must not call this function with disabled interrupts or from a
 * hardware interrupt handler or from a bottom half handler. Preemption
 * must be disabled when calling this function.
 */

Based on this documentation, when the SCF_RUN_LOCAL flag is there and the current CPU is in the mask, it will get called on local CPU also.

Re: [PATCH] cpufreq: acpi-cpufreq: Optimize SMP calls in drv_write() function

[Rafael J. Wysocki]

On Sat, Sep 6, 2025 at 6:22 AM Kumar, Kaushlendra
<kaushlendra.kumar@intel.com> wrote:
>
> > But smp_call_function_many() doesn't run func() on the local CPU, does it?
> >
> > So what do you really do in this patch?
>
> Thanks for the review feedback. I've examined the function documentation for smp_call_function_many():
>
> /**
>  * smp_call_function_many(): Run a function on a set of CPUs.
>  * @mask: The set of cpus to run on (only runs on online subset).
>  * @func: The function to run. This must be fast and non-blocking.
>  * @info: An arbitrary pointer to pass to the function.
>  * @wait: Bitmask that controls the operation. If %SCF_WAIT is set, wait
>  *        (atomically) until function has completed on other CPUs. If
>  *        %SCF_RUN_LOCAL is set, the function will also be run locally
>  *        if the local CPU is set in the @cpumask.

@wait is not a bitmask, so the above description is not accurate.

>  *
>  * If @wait is true, then returns once @func has returned.
>  *
>  * You must not call this function with disabled interrupts or from a
>  * hardware interrupt handler or from a bottom half handler. Preemption
>  * must be disabled when calling this function.
>  */
>
> Based on this documentation, when the SCF_RUN_LOCAL flag is there and the current CPU is in the mask, it will get called on local CPU also.

This applies to smp_call_function_many_cond() which is called by
smp_call_function_many() with the mask of either SCF_WAIT or 0.

In any case, the $subject patch isn't quite right.