Thread (7 messages) 7 messages, 1 author, 4d ago
WARM3d

[PATCH V3 2/6] powerpc/perf: Reject duplicate HTM target reservations

From: Athira Rajeev <hidden>
Date: 2026-07-25 07:00:14
Also in: linux-perf-users
Subsystem: linux for powerpc (32-bit and 64-bit), the rest · Maintainers: Madhavan Srinivasan, Michael Ellerman, Linus Torvalds

HTM tracing is controlled through hypervisor calls and operates on a
system-scoped target identified by HTM type, node index, chip index,
and core index.

HTM events must be opened with cpu=N to pin the AUX buffer file
descriptor to a specific CPU.  The intended usage is:

  perf record -e htm/nodeindex=0,nodalchipindex=2,htm_type=1,cpu=8/ ...

With cpu=N, the event is opened on exactly one CPU and
perf_event_open() is called once for that event.  Multiple HTM events
for different targets (node/chip/core tuples) may be opened
simultaneously on different CPUs.

However, two independent perf_event_open() calls can still target the
same (node, chip, core, type) tuple from different CPUs or processes.
Without driver-side target tracking, both opens succeed htm_event_init()
independently and both proceed to htm_event_add(), where they issue
duplicate H_HTM_OP_CONFIGURE and H_HTM_OP_START hcalls against the same
hardware resource.  This causes conflicts in the underlying H_HTM
operations.

Track reserved HTM targets globally and reject duplicate reservations
for the same target.  The reservation is created during htm_event_init()
and released through the event destroy path.  This prevents concurrent
duplicate opens of the same hardware target while still allowing
different targets to be used simultaneously on different CPUs.

Returning -EBUSY from htm_event_init() for a duplicate open is
intentional and correct.  A user who mistakenly opens the same HTM
target twice (or runs perf record without cpu=N, causing every online
CPU to attempt an open of the same target) receives a clear "PMU
counters are busy" error from the perf tool, directing them to add the
required cpu=N qualifier.  Opening the same HTM node/chip/core target
from multiple CPUs simultaneously has no meaningful purpose: HTM
hardware tracing operates on the target itself, not on the CPU that
issued the hcall.

Extend the existing per-event htm_target_id structure with a list node,
and use the stored htm_config in pmu_private for target comparison.

A cpumask-based approach was considered but not used: cpumask restricts
which CPUs an event may be opened on, but HTM operates on a hardware
target (node/chip/core) that is independent of the CPU opening the
event.  A user may open an HTM event for a specific node/chip/core
target from any CPU in the system, not just CPUs that belong to that
node.  A cpumask would therefore either over-restrict valid opens or
require a per-target mask that mirrors the target list anyway.  The
approach in this patch handles the real constraint: the same hardware
target cannot be configured twice, regardless of which CPU does the
event open.

Signed-off-by: Athira Rajeev <redacted>
---
Changes in V3:
- Commit message rewritten to clarify the intended usage model:
  HTM events must be opened with cpu=N to pin the AUX buffer fd to
  a specific CPU. V2 framed the problem as a system-wide perf record
  -a race; V3 makes the cpu=N requirement and the explicit duplicate
  open scenario the primary motivation.
- Added explanation that -EBUSY from htm_event_init() is intentional:
  the user receives a clear "PMU counters are busy" error directing
  them to add cpu=N.
- No functional change to the driver code in this patch.

Changes in V2:
- New patch.  V1 did not protect against concurrent duplicate opens of
  the same HTM target when 'perf record -a' initialises system-wide
  events in parallel on all CPUs.
- Adds a global reserved-targets list.  htm_event_init() rejects any
  open whose (node, chip, core, type) tuple is already reserved; the
  reservation is released through the event destroy path.
- Different targets can still be opened simultaneously on different CPUs.

 arch/powerpc/perf/htm-perf.c | 42 ++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/perf/htm-perf.c b/arch/powerpc/perf/htm-perf.c
index 4e4c924ecfd0..84a5601ee7f7 100644
--- a/arch/powerpc/perf/htm-perf.c
+++ b/arch/powerpc/perf/htm-perf.c
@@ -77,9 +77,13 @@ struct htm_config {
  * htm_event_start() and htm_event_stop() to make hcall decisions.
  * event->hw.state is kept in sync for the perf core only.
  */
+static LIST_HEAD(htm_active_targets_list);
+static DEFINE_MUTEX(htm_targets_lock);
+
 struct htm_target_id {
 	struct htm_config cfg;
 	int tracing_active;		/* HTM_TRACING_ACTIVE / HTM_TRACING_INACTIVE */
+	struct list_head list;
 };
 
 /* Helper to parse the 28-bit event config into distinct fields */
@@ -155,7 +159,17 @@ static ssize_t htm_return_check(int rc)
 
 static void reset_htm_active(struct perf_event *event)
 {
-	kfree(event->pmu_private);
+	struct htm_target_id *target = event->pmu_private;
+
+	if (!target)
+		return;
+
+	mutex_lock(&htm_targets_lock);
+	if (!list_empty(&target->list))
+		list_del(&target->list);
+	mutex_unlock(&htm_targets_lock);
+
+	kfree(target);
 	event->pmu_private = NULL;
 }
 
@@ -163,6 +177,7 @@ static int htm_event_init(struct perf_event *event)
 {
 	u64 config = event->attr.config;
 	struct htm_config cfg;
+	struct htm_target_id *target, *tmp;
 
 	if (event->attr.inherit)
 		return -EOPNOTSUPP;
@@ -189,11 +204,30 @@ static int htm_event_init(struct perf_event *event)
 	}
 
 	/* Allocate per-event private state; freed via event->destroy */
-	event->pmu_private = kzalloc(sizeof(struct htm_target_id), GFP_KERNEL);
-	if (!event->pmu_private)
+	target = kzalloc(sizeof(*target), GFP_KERNEL);
+	if (!target)
 		return -ENOMEM;
 
-	((struct htm_target_id *)event->pmu_private)->cfg = cfg;
+	target->cfg = cfg;
+	target->tracing_active = HTM_TRACING_INACTIVE;
+	INIT_LIST_HEAD(&target->list);
+
+	mutex_lock(&htm_targets_lock);
+	list_for_each_entry(tmp, &htm_active_targets_list, list) {
+		if (tmp->cfg.htmtype == cfg.htmtype &&
+			tmp->cfg.nodeindex == cfg.nodeindex &&
+			tmp->cfg.nodalchipindex == cfg.nodalchipindex &&
+			tmp->cfg.coreindexonchip == cfg.coreindexonchip) {
+			mutex_unlock(&htm_targets_lock);
+			kfree(target);
+			return -EBUSY;
+		}
+	}
+
+	list_add_tail(&target->list, &htm_active_targets_list);
+	mutex_unlock(&htm_targets_lock);
+
+	event->pmu_private = target;
 	event->destroy = reset_htm_active;
 	return 0;
 }
-- 
2.43.0

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help