Re: [Patch v3 6/7] perf/x86: Use macros to replace magic numbers in attr_rdpmc

[Ian Rogers]

On Tue, Jan 13, 2026 at 5:22 PM Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx> wrote:
>
> Replace magic numbers in attr_rdpmc with macros to improve readability
> and make their meanings clearer for users.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/events/core.c       | 7 ++++---
>  arch/x86/events/intel/p6.c   | 2 +-
>  arch/x86/events/perf_event.h | 7 +++++++
>  3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 0ecac9495d74..c2717cb5034f 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2163,7 +2163,8 @@ static int __init init_hw_perf_events(void)
>
>         pr_cont("%s PMU driver.\n", x86_pmu.name);
>
> -       x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */
> +       /* enable userspace RDPMC usage by default */
> +       x86_pmu.attr_rdpmc = X86_USER_RDPMC_CONDITIONAL_ENABLE;
>
>         for (quirk = x86_pmu.quirks; quirk; quirk = quirk->next)
>                 quirk->func();
> @@ -2643,12 +2644,12 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
>                  */
>                 if (val == 0)

nit: Could 0 here be X86_USER_RDPMC_NEVER_ENABLE to eliminate even
more magic numbers?

>                         static_branch_inc(&rdpmc_never_available_key);
> -               else if (x86_pmu.attr_rdpmc == 0)
> +               else if (x86_pmu.attr_rdpmc == X86_USER_RDPMC_NEVER_ENABLE)
>                         static_branch_dec(&rdpmc_never_available_key);
>
>                 if (val == 2)

Similarly could 2 be X86_USER_RDPMC_ALWAYS_ENABLE ?

Thanks,
Ian

>                         static_branch_inc(&rdpmc_always_available_key);
> -               else if (x86_pmu.attr_rdpmc == 2)
> +               else if (x86_pmu.attr_rdpmc == X86_USER_RDPMC_ALWAYS_ENABLE)
>                         static_branch_dec(&rdpmc_always_available_key);
>
>                 on_each_cpu(cr4_update_pce, NULL, 1);
> diff --git a/arch/x86/events/intel/p6.c b/arch/x86/events/intel/p6.c
> index 6e41de355bd8..fb991e0ac614 100644
> --- a/arch/x86/events/intel/p6.c
> +++ b/arch/x86/events/intel/p6.c
> @@ -243,7 +243,7 @@ static __init void p6_pmu_rdpmc_quirk(void)
>                  */
>                 pr_warn("Userspace RDPMC support disabled due to a CPU erratum\n");
>                 x86_pmu.attr_rdpmc_broken = 1;
> -               x86_pmu.attr_rdpmc = 0;
> +               x86_pmu.attr_rdpmc = X86_USER_RDPMC_NEVER_ENABLE;
>         }
>  }
>
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index f7caabc5d487..24a81d2916e9 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -187,6 +187,13 @@ struct amd_nb {
>          (1ULL << PERF_REG_X86_R14)   | \
>          (1ULL << PERF_REG_X86_R15))
>
> +/* user space rdpmc control values */
> +enum {
> +       X86_USER_RDPMC_NEVER_ENABLE             = 0,
> +       X86_USER_RDPMC_CONDITIONAL_ENABLE       = 1,
> +       X86_USER_RDPMC_ALWAYS_ENABLE            = 2,
> +};
> +
>  /*
>   * Per register state.
>   */
> --
> 2.34.1
>