Re: [RFC PATCH] perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] perf/x86/rapl: 64-bit division in rapl_check_hw_unit() breaks
  32-bit builds
--

commit 64815f3ce3036f1ab83ef82a1cc4dd1237aac3f9
Author: Dhruv Gupta <dhruv.gupta@amd.com>

perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

This commit raises the reference power used to compute the RAPL energy counter
overflow timer interval from 200W to 1000W and simplifies the interval
calculation in rapl_check_hw_unit().

> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
> index 8ed03c32f5605..43ac9e1c98b45 100644
> --- a/arch/x86/events/rapl.c
> +++ b/arch/x86/events/rapl.c
> @@ -106,6 +106,7 @@ static const char *const rapl_core_domain_name __initconst = "core";
>   */
>  #define RAPL_EVENT_MASK	0xFFULL
>  #define RAPL_CNTR_WIDTH 32
> +#define RAPL_TIMER_REF_WATTS 1000
>  
>  #define RAPL_EVENT_ATTR_STR(_name, v, str)					\
>  static struct perf_pmu_events_attr event_attr_##v = {				\
> @@ -639,16 +640,12 @@ static int rapl_check_hw_unit(void)
>  
>  	/*
>  	 * Calculate the timer rate:
> -	 * Use reference of 200W for scaling the timeout to avoid counter
> -	 * overflows. 200W = 200 Joules/sec
> -	 * Divide interval by 2 to avoid lockstep (2 * 100)
> -	 * if hw unit is 32, then we use 2 ms 1/200/2
> +	 * Use reference of 1000W for scaling the timeout to avoid counter
> +	 * overflows. 1000W = 1000 Joules/sec
> +	 * Divide interval by 2 to avoid lockstep (2 * RAPL_TIMER_REF_WATTS)
>  	 */
> -	rapl_timer_ms = 2;
> -	if (rapl_pkg_hw_unit[0] < 32) {
> -		rapl_timer_ms = (1000 / (2 * 100));
> -		rapl_timer_ms *= (1ULL << (32 - rapl_pkg_hw_unit[0] - 1));
> -	}
> +	rapl_timer_ms = (1000ULL << (32 - rapl_core_hw_unit)) /
> +		(2 * RAPL_TIMER_REF_WATTS);

[Severity: High]
Does this 64-bit division break 32-bit x86 builds?  Dividing the runtime
64-bit value (1000ULL << (32 - rapl_core_hw_unit)) by 2000 with the plain
/ operator emits a call to __udivdi3 on CONFIG_X86_32, causing a link
failure when CONFIG_PERF_EVENTS_INTEL_RAPL is enabled.  Using div_u64()
would avoid this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111151050.329724-1-dhruv.gupta@amd.com?part=1