[PATCH V5 3/6] tools/perf: Add arch hook to drain remaining data before event close

While collecting samples using perf record, __cmd_record() disables the
evlist once recording is complete. After that, event fds are no longer
read and any remaining PMU-specific data cannot be drained.

Add a weak arch_perf_record__need_read() hook so architecture code can
indicate that more data remains to be collected before events are
disabled and closed. When the hook reports pending data, perf record
performs another read pass via record__final_data().

A static volatile sig_atomic_t drain_interrupted flag is added.
sig_handler() sets it when a second signal (SIGINT/SIGTERM) arrives
while done is already set, allowing a second Ctrl+C during the drain
loop to abort immediately.  Without this, a user who presses Ctrl+C
twice while HTM data is being drained would have no way to interrupt
a stalled drain loop.

This allows architectures such as powerpc HTM to drain trace data and
associated metadata before the event is closed.

Signed-off-by: Athira Rajeev <redacted>
---
Changes in V5:
- Rename record__final_data() to record__final_aux_data() to make the
  AUX-specific purpose explicit.
- Guard both call sites with if (rec->opts.full_auxtrace) so the
  function is only entered when AUX tracing is actually active.
- Remove the per-thread TLS redirect loop and 'int t' variable from
  record__final_aux_data().  The existing record__init_thread_masks()
  path already rejects --threads combined with full_auxtrace, so
  rec->nr_threads > 1 and rec->opts.full_auxtrace cannot both be true
  at runtime; the check was redundant dead code.  A single
  record__mmap_read_all(rec, true) call on the main thread is sufficient
  and race-free.

Changes in V4:
- Fix drain_interrupted to not trigger on SIGCHLD: change the condition
  in sig_handler() from "if (done)" to "if (done && sig != SIGCHLD)".
  V3 set drain_interrupted on a SIGCHLD that arrived while done was
  already set, aborting the drain loop on normal child workload
  completion even though no second Ctrl+C was pressed.
- Drain all thread mmaps in record__final_data(): iterate over all
  rec->nr_threads slots and temporarily switch the TLS 'thread' pointer
  to each thread_data[t] before calling record__mmap_read_all(), then
  reset it to thread_data[0] afterwards.  V3 called
  record__mmap_read_all() once with whatever 'thread' pointed to,
  leaving the mmaps of worker threads un-drained when --threads is in
  use.  Adds local variable 'int t' for the loop counter.
- Fix the post-loop fallback condition from "if (!disabled)" to
  "if (target__none(&opts->target) || !disabled)" so that the final
  drain is always executed for child workloads.  In the child-workload
  path (target__none == true) the in-loop disable block is never
  entered, so disabled stays false, but the old "!disabled" test
  happened to be true there only by accident; making the intent
  explicit also handles any future path where disabled could be set
  early.  Remove the now-redundant evlist__disable() call that V3
  placed inside the post-loop block, since the disable is handled by
  the existing code below.

Changes in V3:
- Add static volatile sig_atomic_t drain_interrupted.  Set it in
  sig_handler() when a second signal arrives while done is already set.
  This lets a second Ctrl+C abort the drain loop immediately.  V2
  tested done > 1 to detect a second signal, which is unreachable
  because done is only ever set to 1.
- Replace rec->bytes_written with record__bytes_written(rec)
  (which includes rec->thread_bytes_written) so the no-progress
  check accounts for data written by the AIO/thread path.
- Add a retry counter (FINAL_DATA_MAX_RETRIES 20, 20 x 1 ms = 20 ms
  maximum) instead of aborting on the first stall.  The no-progress
  sleep of 1 ms is enough for the AUX ring buffer consumer (perf's
  ~1 ms poll interval) to advance the tail pointer.
- Move record__final_data() into the in-loop disable block
  (before evlist__disable()) as well as into a post-loop if
  (!disabled) block that covers both the early-break and
  child-workload paths.  V2 only called it inside the loop
  with a final_data_drained guard, missing the early-break
  and child-workload cases.
- Remove the now-unnecessary bool final_data_drained local variable.

Changes in V2:
-  V1's callback was responsible for driving the read loop
  including evlist__enable cycling. Removed that logic
