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

[sashiko-bot@kernel.org]

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