[PATCH 0/2] coresight: Retry failed HW_ID writes

[PATCH 0/2] coresight: Retry failed HW_ID writes

[James Clark]

Extracted from the last two commits from [1] as they were only
conceptually related to that patchset, not functionally. This can be
safely applied to the driver separately.

[1] https://lore.kernel.org/linux-perf-users/20260821-james-cs-hw_id-output-failure-v1-0-9d532ddabcc3@linaro.org/

Signed-off-by: James Clark <james.clark@linaro.org>
---
James Clark (2):
      perf/core: Return errors from perf_report_aux_output_id()
      coresight: Retry failed HW_ID writes

 drivers/hwtracing/coresight/coresight-etm-perf.c | 50 +++++++++++++++---------
 include/linux/perf_event.h                       |  2 +-
 kernel/events/core.c                             |  6 ++-
 3 files changed, 36 insertions(+), 22 deletions(-)
---
base-commit: 75ba5c486ea071d44d3f61318c88a7793bc768d4
change-id: 20261001-james-cs-retry-hw_id-8f1ec3cf2db3

Best regards,
--  
James Clark <james.clark@linaro.org>

[PATCH 1/2] perf/core: Return errors from perf_report_aux_output_id()

[James Clark]

It's useful to know if this failed or not so a retry can be attempted
later. Return the error code or success.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 include/linux/perf_event.h | 2 +-
 kernel/events/core.c       | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 48d851fbd8ea..01d318a4de50 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1896,7 +1896,7 @@ static inline struct fasync_struct **perf_event_fasync(struct perf_event *event)
 }
 
 extern void perf_event_addr_filters_sync(struct perf_event *event);
-extern void perf_report_aux_output_id(struct perf_event *event, u64 hw_id);
+extern int perf_report_aux_output_id(struct perf_event *event, u64 hw_id);
 
 extern int perf_output_begin(struct perf_output_handle *handle,
 			     struct perf_sample_data *data,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d7f3e2c2ecb1..232da3b6f393 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10563,7 +10563,7 @@ static void perf_log_itrace_start(struct perf_event *event)
 	perf_output_end(&handle);
 }
 
-void perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
+int perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
 {
 	struct perf_output_handle handle;
 	struct perf_sample_data sample;
@@ -10585,12 +10585,14 @@ void perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
 	ret = perf_output_begin(&handle, &sample, event, rec.header.size);
 
 	if (ret)
-		return;
+		return ret;
 
 	perf_output_put(&handle, rec);
 	perf_event__output_id_sample(event, &handle, &sample);
 
 	perf_output_end(&handle);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(perf_report_aux_output_id);
 

-- 
2.43.0

[PATCH 2/2] coresight: Retry failed HW_ID writes

[James Clark]

Don't set the aux_hwid_done bit when writes fail so that it can be
retried later if the ring buffer was full. Add a second attempt on stop,
which will always succeed if the stop's AUX output also has enough space
to be written. If there is no AUX record then there will be no decoding,
so the missing HW_ID isn't important.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 50 +++++++++++++++---------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 09b21a711a87..9238a12628fb 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -536,6 +536,31 @@ static int etm_event_resume(struct coresight_path *path)
 	return ret;
 }
 
