[PATCH 0/4] perf tools: Support reading PERF_FORMAT_LOST (v2)

[PATCH 0/4] perf tools: Support reading PERF_FORMAT_LOST (v2)

[Namhyung Kim]

Hello,

The kernel v6.0 added PERF_FORMAT_LOST which can read a number of lost
samples for the given event.  As it can change the output format of
read(2) and perf sample data, it needs to access them carefully.

Changes in v2)
 * add a comment in perf_evsel__read_group()  (Jiri)
 * simplify perf_evsel__adjust_values()  (Jiri)


You can get the code from 'perf/read-lost-v2' brach on

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (4):
  tools headers UAPI: Sync linux/perf_event.h with the kernel sources
  tools lib perf: Handle read format in perf_evsel__read()
  tools lib perf: Add a test case for read formats
  perf tools: Support reading PERF_FORMAT_LOST

 tools/include/uapi/linux/perf_event.h         |   5 +-
 tools/lib/perf/evsel.c                        |  79 ++++++++-
 tools/lib/perf/include/perf/event.h           |   3 +-
 tools/lib/perf/include/perf/evsel.h           |   4 +-
 tools/lib/perf/tests/test-evsel.c             | 161 ++++++++++++++++++
 tools/perf/tests/sample-parsing.c             |  14 +-
 tools/perf/util/event.h                       |  18 +-
 tools/perf/util/evsel.c                       |  33 +++-
 .../scripting-engines/trace-event-python.c    |  16 +-
 tools/perf/util/session.c                     |  32 ++--
 tools/perf/util/synthetic-events.c            |  34 +++-
 11 files changed, 360 insertions(+), 39 deletions(-)


base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 1/4] tools headers UAPI: Sync linux/perf_event.h with the kernel sources

[Namhyung Kim]

