On 07/08/2026 17:41, Athira Rajeev wrote:
> Implement the arch_perf_record__need_read() architecture-specific hook
> for powerpc in arch/powerpc/util/evsel.c.
>
> The HTM kernel driver sets event->count to the number of records still
> staged in its internal buffers (total_size / record_size), and to 0
> once the stream is exhausted. This hook reads that count for every open
> htm evsel via perf_evsel__read() and accumulates the values into
> total_pending_records. A non-zero total means at least one HTM target
> still has records pending; the recording loop added in the previous
> patch will perform another mmap-read pass.
>
> The drain uses a two-layer safety check: event->count detects records
> staged by the driver, and record__bytes_written() in the drain loop
> confirms data was actually moved into perf.data. This combination
> handles the case where the driver count is briefly stale while hardware
> is still flushing.
>
> The implementation scans the evlist using evsel__pmu_name() to identify
> HTM events by their kernel-assigned PMU name rather than the
> user-visible event name, preventing false matches. It iterates the fd/
> sample-id xyarray, and skips any evsel whose fd and sample-id arrays are
> mismatched to avoid reading stale state. When the accumulated record
> count reaches zero the hook returns 0 and the recording loop proceeds to
> disable and close the events.
This looks like the proposed driver:
https://lore.kernel.org/all/[email protected]/
breaks the perf ABI.
I am not going to review any more tools patches for now.
>
> Signed-off-by: Athira Rajeev <[email protected]>
> ---
> Changes in V5:
> - When an HTM evsel is a group sibling (evsel->core.leader !=
> &evsel->core), read through its group leader's struct perf_evsel
> instead of the sibling directly. perf_evsel__read_size() uses
> evsel->nr_members to compute the read buffer size; nr_members is 0
> for siblings, so size=0 is passed to readn(), which returns <=0 and
> leaves count.val=0, causing the drain loop to terminate prematurely.
> Reading through the leader avoids the zero-size buffer and correctly
> accumulates the leader's pending count. HTM events are always
> standalone or per-target leaders in practice; the leader redirect
> handles any grouped configuration without losing counts.
>
> Changes in V4:
> - No changes from V3.
>
> Changes in V3:
> - Use evsel__pmu_name(evsel) instead of strstarts(evsel->name, "htm")
> to identify HTM events, matching by kernel-assigned PMU name rather
> than user-visible event name.
> - Remove the redundant two-pass loop (first pass to set found_htm,
> second to accumulate counts); a single pass with evsel__pmu_name()
> is sufficient. if no HTM event exists total_pending_records stays 0
> and the function returns 0.
> - Remove the dead !strcmp(evsel->name, "dummy:u") check;
> - evsel__pmu_name() will never return "htm" for a dummy:u software
> event.
> - Rename total_pending_bytes -> total_pending_records to match what
> the driver actually reports (event->count = total_size / record_size,
> a record count, not a byte count).
> - Add #include <string.h> for musl compatibility (strcmp() without
> it warns on some toolchains).
>
> Changes in V2:
> - Implements the renamed arch_perf_record__need_read() hook (V1
> implemented arch_record__collect_final_data()).
> - Skips evsels whose fd and sample-id xyarrays are mismatched, avoiding
> stale-state reads. V1 had no such guard.
> - evlist__enable cycling is removed; that responsibility now belongs to
> the drain loop in builtin-record.c added in patch 3.
> - File location changed to arch/powerpc/util/evsel.c (V1 used
> arch/powerpc/util/powerpc-htm.c).
> - Patch is now 4/6 instead of 4/9.
>
> tools/perf/arch/powerpc/util/evsel.c | 77 ++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/tools/perf/arch/powerpc/util/evsel.c
> b/tools/perf/arch/powerpc/util/evsel.c
> index 2f733cdc8dbb..2b7851c70677 100644
> --- a/tools/perf/arch/powerpc/util/evsel.c
> +++ b/tools/perf/arch/powerpc/util/evsel.c
> @@ -1,8 +1,85 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <linux/string.h>
> #include "util/evsel.h"
> +#include "util/record.h"
> +#include "util/evlist.h"
> +#include "util/debug.h"
> +#include <internal/xyarray.h>
> +#include <internal/lib.h>
>
> void arch_evsel__set_sample_weight(struct evsel *evsel)
> {
> evsel__set_sample_bit(evsel, WEIGHT_STRUCT);
> }
> +
> +/*
> + * powerpc implementation of arch_perf_record__need_read().
> + *
> + * Reads event->count for every open HTM evsel by issuing a direct
> + * read() on the event fd with a plain u64 buffer, bypassing the
> + * PERF_FORMAT_GROUP path in perf_evsel__read(). When an HTM evsel is
> + * a group sibling, evsel__config() sets PERF_FORMAT_GROUP on its attr;
> + * perf_evsel__read() would then call perf_evsel__read_group() which
> + * sizes the buffer by evsel->nr_members (0 for siblings), causing the
> + * kernel to return -ENOSPC. Reading the fd directly with sizeof(u64)
> + * retrieves the HTM driver's plain pending-record count regardless of
> + * group membership.
> + *
> + * Returns: 1 if more data exists, 0 if collection is complete
> + */
> +int arch_perf_record__need_read(struct evlist *evlist)
> +{
> + struct evsel *evsel;
> + u64 total_pending_records = 0;
> + int x, y;
> +
> + /* there was an error during record__open */
> + if (!evlist)
> + return 0;
> +
> + /* Read HTM event counts to check if more data is available */
> + evlist__for_each_entry(evlist, evsel) {
> + struct perf_evsel *rd_evsel;
> + struct xyarray *xy;
> +
> + if (strcmp(evsel__pmu_name(evsel), "htm"))
> + continue;
> +
> + /*
> + * For group siblings nr_members == 0, which makes
> + * perf_evsel__read_size() return 0 and readn() fail.
> + * Read through the leader instead; perf_evsel__read_group()
> + * extracts the leader's own count from the group buffer.
> + */
> + if (evsel->core.leader != &evsel->core)
> + rd_evsel = evsel->core.leader;
> + else
> + rd_evsel = &evsel->core;
> +
> + xy = rd_evsel->sample_id;
> +
> + if (xy == NULL || rd_evsel->fd == NULL)
> + continue;
> +
> + if (xyarray__max_x(rd_evsel->fd) != xyarray__max_x(xy) ||
> + xyarray__max_y(rd_evsel->fd) != xyarray__max_y(xy)) {
> + pr_debug("Unmatched FD vs sample ID array for HTM
> event\n");
> + continue;
> + }
> +
> + for (x = 0; x < xyarray__max_x(xy); x++) {
> + for (y = 0; y < xyarray__max_y(xy); y++) {
> + struct perf_counts_values count = { .val = 0 };
> +
> + if (perf_evsel__read(rd_evsel, x, y, &count) ==
> 0)
> + total_pending_records += count.val;
> + }
> + }
> + }
> +
> + /* Collection is complete only when ALL hardware queues have no pending
> records */
> + return (total_pending_records > 0) ? 1 : 0;
> +}
