Re: [PATCH] powercap: intel_rapl: Fix rapl_packages list corruption

[Sumeet Pawnikar]

Added linux-pm mailing list now. Earlier by mistake I sent it to acpi
mailing list.


On Sat, Aug 29, 2026 at 10:13 PM Sumeet Pawnikar <sumeet4linux@xxxxxxxxx> wrote:
>
> The rapl_packages list was previously documented as being guarded by the
> CPU hotplug lock (cpus_read_lock()). However, cpus_read_lock() is a
> reader-writer semaphore taken for shared read access. This provides zero
> mutual exclusion between multiple concurrent callers.
>
> Because auxiliary device probes (such as intel_rapl_tpmi_probe) can
> execute concurrently on different CPUs, multiple packages can race on
> list operations (list_add and list_del) inside
> rapl_add_package_cpuslocked() and rapl_remove_package_cpuslocked(),
> leading to list corruption.
>
> Introduce a dedicated rapl_packages_lock mutex to protect all additions,
> removals and iterations of the rapl_packages list.
>
> Signed-off-by: Sumeet Pawnikar <sumeet4linux@xxxxxxxxx>
> ---
>  drivers/powercap/intel_rapl_common.c | 29 ++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
> index 70cab5f08bc6..e544629dbe30 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -175,7 +175,8 @@ static u64 rapl_unit_xlate(struct rapl_domain *rd,
>                            enum unit_type type, u64 value, int to_raw);
>  static void package_power_limit_irq_save(struct rapl_package *rp);
>
> -static LIST_HEAD(rapl_packages);       /* guarded by CPU hotplug lock */
> +static DEFINE_MUTEX(rapl_packages_lock);
> +static LIST_HEAD(rapl_packages);       /* guarded by rapl_packages_lock */
>
>  static const char *const rapl_domain_names[] = {
>         "package",
> @@ -1360,12 +1361,14 @@ static int rapl_pmu_event_init(struct perf_event *event)
>                 return -EINVAL;
>
>         /* Find out which Package the event belongs to */
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(pos, &rapl_packages, plist) {
>                 if (is_rp_pmu_cpu(pos, event->cpu)) {
>                         rp = pos;
>                         break;
>                 }
>         }
> +       mutex_unlock(&rapl_packages_lock);
>         if (!rp)
>                 return -ENODEV;
>
> @@ -1445,9 +1448,11 @@ static ssize_t cpumask_show(struct device *dev,
>         cpumask_clear(cpu_mask);
>
>         /* Choose a cpu for each RAPL Package */
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(rp, &rapl_packages, plist) {
>                 set_pmu_cpumask(rp, cpu_mask);
>         }
> +       mutex_unlock(&rapl_packages_lock);
>         cpus_read_unlock();
>
>         ret = sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpu_mask));
> @@ -1655,11 +1660,15 @@ void rapl_package_remove_pmu_locked(struct rapl_package *rp)
>         if (!rp->has_pmu)
>                 return;
>
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(pos, &rapl_packages, plist) {
>                 /* PMU is still needed */
> -               if (pos->has_pmu && pos != rp)
> +               if (pos->has_pmu && pos != rp) {
> +                       mutex_unlock(&rapl_packages_lock);
>                         return;
> +               }
>         }
> +       mutex_unlock(&rapl_packages_lock);
>
>         if (rapl_pmu.registered)
>                 perf_pmu_unregister(&rapl_pmu.pmu);
> @@ -1704,7 +1713,9 @@ void rapl_remove_package_cpuslocked(struct rapl_package *rp)
>         /* do parent zone last */
>         powercap_unregister_zone(rp->priv->control_type,
>                                  &rd_package->power_zone);
> +       mutex_lock(&rapl_packages_lock);
>         list_del(&rp->plist);
> +       mutex_unlock(&rapl_packages_lock);
>         kfree(rp);
>  }
>  EXPORT_SYMBOL_NS_GPL(rapl_remove_package_cpuslocked, "INTEL_RAPL");
> @@ -1749,11 +1760,15 @@ struct rapl_package *rapl_find_package_domain_cpuslocked(int id, struct rapl_if_
>         else
>                 uid = id;
>
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(rp, &rapl_packages, plist) {
> -               if (rp->id == uid
> -                   && rp->priv->control_type == priv->control_type)
> +               if (rp->id == uid &&
> +                   rp->priv->control_type == priv->control_type) {
> +                       mutex_unlock(&rapl_packages_lock);
>                         return rp;
> +               }
>         }
> +       mutex_unlock(&rapl_packages_lock);
>
>         return NULL;
>  }
> @@ -1810,7 +1825,9 @@ struct rapl_package *rapl_add_package_cpuslocked(int id, struct rapl_if_priv *pr
>         ret = rapl_package_register_powercap(rp);
>         if (!ret) {
>                 INIT_LIST_HEAD(&rp->plist);
> +               mutex_lock(&rapl_packages_lock);
>                 list_add(&rp->plist, &rapl_packages);
> +               mutex_unlock(&rapl_packages_lock);
>                 return rp;
>         }
>
> @@ -1835,6 +1852,7 @@ static void power_limit_state_save(void)
>         int ret, i;
>
>         cpus_read_lock();
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(rp, &rapl_packages, plist) {
>                 if (!rp->power_zone)
>                         continue;
> @@ -1846,6 +1864,7 @@ static void power_limit_state_save(void)
>                                 rd->rpl[i].last_power_limit = 0;
>                 }
>         }
> +       mutex_unlock(&rapl_packages_lock);
>         cpus_read_unlock();
>  }
>
> @@ -1856,6 +1875,7 @@ static void power_limit_state_restore(void)
>         int i;
>
>         cpus_read_lock();
> +       mutex_lock(&rapl_packages_lock);
>         list_for_each_entry(rp, &rapl_packages, plist) {
>                 if (!rp->power_zone)
>                         continue;
> @@ -1865,6 +1885,7 @@ static void power_limit_state_restore(void)
>                                 rapl_write_pl_data(rd, i, PL_LIMIT,
>                                                rd->rpl[i].last_power_limit);
>         }
> +       mutex_unlock(&rapl_packages_lock);
>         cpus_read_unlock();
>  }
>
> --
> 2.43.0
>