To pick the trivial change in:

  119a784c8127 ("perf/core: Add a new read format to get a number of lost samples")

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/include/uapi/linux/perf_event.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index e2b77fbca91e..581ed4bdc062 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -301,6 +301,7 @@ enum {
  *	  { u64		time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
  *	  { u64		time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
  *	  { u64		id;           } && PERF_FORMAT_ID
+ *	  { u64		lost;         } && PERF_FORMAT_LOST
  *	} && !PERF_FORMAT_GROUP
  *
  *	{ u64		nr;
@@ -308,6 +309,7 @@ enum {
  *	  { u64		time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
  *	  { u64		value;
  *	    { u64	id;           } && PERF_FORMAT_ID
+ *	    { u64	lost;         } && PERF_FORMAT_LOST
  *	  }		cntr[nr];
  *	} && PERF_FORMAT_GROUP
  * };
@@ -317,8 +319,9 @@ enum perf_event_read_format {
 	PERF_FORMAT_TOTAL_TIME_RUNNING		= 1U << 1,
 	PERF_FORMAT_ID				= 1U << 2,
 	PERF_FORMAT_GROUP			= 1U << 3,
+	PERF_FORMAT_LOST			= 1U << 4,
 
-	PERF_FORMAT_MAX = 1U << 4,		/* non-ABI */
+	PERF_FORMAT_MAX = 1U << 5,		/* non-ABI */
 };
 
 #define PERF_ATTR_SIZE_VER0	64	/* sizeof first published struct */
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 2/4] tools lib perf: Handle read format in perf_evsel__read()

[Namhyung Kim]

The perf_counts_values should be increased to read the new lost data.
Also adjust values after read according the read format.

This supports PERF_FORMAT_GROUP which has a different data format but
it's only available for leader events.  Currently it doesn't have an API
to read sibling (member) events in the group.  But users may read the
sibling event directly.

Also reading from mmap would be disabled when the read format has ID or
LOST bit as it's not exposed via mmap.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/perf/evsel.c              | 79 ++++++++++++++++++++++++++++-
 tools/lib/perf/include/perf/event.h |  3 +-
 tools/lib/perf/include/perf/evsel.h |  4 +-
 3 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 952f3520d5c2..8ce5bbd09666 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -305,6 +305,9 @@ int perf_evsel__read_size(struct perf_evsel *evsel)
 	if (read_format & PERF_FORMAT_ID)
 		entry += sizeof(u64);
 
+	if (read_format & PERF_FORMAT_LOST)
+		entry += sizeof(u64);
+
 	if (read_format & PERF_FORMAT_GROUP) {
 		nr = evsel->nr_members;
 		size += sizeof(u64);
@@ -314,24 +317,98 @@ int perf_evsel__read_size(struct perf_evsel *evsel)
 	return size;
 }
 
+/* This only reads values for the leader */
+static int perf_evsel__read_group(struct perf_evsel *evsel, int cpu_map_idx,
+				  int thread, struct perf_counts_values *count)
+{
+	size_t size = perf_evsel__read_size(evsel);
+	int *fd = FD(evsel, cpu_map_idx, thread);
+	u64 read_format = evsel->attr.read_format;
+	u64 *data;
+	int idx = 1;
+
+	if (fd == NULL || *fd < 0)
+		return -EINVAL;
+
+	data = calloc(1, size);
+	if (data == NULL)
+		return -ENOMEM;
+
+	if (readn(*fd, data, size) <= 0) {
+		free(data);
+		return -errno;
+	}
+
+	/*
+	 * This reads only the leader event intentionally since we don't have
+	 * perf counts values for sibling events.
+	 */
+	if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+		count->ena = data[idx++];
+	if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+		count->run = data[idx++];
+
+	/* value is always available */
+	count->val = data[idx++];
+	if (read_format & PERF_FORMAT_ID)
+		count->id = data[idx++];
+	if (read_format & PERF_FORMAT_LOST)
+		count->lost = data[idx++];
+
+	free(data);
+	return 0;
+}
+
+/*
+ * The perf read format is very flexible.  It needs to set the proper
+ * values according to the read format.
+ */
+static void perf_evsel__adjust_values(struct perf_evsel *evsel, u64 *buf,
+				      struct perf_counts_values *count)
+{
+	u64 read_format = evsel->attr.read_format;
+	int n = 0;
+
+	count->val = buf[n++];
+
+	if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+		count->ena = buf[n++];
+
+	if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+		count->run = buf[n++];
+
+	if (read_format & PERF_FORMAT_ID)
+		count->id = buf[n++];
+
+	if (read_format & PERF_FORMAT_LOST)
+		count->lost = buf[n++];
+}
+
 int perf_evsel__read(struct perf_evsel *evsel, int cpu_map_idx, int thread,
 		     struct perf_counts_values *count)
 {
 	size_t size = perf_evsel__read_size(evsel);
 	int *fd = FD(evsel, cpu_map_idx, thread);
+	u64 read_format = evsel->attr.read_format;
+	struct perf_counts_values buf;
 
 	memset(count, 0, sizeof(*count));
 
 	if (fd == NULL || *fd < 0)
 		return -EINVAL;
 
+	if (read_format & PERF_FORMAT_GROUP)
+		return perf_evsel__read_group(evsel, cpu_map_idx, thread, count);
+
 	if (MMAP(evsel, cpu_map_idx, thread) &&
+	    !(read_format & (PERF_FORMAT_ID | PERF_FORMAT_LOST)) &&
 	    !perf_mmap__read_self(MMAP(evsel, cpu_map_idx, thread), count))
 		return 0;
 
-	if (readn(*fd, count->values, size) <= 0)
+	if (readn(*fd, buf.values, size) <= 0)
 		return -errno;
 
+	perf_evsel__adjust_values(evsel, buf.values, count);
 	return 0;
 }
 
diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
index 556bb06798f2..38dd35cbca71 100644
--- a/tools/lib/perf/include/perf/event.h
+++ b/tools/lib/perf/include/perf/event.h
@@ -76,7 +76,7 @@ struct perf_record_lost_samples {
 };
 
 /*
- * PERF_FORMAT_ENABLED | PERF_FORMAT_RUNNING | PERF_FORMAT_ID
+ * PERF_FORMAT_ENABLED | PERF_FORMAT_RUNNING | PERF_FORMAT_ID | PERF_FORMAT_LOST
  */
 struct perf_record_read {
 	struct perf_event_header header;
@@ -85,6 +85,7 @@ struct perf_record_read {
 	__u64			 time_enabled;
 	__u64			 time_running;
 	__u64			 id;
+	__u64			 lost;
 };
 
 struct perf_record_throttle {
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index 699c0ed97d34..6f92204075c2 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -18,8 +18,10 @@ struct perf_counts_values {
 			uint64_t val;
 			uint64_t ena;
 			uint64_t run;
+			uint64_t id;
+			uint64_t lost;
 		};
-		uint64_t values[3];
+		uint64_t values[5];
 	};
 };
 
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 3/4] tools lib perf: Add a test case for read formats

[Namhyung Kim]

It checks a various combination of the read format settings and verify
it return the value in a proper position.  The test uses task-clock
software events to guarantee it's always active and sets enabled/running
time.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/perf/tests/test-evsel.c | 161 ++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c
index 89be89afb24d..a11fc51bfb68 100644
--- a/tools/lib/perf/tests/test-evsel.c
+++ b/tools/lib/perf/tests/test-evsel.c
@@ -1,10 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <stdarg.h>
 #include <stdio.h>
+#include <string.h>
 #include <linux/perf_event.h>
+#include <linux/kernel.h>
 #include <perf/cpumap.h>
 #include <perf/threadmap.h>
 #include <perf/evsel.h>
+#include <internal/evsel.h>
 #include <internal/tests.h>
 #include "tests.h"
 
@@ -189,6 +192,163 @@ static int test_stat_user_read(int event)
 	return 0;
 }
 