+static void etm_output_hw_id(struct perf_event *event,
+			     struct etm_event_data *event_data,
+			     struct coresight_path *path,
+			     struct coresight_device *sink)
+{
+	int cpu = smp_processor_id();
+	u64 hw_id;
+
+	/*
+	 * output cpu / trace ID in perf record, once for the lifetime
+	 * of the event.
+	 */
+	if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
+		hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,
+				CS_AUX_HW_ID_MAJOR_VERSION);
+		hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,
+				CS_AUX_HW_ID_MINOR_VERSION);
+		hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id);
+		hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));
+
+		if (!perf_report_aux_output_id(event, hw_id))
+			cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
+	}
+}
+
 static void etm_event_start(struct perf_event *event, int flags)
 {
 	int cpu = smp_processor_id();
@@ -544,7 +569,6 @@ static void etm_event_start(struct perf_event *event, int flags)
 	struct perf_output_handle *handle = &ctxt->handle;
 	struct coresight_device *source, *sink;
 	struct coresight_path *path;
-	u64 hw_id;
 
 	if (flags & PERF_EF_RESUME) {
 		path = etm_event_get_ctxt_path(ctxt);
@@ -596,22 +620,7 @@ static void etm_event_start(struct perf_event *event, int flags)
 	if (coresight_enable_source(source, event, CS_MODE_PERF, path))
 		goto fail_disable_path;
 
-	/*
-	 * output cpu / trace ID in perf record, once for the lifetime
-	 * of the event.
-	 */
-	if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) {
-		cpumask_set_cpu(cpu, &event_data->aux_hwid_done);
-
-		hw_id = FIELD_PREP(CS_AUX_HW_ID_MAJOR_VERSION_MASK,
-				CS_AUX_HW_ID_MAJOR_VERSION);
-		hw_id |= FIELD_PREP(CS_AUX_HW_ID_MINOR_VERSION_MASK,
-				CS_AUX_HW_ID_MINOR_VERSION);
-		hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, path->trace_id);
-		hw_id |= FIELD_PREP(CS_AUX_HW_ID_SINK_ID_MASK, coresight_get_sink_id(sink));
-
-		perf_report_aux_output_id(event, hw_id);
-	}
+	etm_output_hw_id(event, event_data, path, sink);
 
 out:
 	/* Tell the perf core the event is alive */
@@ -679,6 +688,7 @@ static void etm_event_pause(struct coresight_path *path,
 		if (!size)
 			return;
 
+		etm_output_hw_id(event, event_data, path, sink);
 		perf_aux_output_end(handle, size);
 		perf_aux_output_begin(handle, event);
 	} else {
@@ -769,10 +779,12 @@ static void etm_event_stop(struct perf_event *event, int mode)
 		 * But we should never get a non-zero size with
 		 * an invalid handle.
 		 */
-		if (READ_ONCE(handle->event))
+		if (READ_ONCE(handle->event)) {
+			etm_output_hw_id(event, event_data, path, sink);
 			perf_aux_output_end(handle, size);
-		else
+		} else {
 			WARN_ON(size);
+		}
 	}
 
 	/* Disabling the path make its elements available to other sessions */

-- 
2.43.0

Re: [PATCH 1/2] perf/core: Return errors from perf_report_aux_output_id()

[Suzuki K Poulose]

Hi Peter, Arnaldo, Mark, Ingo et al



On 01/10/2026 15:21, James Clark wrote:
> It's useful to know if this failed or not so a retry can be attempted
> later. Return the error code or success.
> 
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>   include/linux/perf_event.h | 2 +-
>   kernel/events/core.c       | 6 ++++--
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 48d851fbd8ea..01d318a4de50 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1896,7 +1896,7 @@ static inline struct fasync_struct **perf_event_fasync(struct perf_event *event)
>   }
>   
>   extern void perf_event_addr_filters_sync(struct perf_event *event);
> -extern void perf_report_aux_output_id(struct perf_event *event, u64 hw_id);
> +extern int perf_report_aux_output_id(struct perf_event *event, u64 hw_id);

This looks a trivial change with minor impact. Are you happy for me to
pull this via the coresight tree ? Please could I have an Ack on this ?


Cheers
Suzuki
>   
>   extern int perf_output_begin(struct perf_output_handle *handle,
>   			     struct perf_sample_data *data,
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index d7f3e2c2ecb1..232da3b6f393 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10563,7 +10563,7 @@ static void perf_log_itrace_start(struct perf_event *event)
>   	perf_output_end(&handle);
>   }
>   
> -void perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
> +int perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
>   {
>   	struct perf_output_handle handle;
>   	struct perf_sample_data sample;
> @@ -10585,12 +10585,14 @@ void perf_report_aux_output_id(struct perf_event *event, u64 hw_id)
>   	ret = perf_output_begin(&handle, &sample, event, rec.header.size);
>   
>   	if (ret)
> -		return;
> +		return ret;
>   
>   	perf_output_put(&handle, rec);
>   	perf_event__output_id_sample(event, &handle, &sample);
>   
>   	perf_output_end(&handle);
> +
> +	return 0;
>   }
>   EXPORT_SYMBOL_GPL(perf_report_aux_output_id);
>   
>