Re: [PATCH v4 07/14] perf stat: Implement CSV formatting callbacks

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Jul 16, 2026 at 12:02:56AM -0700, Ian Rogers wrote:
> This patch implements CSV output formatting callbacks inside
> util/stat-print-csv.c, replacing the empty stubs introduced in Commit 1.
> 
> Defines the format-private `struct queued_event` and `struct
> queued_metric` DOM nodes to buffer traversal streams, and fully
> encapsulates CSV queued lists lifecycle and deallocations inside
> csv_print_start() and csv_print_end().
> 
> Utilizes the newly centralized unified aggregation helpers to format CPU
> and thread column prefixes cleanly, fixes metrics separators padding,
> and incorporates full interval-mode timestamp printing support.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Assisted-by: Antigravity:gemini-3.5-flash
> Acked-by: Chun-Tse Shao <ctshao@google.com>
> ---
[SNIP]
> +/**
> + * print_aggr_id_csv - Print the aggregation prefix for CSV format.
> + *
> + * Copied and adapted from stat-display.c.
> + */
> +static void print_aggr_id_csv(const struct perf_stat_config *config, FILE *output,
> +			      struct evsel *evsel, struct aggr_cpu_id id, int aggr_nr)
> +{
> +	const char *sep = config->csv_sep;
> +
> +	switch (config->aggr_mode) {
> +	case AGGR_CORE:
> +		fprintf(output, "S%d-D%d-C%d%s%d%s", id.socket, id.die, id.core, sep, aggr_nr, sep);
> +		break;
> +	case AGGR_CACHE:
> +		fprintf(output, "S%d-D%d-L%d-ID%d%s%d%s", id.socket, id.die, id.cache_lvl, id.cache,
> +			sep, aggr_nr, sep);
> +		break;
> +	case AGGR_CLUSTER:
> +		fprintf(output, "S%d-D%d-CLS%d%s%d%s", id.socket, id.die, id.cluster, sep, aggr_nr,
> +			sep);
> +		break;
> +	case AGGR_DIE:
> +		fprintf(output, "S%d-D%d%s%d%s", id.socket, id.die, sep, aggr_nr, sep);
> +		break;
> +	case AGGR_SOCKET:
> +		fprintf(output, "S%d%s%d%s", id.socket, sep, aggr_nr, sep);
> +		break;
> +	case AGGR_NODE:
> +		fprintf(output, "N%d%s%d%s", id.node, sep, aggr_nr, sep);
> +		break;
> +	case AGGR_NONE:
> +		if (evsel->percore && !config->percore_show_thread)
> +			fprintf(output, "S%d-D%d-C%d%s", id.socket, id.die, id.core, sep);
> +		else if (id.cpu.cpu > -1)
> +			fprintf(output, "CPU%d%s", id.cpu.cpu, sep);
> +		break;
> +	case AGGR_THREAD: {
> +		const char *comm = "unknown";
> +		int pid = -1;
> +
> +		if (evsel && evsel->core.threads && id.thread_idx >= 0 &&
> +		    id.thread_idx < perf_thread_map__nr(evsel->core.threads)) {
> +			comm = perf_thread_map__comm(evsel->core.threads,
> +						      id.thread_idx);
> +			pid = perf_thread_map__pid(evsel->core.threads,
> +						    id.thread_idx);
> +		}
> +		fprintf(output, "%s-%d%s", comm, pid, sep);
> +		break;
> +	}

Can you use the type-agnostic helper here?


> +	case AGGR_GLOBAL:
> +	case AGGR_UNSET:
> +	case AGGR_MAX:
> +	default:
> +		break;
> +	}
> +}
[SNIP]
> +static int csv_print_end(void *ctx, struct perf_stat_config *config)
> +{
> +	struct csv_print_state *ps = ctx;
> +	struct queued_event *ev, *tmp_ev;
> +	struct queued_metric *met, *tmp_met;
> +	FILE *output = ps->fp;
> +	const char *sep = ps->sep;
> +	bool has_metrics;
> +
> +	list_for_each_entry_safe(ev, tmp_ev, &ps->events_list, list) {
> +		struct evsel *evsel = ev->evsel;
> +		bool ok = (ev->run != 0 && ev->ena != 0);
> +		const char *bad_count = ev->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED;
> +		double enabled_percent = 100;
> +
> +		/* Print interval timestamp first if configured */
> +		if (config->interval && ps->timestamp[0])
> +			fprintf(output, "%s", ps->timestamp);

Doesn't it need a separator?


> +
> +		/* Print aggregation prefix first in CSV normal mode */
> +		if (config->aggr_map && ev->aggr_idx >= 0) {
> +			struct aggr_cpu_id id = config->aggr_map->map[ev->aggr_idx];
> +			int aggr_nr = 0;
> +
> +			if (evsel->stats && evsel->stats->aggr)
> +				aggr_nr = evsel->stats->aggr[ev->aggr_idx].nr;
> +
> +			print_aggr_id_csv(config, output, evsel, id, aggr_nr);
> +		}
> +
> +		/* 1. Print Value, Unit, Name (Columns 1, 2, 3) */
> +		if (ok) {
> +			double sc = ev->scale;
> +			double avg = ev->val * sc;
> +			const char *fmt = floor(sc) != sc ? "%.2f%s" : "%.0f%s";
> +
> +			fprintf(output, fmt, avg, sep);
> +		} else {
> +			fprintf(output, "%s%s", bad_count, sep);
> +		}
>  
> -int perf_stat__print_csv(struct evlist *evlist __maybe_unused,
> -			 const struct perf_stat_config *config __maybe_unused,
> -			 const struct target *target __maybe_unused,
> -			 const struct timespec *ts __maybe_unused,
> -			 int argc __maybe_unused,
> -			 const char **argv __maybe_unused)
> +		if (ev->unit)
> +			fprintf(output, "%s%s", ev->unit, sep);
> +		else
> +			fprintf(output, "%s", sep);
> +
> +		fprintf(output, "%s", ev->name);
> +
> +		/* 2. Print Runtime and Enabled Percentage (Columns 4, 5) */
> +		if (ev->run != ev->ena)
> +			enabled_percent = 100.0 * ev->run / ev->ena;
> +		fprintf(output, "%s%" PRIu64 "%s%.2f", sep, ev->run, sep,
> +			enabled_percent);

Cgroups?

Thanks,
Namhyung

> +
> +		/* 3. Print Metrics (Columns 6, 7) */
> +		has_metrics = false;
> +		list_for_each_entry_safe(met, tmp_met, &ev->metrics_list, list) {
> +			if (!has_metrics) {
> +				has_metrics = true;
> +			} else {
> +				fprintf(output, "\n");
> +				if (config->interval && ps->timestamp[0])
> +					fprintf(output, "%s", ps->timestamp);
> +				if (config->aggr_map && ev->aggr_idx >= 0) {
> +					struct aggr_cpu_id id = config->aggr_map->map[ev->aggr_idx];
> +					int aggr_nr = 0;
> +
> +					if (evsel->stats && evsel->stats->aggr)
> +						aggr_nr = evsel->stats->aggr[ev->aggr_idx].nr;
> +
> +					print_aggr_id_csv(config, output, evsel, id, aggr_nr);
> +				}
> +				/*
> +				 * Pad exactly 4 commas (Value, Unit, Name, Run, Enabled)
> +				 * to line up with the Metric Value column.
> +				 */
> +				fprintf(output, "%s%s%s%s", sep, sep, sep, sep);
> +			}
> +			fprintf(output, "%s%.2f%s", sep, met->val, sep);
> +			if (met->name && met->name[0])
> +				fprintf(output, "%s", met->name);
> +
> +			list_del(&met->list);
> +			free(met->name);
> +			free(met->unit);
> +			free(met);
> +		}
> +		if (!has_metrics)
> +			fprintf(output, "%s%s", sep, sep);
> +		fprintf(output, "\n");
> +
> +		list_del(&ev->list);
> +		free(ev->name);
> +		free(ev->unit);
> +		free(ev->cgrp);
> +		free(ev);
> +	}
> +	return 0;
> +}