- Use bytes written to check if session needs to be continued.
- Patch is now 3/6 instead of 3/9.

 tools/perf/builtin-record.c | 65 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/record.h    |  4 +++
 2 files changed, 69 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f58d7e3c7879..c3eb0af31142 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c

@@ -192,6 +192,7 @@ struct record {
 };
 
 static volatile int done;
+static volatile sig_atomic_t drain_interrupted;
 
 static volatile int auxtrace_record__snapshot_started;
 static DEFINE_TRIGGER(auxtrace_snapshot_trigger);

@@ -681,6 +682,8 @@ static void sig_handler(int sig)
 	else
 		signr = sig;
 
+	if (done && sig != SIGCHLD)
+		drain_interrupted = 1;
 	done = 1;
 #ifdef HAVE_EVENTFD_SUPPORT
 	if (done_fd >= 0) {

@@ -2437,6 +2440,57 @@ static unsigned long record__waking(struct record *rec)
 	return waking;
 }
 
+/*
+ * Weak arch hook called by record__final_data().
+ * Returns 1 if the arch PMU driver still has records pending (the caller
+ * will call record__mmap_read_all() and retry), 0 when done.
+ * Implementations use perf_evsel__read() so this must be called while
+ * events are still ACTIVE (before evlist__disable()).
+ */
+__weak int arch_perf_record__need_read(struct evlist *evlist __maybe_unused)
+{
+	return 0;
+}
+
+static void record__final_aux_data(struct record *rec)
+{
+	u64 last_bytes_written = 0;
+	int retries = 0;
+#define FINAL_DATA_MAX_RETRIES 20  /* 20 * 1 ms = 20 ms max wait */
+
+	/*
+	 * Drain any remaining AUX data.  Called only when full_auxtrace is
+	 * set; --threads is mutually exclusive with full_auxtrace and is
+	 * rejected at open time, so the main thread's thread_data[0] covers
+	 * all CPUs here.
+	 * arch_perf_record__need_read() calls perf_evsel__read() and
+	 * therefore requires events to still be ACTIVE (before
+	 * evlist__disable()).
+	 * A second SIGINT/SIGTERM sets drain_interrupted to abort immediately.
+	 */
+	while (arch_perf_record__need_read(rec->evlist)) {
+		if (drain_interrupted)
+			break;
+
+		last_bytes_written = record__bytes_written(rec);
+
+		if (record__mmap_read_all(rec, true) < 0)
+			return;
+
+		if (record__bytes_written(rec) == last_bytes_written) {
+			if (++retries >= FINAL_DATA_MAX_RETRIES) {
+				pr_warning("Final AUX data drain made no forward progress after %d retries.\n",
+					   FINAL_DATA_MAX_RETRIES);
+				break;
+			}
+			usleep(1000); /* 1 ms: let AUX ring buffer consumer advance */
+		} else {
+			retries = 0;
+			usleep(100);
+		}
+	}
+}
+
 static int __cmd_record(struct record *rec, int argc, const char **argv)
 {
 	int err;

@@ -2864,11 +2918,22 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		 */
 		if (done && !disabled && !target__none(&opts->target)) {
 			trigger_off(&auxtrace_snapshot_trigger);
+			if (rec->opts.full_auxtrace)
+				record__final_aux_data(rec);
 			evlist__disable(rec->evlist);
 			disabled = true;
 		}
 	}
 
+	/*
+	 * If the loop exited without entering the in-loop disable block
+	 * (early break, or child workload where target__none is true and
+	 * the block is never reached), drain any remaining AUX data now.
+	 * Events are still live at this point.
+	 */
+	if ((target__none(&opts->target) || !disabled) && rec->opts.full_auxtrace)
+		record__final_aux_data(rec);
+
 	trigger_off(&auxtrace_snapshot_trigger);
 	trigger_off(&switch_output_trigger);
 

diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 93627c9a7338..73bc3de97b2a 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h

@@ -95,4 +95,8 @@ static inline bool record_opts__no_switch_events(const struct record_opts *opts)
 	return opts->record_switch_events_set && !opts->record_switch_events;
 }
 
+struct evlist;
+struct record;
+int arch_perf_record__need_read(struct evlist *evlist);
+
 #endif // _PERF_RECORD_H

-- 
2.53.0