[PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this

[PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this

[Mikhail Gavrilov]

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.

Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
 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);
 	}
 }
-- 
2.53.0

Re: [PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this

[Gabriele Monaco]

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 <gmonaco@redhat.com>

Thanks,
Gabriele

> 
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
>  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);
>  	}
>  }

[PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this

[Mikhail Gavrilov]

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. The requirement
for this variable to be unique although static exists for modules on
specific architectures (alpha) and if the kernel is built with
CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).

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.

Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
 include/rv/da_monitor.h | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..7511f5464c48 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -20,6 +20,12 @@
 #include <linux/bug.h>
 #include <linux/sched.h>
 
+/*
+ * Per-cpu variables require a unique name although static in some
+ * configurations (e.g. CONFIG_DEBUG_FORCE_WEAK_PER_CPU or alpha modules).
+ */
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
+
 static struct rv_monitor rv_this;
 
 static void react(enum states curr_state, enum events event)
@@ -183,14 +189,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 +229,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 +248,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);
 	}
 }
-- 
2.53.0

Re: [PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this

[Steven Rostedt]

FYI, please send new versions of a patch as a separate thread. Do not send
it as a reply to the first version. That makes it much more difficult for
maintainers to keep track of.

You should also add what changed since the first version:

> 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. The requirement
> for this variable to be unique although static exists for modules on
> specific architectures (alpha) and if the kernel is built with
> CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
> 
> 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.
> 
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---

Changes since v1: https://lore.kernel.org/all/20260216074727.741822-1-mikhail.v.gavrilov@gmail.com/

- <list changes here>

>  include/rv/da_monitor.h | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..7511f5464c48 10064


This way this patch has a link to the previous version. Then when this
patch gets committed, it the commit has a link to this patch, and that
patch has a link to a previous version and the chain of updates is archived
nicely.

-- Steve