+static int test_stat_read_format_single(struct perf_event_attr *attr, struct perf_thread_map *threads)
+{
+	struct perf_evsel *evsel;
+	struct perf_counts_values counts;
+	volatile int count = 0x100000;
+	int err;
+
+	evsel = perf_evsel__new(attr);
+	__T("failed to create evsel", evsel);
+
+	/* skip old kernels that don't support the format */
+	err = perf_evsel__open(evsel, NULL, threads);
+	if (err < 0)
+		return 0;
+
+	while (count--) ;
+
+	memset(&counts, -1, sizeof(counts));
+	perf_evsel__read(evsel, 0, 0, &counts);
+
+	__T("failed to read value", counts.val);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+		__T("failed to read TOTAL_TIME_ENABLED", counts.ena);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+		__T("failed to read TOTAL_TIME_RUNNING", counts.run);
+	if (attr->read_format & PERF_FORMAT_ID)
+		__T("failed to read ID", counts.id);
+	if (attr->read_format & PERF_FORMAT_LOST)
+		__T("failed to read LOST", counts.lost == 0);
+
+	perf_evsel__close(evsel);
+	perf_evsel__delete(evsel);
+	return 0;
+}
+
+static int test_stat_read_format_group(struct perf_event_attr *attr, struct perf_thread_map *threads)
+{
+	struct perf_evsel *leader, *member;
+	struct perf_counts_values counts;
+	volatile int count = 0x100000;
+	int err;
+
+	attr->read_format |= PERF_FORMAT_GROUP;
+	leader = perf_evsel__new(attr);
+	__T("failed to create leader", leader);
+
+	attr->read_format &= ~PERF_FORMAT_GROUP;
+	member = perf_evsel__new(attr);
+	__T("failed to create member", member);
+
+	member->leader = leader;
+	leader->nr_members = 2;
+
+	/* skip old kernels that don't support the format */
+	err = perf_evsel__open(leader, NULL, threads);
+	if (err < 0)
+		return 0;
+	err = perf_evsel__open(member, NULL, threads);
+	if (err < 0)
+		return 0;
+
+	while (count--) ;
+
+	memset(&counts, -1, sizeof(counts));
+	perf_evsel__read(leader, 0, 0, &counts);
+
+	__T("failed to read leader value", counts.val);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+		__T("failed to read leader TOTAL_TIME_ENABLED", counts.ena);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+		__T("failed to read leader TOTAL_TIME_RUNNING", counts.run);
+	if (attr->read_format & PERF_FORMAT_ID)
+		__T("failed to read leader ID", counts.id);
+	if (attr->read_format & PERF_FORMAT_LOST)
+		__T("failed to read leader LOST", counts.lost == 0);
+
+	memset(&counts, -1, sizeof(counts));
+	perf_evsel__read(member, 0, 0, &counts);
+
+	__T("failed to read member value", counts.val);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+		__T("failed to read member TOTAL_TIME_ENABLED", counts.ena);
+	if (attr->read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+		__T("failed to read member TOTAL_TIME_RUNNING", counts.run);
+	if (attr->read_format & PERF_FORMAT_ID)
+		__T("failed to read member ID", counts.id);
+	if (attr->read_format & PERF_FORMAT_LOST)
+		__T("failed to read member LOST", counts.lost == 0);
+
+	perf_evsel__close(member);
+	perf_evsel__close(leader);
+	perf_evsel__delete(member);
+	perf_evsel__delete(leader);
+	return 0;
+}
+
+static int test_stat_read_format(void)
+{
+	struct perf_thread_map *threads;
+	struct perf_event_attr attr = {
+		.type	= PERF_TYPE_SOFTWARE,
+		.config	= PERF_COUNT_SW_TASK_CLOCK,
+	};
+	int err, i;
+
+#define FMT(_fmt)  PERF_FORMAT_ ## _fmt
+#define FMT_TIME  (FMT(TOTAL_TIME_ENABLED) | FMT(TOTAL_TIME_RUNNING))
+
+	uint64_t test_formats [] = {
+		0,
+		FMT_TIME,
+		FMT(ID),
+		FMT(LOST),
+		FMT_TIME | FMT(ID),
+		FMT_TIME | FMT(LOST),
+		FMT_TIME | FMT(ID) | FMT(LOST),
+		FMT(ID) | FMT(LOST),
+	};
+
+#undef FMT
+#undef FMT_TIME
+
+	threads = perf_thread_map__new_dummy();
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+
+	for (i = 0; i < (int)ARRAY_SIZE(test_formats); i++) {
+		attr.read_format = test_formats[i];
+		__T_VERBOSE("testing single read with read_format: %lx\n",
+			    (unsigned long)test_formats[i]);
+
+		err = test_stat_read_format_single(&attr, threads);
+		__T("failed to read single format", err == 0);
+	}
+
+	perf_thread_map__put(threads);
+
+	threads = perf_thread_map__new_array(2, NULL);
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+	perf_thread_map__set_pid(threads, 1, 0);
+
+	for (i = 0; i < (int)ARRAY_SIZE(test_formats); i++) {
+		attr.read_format = test_formats[i];
+		__T_VERBOSE("testing group read with read_format: %lx\n",
+			    (unsigned long)test_formats[i]);
+
+		err = test_stat_read_format_group(&attr, threads);
+		__T("failed to read group format", err == 0);
+	}
+
+	perf_thread_map__put(threads);
+	return 0;
+}
+
 int test_evsel(int argc, char **argv)
 {
 	__T_START;
@@ -200,6 +360,7 @@ int test_evsel(int argc, char **argv)
 	test_stat_thread_enable();
 	test_stat_user_read(PERF_COUNT_HW_INSTRUCTIONS);
 	test_stat_user_read(PERF_COUNT_HW_CPU_CYCLES);
+	test_stat_read_format();
 
 	__T_END;
 	return tests_failed == 0 ? 0 : -1;
-- 
2.37.1.595.g718a3a8f04-goog

[PATCH 4/4] perf tools: Support reading PERF_FORMAT_LOST

[Namhyung Kim]

The recent kernel added lost count can be read from either read(2) or
ring buffer data with PERF_SAMPLE_READ.  As it's a variable length data
we need to access it according to the format info.

But for perf tools use cases, PERF_FORMAT_ID is always set.  So we can
only check PERF_FORMAT_LOST bit to determine the data format.

Add sample_read_value_size() and next_sample_read_value() helpers to
make it a bit easier to access.  Use them in all places where it reads
the struct sample_read_value.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/sample-parsing.c             | 14 +++++---
 tools/perf/util/event.h                       | 18 +++++++++-
 tools/perf/util/evsel.c                       | 33 +++++++++++++-----
 .../scripting-engines/trace-event-python.c    | 16 ++++++---
 tools/perf/util/session.c                     | 32 +++++++++++------
 tools/perf/util/synthetic-events.c            | 34 +++++++++++++++----
 6 files changed, 112 insertions(+), 35 deletions(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 07f2411b0ad4..20930dd48ee0 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -86,10 +86,15 @@ static bool samples_same(const struct perf_sample *s1,
 			COMP(read.time_running);
 		/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
 		if (read_format & PERF_FORMAT_GROUP) {
-			for (i = 0; i < s1->read.group.nr; i++)
-				MCOMP(read.group.values[i]);
+			for (i = 0; i < s1->read.group.nr; i++) {
+				/* FIXME: check values without LOST */
+				if (read_format & PERF_FORMAT_LOST)
+					MCOMP(read.group.values[i]);
+			}
 		} else {
 			COMP(read.one.id);
+			if (read_format & PERF_FORMAT_LOST)
+				COMP(read.one.lost);
 		}
 	}
 
@@ -263,7 +268,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
 			.data	= (void *)aux_data,
 		},
 	};
-	struct sample_read_value values[] = {{1, 5}, {9, 3}, {2, 7}, {6, 4},};
+	struct sample_read_value values[] = {{1, 5, 0}, {9, 3, 0}, {2, 7, 0}, {6, 4, 1},};
 	struct perf_sample sample_out, sample_out_endian;
 	size_t i, sz, bufsz;
 	int err, ret = -1;
@@ -286,6 +291,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
 	} else {
 		sample.read.one.value = 0x08789faeb786aa87ULL;
 		sample.read.one.id    = 99;
+		sample.read.one.lost  = 1;
 	}
 
 	sz = perf_event__sample_event_size(&sample, sample_type, read_format);
