On Mon, 2026-02-16 at 12:47 +0500, Mikhail Gavrilov wrote:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. When multiple
> per-cpu monitors (e.g. sco and sts) are built-in simultaneously, they
> all produce the same __pcpu_unique_da_mon_this symbol, causing a link
> error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
Thanks Mikhail,
I just got a report but you were faster sending the patch!
The __pcpu_unique_ variable (and with it the requirement for per-cpu variables
to be unique although static) exists:
* for modules on specific achitectures (alpha)
* if the kernel is built with CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's
debug kernel)
Could you mention this in the commit message?
Also, it's alright using a unique name also for global monitors although not
required, but please add a comment stating why this is needed at least for CPU
variables, to avoid the next "refactoring" to change it again. Something like:
Per-cpu variables requires a unique name although static in some
configurations
Other than that, the change looks good.
Reviewed-by: Gabriele Monaco <[email protected]>
Thanks,
Gabriele
>
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Signed-off-by: Mikhail Gavrilov <[email protected]>
> ---
> include/rv/da_monitor.h | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..f6da07863ed8 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -19,6 +19,7 @@
> #include <linux/stringify.h>
> #include <linux/bug.h>
> #include <linux/sched.h>
> +#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
>
> static struct rv_monitor rv_this;
>
> @@ -183,14 +184,14 @@ static inline bool da_event(struct da_monitor *da_mon,
> struct task_struct *tsk,
> /*
> * global monitor (a single variable)
> */
> -static struct da_monitor da_mon_this;
> +static struct da_monitor DA_MON_NAME;
>
> /*
> * da_get_monitor - return the global monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return &da_mon_this;
> + return &DA_MON_NAME;
> }
>
> /*
> @@ -223,14 +224,14 @@ static inline void da_monitor_destroy(void) { }
> /*
> * per-cpu monitor variables
> */
> -static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
> +static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
>
> /*
> * da_get_monitor - return current CPU monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return this_cpu_ptr(&da_mon_this);
> + return this_cpu_ptr(&DA_MON_NAME);
> }
>
> /*
> @@ -242,7 +243,7 @@ static void da_monitor_reset_all(void)
> int cpu;
>
> for_each_cpu(cpu, cpu_online_mask) {
> - da_mon = per_cpu_ptr(&da_mon_this, cpu);
> + da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
> da_monitor_reset(da_mon);
> }
> }
