Re: [PATCH 14/23] hwmon/coretemp: Use RCU to protect access of HK_TYPE_MISC cpumask

sashiko-bot
2026-04-22T03:09:03Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Mixing goto-based cleanup labels with scope-based cleanup helpers (`scoped_guard`) in the same function.
--

commit 83c63b96668998046d82d5fd34944cc02b7b7876
Author: Waiman Long <longman@redhat.com>

hwmon/coretemp: Use RCU to protect access of HK_TYPE_MISC cpumask

This patch wraps the housekeeping_cpu() check in a scoped_guard for RCU
since the HK_TYPE_MISC cpumask can change at runtime.

> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -563,8 +563,10 @@ static int create_core_data(struct platform_device *pdev, unsigned int cpu,
> u32 eax, edx;
> int err;
>
> - if (!housekeeping_cpu(cpu, HK_TYPE_MISC))
> - return 0;
> + scoped_guard(rcu) {
> + if (!housekeeping_cpu(cpu, HK_TYPE_MISC))
> + return 0;
> + }

Is it okay to use scoped_guard in a function that also relies on goto
labels for error handling?

Looking at include/linux/cleanup.h, the guidelines state that usage of
goto and cleanup helpers should never be mixed in the same function. We
should either convert all resources to scope-based cleanup or convert
none of them.

[ ... ]

> /* Test if we can access the status register */
> err = rdmsr_safe_on_cpu(cpu, tdata->status_reg, &eax, &edx);
> if (err)
> goto err;

[ ... ]

> err:
> destroy_temp_data(pdata, tdata);
> return err;
> }

Since create_core_data() still relies on the goto err label to clean up
tdata, would it be better to use explicit rcu_read_lock() and
rcu_read_unlock() around the housekeeping_cpu() check, or alternatively
convert tdata to use a cleanup helper?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=14