@@ -370,7 +376,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
  */
 static int test__sample_parsing(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
 {
-	const u64 rf[] = {4, 5, 6, 7, 12, 13, 14, 15};
+	const u64 rf[] = {4, 5, 6, 7, 12, 13, 14, 15, 20, 21, 22, 28, 29, 30, 31};
 	u64 sample_type;
 	u64 sample_regs;
 	size_t i;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index a7b0931d5137..7753368d70d6 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -65,7 +65,8 @@ struct stack_dump {
 
 struct sample_read_value {
 	u64 value;
-	u64 id;
+	u64 id;   /* only if PERF_FORMAT_ID */
+	u64 lost; /* only if PERF_FORMAT_LOST */
 };
 
 struct sample_read {
@@ -80,6 +81,21 @@ struct sample_read {
 	};
 };
 
+static inline size_t sample_read_value_size(u64 read_format)
+{
+	/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
+	if (read_format & PERF_FORMAT_LOST)
+		return sizeof(struct sample_read_value);
+	else
+		return offsetof(struct sample_read_value, lost);
+}
+
+static inline struct sample_read_value *
+next_sample_read_value(struct sample_read_value *v, u64 read_format)
+{
+	return (void *)v + sample_read_value_size(read_format);
+}
+
 struct ip_callchain {
 	u64 nr;
 	u64 ips[];
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 4852089e1d79..781367ccdb84 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1541,7 +1541,7 @@ static int evsel__read_one(struct evsel *evsel, int cpu_map_idx, int thread)
 }
 
 static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
-			     u64 val, u64 ena, u64 run)
+			     u64 val, u64 ena, u64 run, u64 lost)
 {
 	struct perf_counts_values *count;
 
@@ -1550,6 +1550,7 @@ static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
 	count->val    = val;
 	count->ena    = ena;
 	count->run    = run;
+	count->lost   = lost;
 
 	perf_counts__set_loaded(counter->counts, cpu_map_idx, thread, true);
 }
@@ -1558,7 +1559,7 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
 {
 	u64 read_format = leader->core.attr.read_format;
 	struct sample_read_value *v;
-	u64 nr, ena = 0, run = 0, i;
+	u64 nr, ena = 0, run = 0, lost = 0, i;
 
 	nr = *data++;
 
@@ -1573,16 +1574,25 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
 
 	v = (struct sample_read_value *) data;
 
-	evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run);
+	if (read_format & PERF_FORMAT_LOST)
+		lost = v->lost;
+
+	evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run, lost);
+
+	v = next_sample_read_value(v, read_format);
 
 	for (i = 1; i < nr; i++) {
 		struct evsel *counter;
 
-		counter = evlist__id2evsel(leader->evlist, v[i].id);
+		counter = evlist__id2evsel(leader->evlist, v->id);
 		if (!counter)
 			return -EINVAL;
 
-		evsel__set_count(counter, cpu_map_idx, thread, v[i].value, ena, run);
+		if (read_format & PERF_FORMAT_LOST)
+			lost = v->lost;
+
+		evsel__set_count(counter, cpu_map_idx, thread, v->value, ena, run, lost);
+		v = next_sample_read_value(v, read_format);
 	}
 
 	return 0;
