Re: [PATCH] perf/x86/intel/p4: Fix unused variable warning in p4_pmu_init()
From: Aldo Conte
Date: Thu Mar 19 2026 - 18:21:47 EST
On 3/19/26 4:30 PM, Dave Hansen wrote:
On 3/19/26 08:22, Aldo Conte wrote:Okay, your comment actually caught my attention. I looked into it further and noticed that `rdmsr` is defined as a macro in both
This patch prints the full content of Model-Specific Register
via `pr_cont` and so both the low and high part. It is also
very useful to have the contents of MSR_IA32_MISC_ENABLE
in dmesg for debugging purposes.
I'd probably just switch it over to rdmsrq():
unsigned int misc;
rdmsr(MSR_IA32_MISC_ENABLE, misc);
if (!(misc & MSR_IA32_MISC_ENABLE_EMON)) {
...
I'm kinda surprised the compiler is complaining, though. I suspect we've
got a ton of these around where one of the registers isn't used.
`arch/x86/include/asm/msr.h` and `/arch/x86/include/asm/paravirt.h`. In the first header, however, the expansion shows that high and low are evaluated as follows:
(void)((low) = (u32)__val); \
(void)((high) = (u32)(__val >> 32)); \
so with a sort of warning suppressor, whereas in the second header, the (void) is not used.
Now, the file `arch/x86/events/intel/p4.c` includes `msr.h` and thus the expansion of `rdmsr`, which in theory contains `void`.
However, scrolling through the msr.h file, you can see that if the CONFIG_PARAVIRT_XXL configuration parameter is defined, then the second header (paravirt.h) is included, which then causes rdmsr to expand to
#define rdmsr(msr, val1, val2) \
do { \
u64 _l = paravirt_read_msr(msr); \
val1 = (u32)_l; \
val2 = _l >> 32; \
} while (0)
Since there is no (void) in front of val1 and val2, this will trigger a warning.
I admit I'm a novice in this field, so what do you suggest is the best fix at this point?
Should I use the “warning suppressor" with something like
(void) val1 = (u32)_l; \
(void) val2 = _l >> 32; \
or implement the solution you suggested?