@@ -2475,16 +2485,21 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 
 			if (data->read.group.nr > max_group_nr)
 				return -EFAULT;
-			sz = data->read.group.nr *
-			     sizeof(struct sample_read_value);
+
+			sz = data->read.group.nr * sample_read_value_size(read_format);
 			OVERFLOW_CHECK(array, sz, max_size);
-			data->read.group.values =
-					(struct sample_read_value *)array;
+			data->read.group.values = (void *)array;
 			array = (void *)array + sz;
 		} else {
 			OVERFLOW_CHECK_u64(array);
 			data->read.one.id = *array;
 			array++;
+
+			if (read_format & PERF_FORMAT_LOST) {
+				OVERFLOW_CHECK_u64(array);
+				data->read.one.lost = *array;
+				array++;
+			}
 		}
 	}
 
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 9ef2406e0ede..7e15e64f311b 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -642,15 +642,19 @@ static PyObject *python_process_brstacksym(struct perf_sample *sample,
 	return pylist;
 }
 
-static PyObject *get_sample_value_as_tuple(struct sample_read_value *value)
+static PyObject *get_sample_value_as_tuple(struct sample_read_value *value,
+					   u64 read_format)
 {
 	PyObject *t;
 
-	t = PyTuple_New(2);
+	t = PyTuple_New(3);
 	if (!t)
 		Py_FatalError("couldn't create Python tuple");
 	PyTuple_SetItem(t, 0, PyLong_FromUnsignedLongLong(value->id));
 	PyTuple_SetItem(t, 1, PyLong_FromUnsignedLongLong(value->value));
+	if (read_format & PERF_FORMAT_LOST)
+		PyTuple_SetItem(t, 2, PyLong_FromUnsignedLongLong(value->lost));
+
 	return t;
 }
 
@@ -681,12 +685,16 @@ static void set_sample_read_in_dict(PyObject *dict_sample,
 		Py_FatalError("couldn't create Python list");
 
 	if (read_format & PERF_FORMAT_GROUP) {
+		struct sample_read_value *v = sample->read.group.values;
+
 		for (i = 0; i < sample->read.group.nr; i++) {
-			PyObject *t = get_sample_value_as_tuple(&sample->read.group.values[i]);
+			PyObject *t = get_sample_value_as_tuple(v, read_format);
 			PyList_SET_ITEM(values, i, t);
+			v = next_sample_read_value(v, read_format);
 		}
 	} else {
-		PyObject *t = get_sample_value_as_tuple(&sample->read.one);
+		PyObject *t = get_sample_value_as_tuple(&sample->read.one,
+							read_format);
 		PyList_SET_ITEM(values, 0, t);
 	}
 	pydict_set_item_string_decref(dict_sample, "values", values);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 98e16659a149..dd818764bd4d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1284,20 +1284,26 @@ static void sample_read__printf(struct perf_sample *sample, u64 read_format)
 
 	if (read_format & PERF_FORMAT_GROUP) {
 		u64 i;
+		struct sample_read_value *value = sample->read.group.values;
 
 		printf(".... group nr %" PRIu64 "\n", sample->read.group.nr);
 
 		for (i = 0; i < sample->read.group.nr; i++) {
-			struct sample_read_value *value;
-
-			value = &sample->read.group.values[i];
 			printf("..... id %016" PRIx64
-			       ", value %016" PRIx64 "\n",
+			       ", value %016" PRIx64,
 			       value->id, value->value);
+			if (read_format & PERF_FORMAT_LOST)
+				printf(", lost %" PRIu64, value->lost);
+			printf("\n");
+			value = next_sample_read_value(value, read_format);
 		}
-	} else
-		printf("..... id %016" PRIx64 ", value %016" PRIx64 "\n",
+	} else {
+		printf("..... id %016" PRIx64 ", value %016" PRIx64,
 			sample->read.one.id, sample->read.one.value);
+		if (read_format & PERF_FORMAT_LOST)
+			printf(", lost %" PRIu64, sample->read.one.lost);
+		printf("\n");
+	}
 }
 
 static void dump_event(struct evlist *evlist, union perf_event *event,
@@ -1411,6 +1417,9 @@ static void dump_read(struct evsel *evsel, union perf_event *event)
 
 	if (read_format & PERF_FORMAT_ID)
 		printf("... id           : %" PRI_lu64 "\n", read_event->id);
+
+	if (read_format & PERF_FORMAT_LOST)
+		printf("... lost         : %" PRI_lu64 "\n", read_event->lost);
 }
 
 static struct machine *machines__find_for_cpumode(struct machines *machines,
@@ -1479,17 +1488,20 @@ static int deliver_sample_group(struct evlist *evlist,
 				struct perf_tool *tool,
 				union  perf_event *event,
 				struct perf_sample *sample,
-				struct machine *machine)
+				struct machine *machine,
+				u64 read_format)
 {
 	int ret = -EINVAL;
 	u64 i;
+	struct sample_read_value *v = sample->read.group.values;
 
 	for (i = 0; i < sample->read.group.nr; i++) {
-		ret = deliver_sample_value(evlist, tool, event, sample,
-					   &sample->read.group.values[i],
+		ret = deliver_sample_value(evlist, tool, event, sample, v,
 					   machine);
 		if (ret)
 			break;
+
+		v = next_sample_read_value(v, read_format);
 	}
 
 	return ret;
@@ -1510,7 +1522,7 @@ static int evlist__deliver_sample(struct evlist *evlist, struct perf_tool *tool,
 	/* For PERF_SAMPLE_READ we have either single or group mode. */
 	if (read_format & PERF_FORMAT_GROUP)
 		return deliver_sample_group(evlist, tool, event, sample,
-					    machine);
+					    machine, read_format);
 	else
 		return deliver_sample_value(evlist, tool, event, sample,
 					    &sample->read.one, machine);
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 2ae59c03ae77..9b8dc7633a02 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1432,11 +1432,12 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
 			result += sizeof(u64);
 		/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
 		if (read_format & PERF_FORMAT_GROUP) {
-			sz = sample->read.group.nr *
-			     sizeof(struct sample_read_value);
-			result += sz;
+			sz = sample_read_value_size(read_format);
+			result += sz * sample->read.group.nr;
 		} else {
 			result += sizeof(u64);
+			if (read_format & PERF_FORMAT_LOST)
+				result += sizeof(u64);
 		}
 	}
 
@@ -1521,6 +1522,22 @@ void __weak arch_perf_synthesize_sample_weight(const struct perf_sample *data,
 	*array = data->weight;
 }
 
+static __u64 *copy_read_group_values(__u64 *array, __u64 read_format,
+				     const struct perf_sample *sample)
+{
+	size_t i, sz;
+	void *data = sample->read.group.values;
+
+	sz = sample_read_value_size(read_format);
+	for (i = 0; i < sample->read.group.nr; i++) {
+		memcpy(array, data, sz);
+
+		data += sz;
+		array = (void *)array + sz;
+	}
+	return array;
+}
+
 int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_format,
 				  const struct perf_sample *sample)
 {
@@ -1602,13 +1619,16 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_fo
 
 		/* PERF_FORMAT_ID is forced for PERF_SAMPLE_READ */
 		if (read_format & PERF_FORMAT_GROUP) {
-			sz = sample->read.group.nr *
-			     sizeof(struct sample_read_value);
-			memcpy(array, sample->read.group.values, sz);
-			array = (void *)array + sz;
+			array = copy_read_group_values(array, read_format,
+						       sample);
 		} else {
 			*array = sample->read.one.id;
 			array++;
+
+			if (read_format & PERF_FORMAT_LOST) {
+				*array = sample->read.one.lost;
+				array++;
+			}
 		}
 	}
 
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH 4/4] perf tools: Support reading PERF_FORMAT_LOST

[Jiri Olsa]

On Tue, Aug 16, 2022 at 03:17:47PM -0700, Namhyung Kim wrote:

SNIP

> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index a7b0931d5137..7753368d70d6 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -65,7 +65,8 @@ struct stack_dump {
>  
>  struct sample_read_value {
>  	u64 value;
> -	u64 id;
> +	u64 id;   /* only if PERF_FORMAT_ID */
> +	u64 lost; /* only if PERF_FORMAT_LOST */
>  };

I was wondering why not to split this patch into smaller piece,
but once you change this struct you break all the places

SNIP

> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1541,7 +1541,7 @@ static int evsel__read_one(struct evsel *evsel, int cpu_map_idx, int thread)
>  }
>  
>  static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
> -			     u64 val, u64 ena, u64 run)
> +			     u64 val, u64 ena, u64 run, u64 lost)
>  {
>  	struct perf_counts_values *count;
>  
> @@ -1550,6 +1550,7 @@ static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
>  	count->val    = val;
>  	count->ena    = ena;
>  	count->run    = run;
> +	count->lost   = lost;
>  
>  	perf_counts__set_loaded(counter->counts, cpu_map_idx, thread, true);
>  }
> @@ -1558,7 +1559,7 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
>  {
>  	u64 read_format = leader->core.attr.read_format;
>  	struct sample_read_value *v;
> -	u64 nr, ena = 0, run = 0, i;
> +	u64 nr, ena = 0, run = 0, lost = 0, i;
>  
>  	nr = *data++;
>  
> @@ -1573,16 +1574,25 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
>  
>  	v = (struct sample_read_value *) data;
>  
> -	evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run);
> +	if (read_format & PERF_FORMAT_LOST)
> +		lost = v->lost;
> +
> +	evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run, lost);
> +
> +	v = next_sample_read_value(v, read_format);

oneway of making this simpler here and share with other places
could be adding something like:

  for_each_group_data(v, i, nr, read_format) {
  }

but not sure how would that turn out, thoughts?

>  
>  	for (i = 1; i < nr; i++) {
>  		struct evsel *counter;
>  
> -		counter = evlist__id2evsel(leader->evlist, v[i].id);
> +		counter = evlist__id2evsel(leader->evlist, v->id);
>  		if (!counter)
>  			return -EINVAL;
>  
> -		evsel__set_count(counter, cpu_map_idx, thread, v[i].value, ena, run);
> +		if (read_format & PERF_FORMAT_LOST)
> +			lost = v->lost;
> +
> +		evsel__set_count(counter, cpu_map_idx, thread, v->value, ena, run, lost);
> +		v = next_sample_read_value(v, read_format);
>  	}
>  
>  	return 0;
> @@ -2475,16 +2485,21 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
>  
>  			if (data->read.group.nr > max_group_nr)
>  				return -EFAULT;
> -			sz = data->read.group.nr *
> -			     sizeof(struct sample_read_value);
> +
> +			sz = data->read.group.nr * sample_read_value_size(read_format);
>  			OVERFLOW_CHECK(array, sz, max_size);
> -			data->read.group.values =
> -					(struct sample_read_value *)array;
> +			data->read.group.values = (void *)array;

nit, is this void casting needed?

thanks,
jirka

>  			array = (void *)array + sz;
>  		} else {
>  			OVERFLOW_CHECK_u64(array);
>  			data->read.one.id = *array;
>  			array++;
> +
> +			if (read_format & PERF_FORMAT_LOST) {
> +				OVERFLOW_CHECK_u64(array);
> +				data->read.one.lost = *array;
> +				array++;
> +			}
>  		}
>  	}
>  

SNIP

Re: [PATCH 4/4] perf tools: Support reading PERF_FORMAT_LOST

[Namhyung Kim]

Hi Jiri,

On Thu, Aug 18, 2022 at 5:04 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Aug 16, 2022 at 03:17:47PM -0700, Namhyung Kim wrote:
>
> SNIP
>
> > diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> > index a7b0931d5137..7753368d70d6 100644
> > --- a/tools/perf/util/event.h
> > +++ b/tools/perf/util/event.h
> > @@ -65,7 +65,8 @@ struct stack_dump {
> >
> >  struct sample_read_value {
> >       u64 value;
> > -     u64 id;
> > +     u64 id;   /* only if PERF_FORMAT_ID */
> > +     u64 lost; /* only if PERF_FORMAT_LOST */
> >  };
>
> I was wondering why not to split this patch into smaller piece,
> but once you change this struct you break all the places

Right.. I'd like to do so but couldn't.. :)

>
> SNIP
>
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -1541,7 +1541,7 @@ static int evsel__read_one(struct evsel *evsel, int cpu_map_idx, int thread)
> >  }
> >
> >  static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
> > -                          u64 val, u64 ena, u64 run)
> > +                          u64 val, u64 ena, u64 run, u64 lost)
> >  {
> >       struct perf_counts_values *count;
> >
> > @@ -1550,6 +1550,7 @@ static void evsel__set_count(struct evsel *counter, int cpu_map_idx, int thread,
> >       count->val    = val;
> >       count->ena    = ena;
> >       count->run    = run;
> > +     count->lost   = lost;
> >
> >       perf_counts__set_loaded(counter->counts, cpu_map_idx, thread, true);
> >  }
> > @@ -1558,7 +1559,7 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
> >  {
> >       u64 read_format = leader->core.attr.read_format;
> >       struct sample_read_value *v;
> > -     u64 nr, ena = 0, run = 0, i;
> > +     u64 nr, ena = 0, run = 0, lost = 0, i;
> >
> >       nr = *data++;
> >
> > @@ -1573,16 +1574,25 @@ static int evsel__process_group_data(struct evsel *leader, int cpu_map_idx, int
> >
> >       v = (struct sample_read_value *) data;
> >
> > -     evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run);
> > +     if (read_format & PERF_FORMAT_LOST)
> > +             lost = v->lost;
> > +
> > +     evsel__set_count(leader, cpu_map_idx, thread, v[0].value, ena, run, lost);
> > +
> > +     v = next_sample_read_value(v, read_format);
>
> oneway of making this simpler here and share with other places
> could be adding something like:
>
>   for_each_group_data(v, i, nr, read_format) {
>   }
>
> but not sure how would that turn out, thoughts?

Looks good.  Let me try. :)


>
> >
> >       for (i = 1; i < nr; i++) {
> >               struct evsel *counter;
> >
> > -             counter = evlist__id2evsel(leader->evlist, v[i].id);
> > +             counter = evlist__id2evsel(leader->evlist, v->id);
> >               if (!counter)
> >                       return -EINVAL;
> >
> > -             evsel__set_count(counter, cpu_map_idx, thread, v[i].value, ena, run);
> > +             if (read_format & PERF_FORMAT_LOST)
> > +                     lost = v->lost;
> > +
> > +             evsel__set_count(counter, cpu_map_idx, thread, v->value, ena, run, lost);
> > +             v = next_sample_read_value(v, read_format);
> >       }
> >
> >       return 0;
> > @@ -2475,16 +2485,21 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> >
> >                       if (data->read.group.nr > max_group_nr)
> >                               return -EFAULT;
> > -                     sz = data->read.group.nr *
> > -                          sizeof(struct sample_read_value);
> > +
> > +                     sz = data->read.group.nr * sample_read_value_size(read_format);
> >                       OVERFLOW_CHECK(array, sz, max_size);
> > -                     data->read.group.values =
> > -                                     (struct sample_read_value *)array;
> > +                     data->read.group.values = (void *)array;
>
> nit, is this void casting needed?

Well.. the array is a pointer to u64 so the casting is needed.
But it's an unrelated change, can be dropped.

Thanks,
Namhyung


>
> >                       array = (void *)array + sz;
> >               } else {
> >                       OVERFLOW_CHECK_u64(array);
> >                       data->read.one.id = *array;
> >                       array++;
> > +
> > +                     if (read_format & PERF_FORMAT_LOST) {
> > +                             OVERFLOW_CHECK_u64(array);
> > +                             data->read.one.lost = *array;
> > +                             array++;
> > +                     }
> >               }
> >       }
> >
>
> SNIP

Re: [PATCH 0/4] perf tools: Support reading PERF_FORMAT_LOST (v2)

[Arnaldo Carvalho de Melo]

Em Tue, Aug 16, 2022 at 03:17:43PM -0700, Namhyung Kim escreveu:
> Hello,
> 
> The kernel v6.0 added PERF_FORMAT_LOST which can read a number of lost
> samples for the given event.  As it can change the output format of
> read(2) and perf sample data, it needs to access them carefully.
> 
> Changes in v2)
>  * add a comment in perf_evsel__read_group()  (Jiri)
>  * simplify perf_evsel__adjust_values()  (Jiri)

Replaced v1 with this one in my local tree.

- Arnaldo
 
> 
> You can get the code from 'perf/read-lost-v2' brach on
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (4):
>   tools headers UAPI: Sync linux/perf_event.h with the kernel sources
>   tools lib perf: Handle read format in perf_evsel__read()
>   tools lib perf: Add a test case for read formats
>   perf tools: Support reading PERF_FORMAT_LOST
> 
>  tools/include/uapi/linux/perf_event.h         |   5 +-
>  tools/lib/perf/evsel.c                        |  79 ++++++++-
>  tools/lib/perf/include/perf/event.h           |   3 +-
>  tools/lib/perf/include/perf/evsel.h           |   4 +-
>  tools/lib/perf/tests/test-evsel.c             | 161 ++++++++++++++++++
>  tools/perf/tests/sample-parsing.c             |  14 +-
>  tools/perf/util/event.h                       |  18 +-
>  tools/perf/util/evsel.c                       |  33 +++-
>  .../scripting-engines/trace-event-python.c    |  16 +-
>  tools/perf/util/session.c                     |  32 ++--
>  tools/perf/util/synthetic-events.c            |  34 +++-
>  11 files changed, 360 insertions(+), 39 deletions(-)
> 
> 
> base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
> -- 
> 2.37.1.595.g718a3a8f04-goog

-- 

- Arnaldo