linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] perf python: Add missing infra pieces for counting
@ 2025-05-19 19:51 Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 1/7] libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map Ian Rogers
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

Add the missing infra code in the perf python bindings for measuring and
reading the counter values for the given perf event. Demonstrate the
usage of this with counting.py - a python version of counting.c

Ian v2 -> v3:
1. Make the read API use CPUs and threads rather than indices as
   discussed in:
https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
2. Add python cpus and threads functions to evsel so that these can be
   iterated to avoid reading CPUs not present on a parsed evsel.
3. Clean up bits of the perf_thread_map libperf API.
3. Allow a command line event to be specified for parsing, for example:
```
$ tools/perf/python/counting.py -e data_read
For evsel(uncore_imc_free_running_0/data_read/) val: 23062 enable: 1612523 run: 1612523
For evsel(uncore_imc_free_running_1/data_read/) val: 22423 enable: 1599354 run: 1599354
```

Gautam v1 -> v2:
1. Use the existing iteration support for evlist
2. Drop the use of next method
3. Use existing helper functions for python example

Gautam Menghani (4):
  perf python: Add support for perf_counts_values to return counter data
  perf python: Add evsel read method
  perf python: Add evlist close support
  perf python: Add counting.py as example for counting perf events

Ian Rogers (3):
  libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map
  libperf threadmap: Add perf_thread_map__idx
  perf python: Add evsel cpus and threads functions

 tools/lib/perf/include/perf/threadmap.h |   1 +
 tools/lib/perf/threadmap.c              |  17 +++
 tools/perf/python/counting.py           |  36 +++++
 tools/perf/util/python.c                | 178 +++++++++++++++++++++++-
 4 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100755 tools/perf/python/counting.py

-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 1/7] libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 2/7] libperf threadmap: Add perf_thread_map__idx Ian Rogers
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

perf_thread_map__nr returns length 1 if the perf_thread_map is NULL,
meaning index 0 is valid. When perf_thread_map__pid of index 0 is read
then return the expected "any" -1 value. Assert this is only done for
index 0.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/perf/threadmap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/perf/threadmap.c b/tools/lib/perf/threadmap.c
index 07968f3ea093..3ca9ba4987fc 100644
--- a/tools/lib/perf/threadmap.c
+++ b/tools/lib/perf/threadmap.c
@@ -97,5 +97,10 @@ int perf_thread_map__nr(struct perf_thread_map *threads)
 
 pid_t perf_thread_map__pid(struct perf_thread_map *map, int idx)
 {
+	if (!map) {
+		assert(idx == 0);
+		return -1;
+	}
+
 	return map->map[idx].pid;
 }
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 2/7] libperf threadmap: Add perf_thread_map__idx
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 1/7] libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 3/7] perf python: Add evsel cpus and threads functions Ian Rogers
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

Allow computation of thread map index from a PID. Note, with a
perf_cpu_map the sorted nature allows for a binary search to compute
the index which isn't currently possible with a perf_thread_map as
they aren't guaranteed sorted.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/perf/include/perf/threadmap.h |  1 +
 tools/lib/perf/threadmap.c              | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/tools/lib/perf/include/perf/threadmap.h b/tools/lib/perf/include/perf/threadmap.h
index 8b40e7777cea..44deb815b817 100644
--- a/tools/lib/perf/include/perf/threadmap.h
+++ b/tools/lib/perf/include/perf/threadmap.h
@@ -14,6 +14,7 @@ LIBPERF_API void perf_thread_map__set_pid(struct perf_thread_map *map, int idx,
 LIBPERF_API char *perf_thread_map__comm(struct perf_thread_map *map, int idx);
 LIBPERF_API int perf_thread_map__nr(struct perf_thread_map *threads);
 LIBPERF_API pid_t perf_thread_map__pid(struct perf_thread_map *map, int idx);
+LIBPERF_API int perf_thread_map__idx(struct perf_thread_map *map, pid_t pid);
 
 LIBPERF_API struct perf_thread_map *perf_thread_map__get(struct perf_thread_map *map);
 LIBPERF_API void perf_thread_map__put(struct perf_thread_map *map);
diff --git a/tools/lib/perf/threadmap.c b/tools/lib/perf/threadmap.c
index 3ca9ba4987fc..db431b036f57 100644
--- a/tools/lib/perf/threadmap.c
+++ b/tools/lib/perf/threadmap.c
@@ -104,3 +104,15 @@ pid_t perf_thread_map__pid(struct perf_thread_map *map, int idx)
 
 	return map->map[idx].pid;
 }
+
+int perf_thread_map__idx(struct perf_thread_map *threads, pid_t pid)
+{
+	if (!threads)
+		return pid == -1 ? 0 : -1;
+
+	for (int i = 0; i < threads->nr; ++i) {
+		if (threads->map[i].pid == pid)
+			return i;
+	}
+	return -1;
+}
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 3/7] perf python: Add evsel cpus and threads functions
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 1/7] libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 2/7] libperf threadmap: Add perf_thread_map__idx Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data Ian Rogers
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

Allow access to cpus and thread_map associated with an evsel.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index f3c05da25b4a..ead3afd2d996 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -781,6 +781,27 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
 	return Py_None;
 }
 
+static PyObject *pyrf_evsel__cpus(struct pyrf_evsel *pevsel)
+{
+	struct pyrf_cpu_map *pcpu_map = PyObject_New(struct pyrf_cpu_map, &pyrf_cpu_map__type);
+
+	if (pcpu_map)
+		pcpu_map->cpus = perf_cpu_map__get(pevsel->evsel.core.cpus);
+
+	return (PyObject *)pcpu_map;
+}
+
+static PyObject *pyrf_evsel__threads(struct pyrf_evsel *pevsel)
+{
+	struct pyrf_thread_map *pthread_map =
+		PyObject_New(struct pyrf_thread_map, &pyrf_thread_map__type);
+
+	if (pthread_map)
+		pthread_map->threads = perf_thread_map__get(pevsel->evsel.core.threads);
+
+	return (PyObject *)pthread_map;
+}
+
 static PyObject *pyrf_evsel__str(PyObject *self)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
@@ -799,6 +820,18 @@ static PyMethodDef pyrf_evsel__methods[] = {
 		.ml_flags = METH_VARARGS | METH_KEYWORDS,
 		.ml_doc	  = PyDoc_STR("open the event selector file descriptor table.")
 	},
+	{
+		.ml_name  = "cpus",
+		.ml_meth  = (PyCFunction)pyrf_evsel__cpus,
+		.ml_flags = METH_NOARGS,
+		.ml_doc	  = PyDoc_STR("CPUs the event is to be used with.")
+	},
+	{
+		.ml_name  = "threads",
+		.ml_meth  = (PyCFunction)pyrf_evsel__threads,
+		.ml_flags = METH_NOARGS,
+		.ml_doc	  = PyDoc_STR("threads the event is to be used with.")
+	},
 	{ .ml_name = NULL, }
 };
 
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
                   ` (2 preceding siblings ...)
  2025-05-19 19:51 ` [PATCH v3 3/7] perf python: Add evsel cpus and threads functions Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-21 13:20   ` Arnaldo Carvalho de Melo
  2025-05-22 22:20   ` Arnaldo Carvalho de Melo
  2025-05-19 19:51 ` [PATCH v3 5/7] perf python: Add evsel read method Ian Rogers
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

From: Gautam Menghani <gautam@linux.ibm.com>

Add support for perf_counts_values struct to enable the python
bindings to read and return the counter data.

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
 tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index ead3afd2d996..1cbddfe77c7c 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
 	return PyType_Ready(&pyrf_thread_map__type);
 }
 
+struct pyrf_counts_values {
+	PyObject_HEAD
+
+	struct perf_counts_values values;
+};
+
+static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
+
+static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
+{
+	Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
+}
+
+#define counts_values_member_def(member, ptype, help) \
+	{ #member, ptype, \
+	  offsetof(struct pyrf_counts_values, values.member), \
+	  0, help }
+
+static PyMemberDef pyrf_counts_values_members[] = {
+	counts_values_member_def(val, Py_T_ULONG, "Value of event"),
+	counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
+	counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
+	counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
+	counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
+	{NULL}
+};
+
+static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
+{
+	PyObject *vals = PyList_New(5);
+
+	if (!vals)
+		return NULL;
+	for (int i = 0; i < 5; i++)
+		PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
+
+	return vals;
+}
+
+static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
+					 void *closure)
+{
+	Py_ssize_t size;
+	PyObject *item = NULL;
+
+	if (!PyList_Check(list)) {
+		PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
+		return -1;
+	}
+
+	size = PyList_Size(list);
+	for (Py_ssize_t i = 0; i < size; i++) {
+		item = PyList_GetItem(list, i);
+		if (!PyLong_Check(item)) {
+			PyErr_SetString(PyExc_TypeError, "List members should be numbers");
+			return -1;
+		}
+		self->values.values[i] = PyLong_AsLong(item);
+	}
+
+	return 0;
+}
+
+static PyGetSetDef pyrf_counts_values_getset[] = {
+	{"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
+		"Name field", NULL},
+	{NULL}
+};
+
+static PyTypeObject pyrf_counts_values__type = {
+	PyVarObject_HEAD_INIT(NULL, 0)
+	.tp_name	= "perf.counts_values",
+	.tp_basicsize	= sizeof(struct pyrf_counts_values),
+	.tp_dealloc	= (destructor)pyrf_counts_values__delete,
+	.tp_flags	= Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
+	.tp_doc		= pyrf_counts_values__doc,
+	.tp_members	= pyrf_counts_values_members,
+	.tp_getset	= pyrf_counts_values_getset,
+};
+
+static int pyrf_counts_values__setup_types(void)
+{
+	pyrf_counts_values__type.tp_new = PyType_GenericNew;
+	return PyType_Ready(&pyrf_counts_values__type);
+}
+
 struct pyrf_evsel {
 	PyObject_HEAD
 
@@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
 	    pyrf_evlist__setup_types() < 0 ||
 	    pyrf_evsel__setup_types() < 0 ||
 	    pyrf_thread_map__setup_types() < 0 ||
-	    pyrf_cpu_map__setup_types() < 0)
+	    pyrf_cpu_map__setup_types() < 0 ||
+	    pyrf_counts_values__setup_types() < 0)
 		return module;
 
 	/* The page_size is placed in util object. */
@@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
 	Py_INCREF(&pyrf_cpu_map__type);
 	PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
 
+	Py_INCREF(&pyrf_counts_values__type);
+	PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
+
 	dict = PyModule_GetDict(module);
 	if (dict == NULL)
 		goto error;
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 5/7] perf python: Add evsel read method
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
                   ` (3 preceding siblings ...)
  2025-05-19 19:51 ` [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-21 13:18   ` Arnaldo Carvalho de Melo
  2025-05-19 19:51 ` [PATCH v3 6/7] perf python: Add evlist close support Ian Rogers
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

From: Gautam Menghani <gautam@linux.ibm.com>

Add the evsel read method to enable python to read counter data for the
given evsel.

Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
Ian modified from v2 to make the API take a CPU and thread then
compute from these the appropriate indices. This was discussed as the
preferred API with Arnaldo:
https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
---
 tools/perf/util/python.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 1cbddfe77c7c..281e706e102d 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -888,6 +888,37 @@ static PyObject *pyrf_evsel__threads(struct pyrf_evsel *pevsel)
 	return (PyObject *)pthread_map;
 }
 
+static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
+				  PyObject *args, PyObject *kwargs)
+{
+	struct evsel *evsel = &pevsel->evsel;
+	int cpu = 0, cpu_idx, thread = 0, thread_idx;
+	struct perf_counts_values counts;
+	struct pyrf_counts_values *count_values = PyObject_New(struct pyrf_counts_values,
+							       &pyrf_counts_values__type);
+
+	if (!count_values)
+		return NULL;
+
+	if (!PyArg_ParseTuple(args, "ii", &cpu, &thread))
+		return NULL;
+
+	cpu_idx = perf_cpu_map__idx(evsel->core.cpus, (struct perf_cpu){.cpu = cpu});
+	if (cpu_idx < 0) {
+		PyErr_Format(PyExc_TypeError, "CPU %d is not part of evsel's CPUs", cpu);
+		return NULL;
+	}
+	thread_idx = perf_thread_map__idx(evsel->core.threads, thread);
+	if (cpu_idx < 0) {
+		PyErr_Format(PyExc_TypeError, "Thread %d is not part of evsel's threads",
+			     thread);
+		return NULL;
+	}
+	perf_evsel__read(&(evsel->core), cpu_idx, thread_idx, &counts);
+	count_values->values = counts;
+	return (PyObject *)count_values;
+}
+
 static PyObject *pyrf_evsel__str(PyObject *self)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
@@ -918,6 +949,12 @@ static PyMethodDef pyrf_evsel__methods[] = {
 		.ml_flags = METH_NOARGS,
 		.ml_doc	  = PyDoc_STR("threads the event is to be used with.")
 	},
+	{
+		.ml_name  = "read",
+		.ml_meth  = (PyCFunction)pyrf_evsel__read,
+		.ml_flags = METH_VARARGS | METH_KEYWORDS,
+		.ml_doc	  = PyDoc_STR("read counters")
+	},
 	{ .ml_name = NULL, }
 };
 
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 6/7] perf python: Add evlist close support
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
                   ` (4 preceding siblings ...)
  2025-05-19 19:51 ` [PATCH v3 5/7] perf python: Add evsel read method Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-19 19:51 ` [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events Ian Rogers
  2025-05-21 12:27 ` [PATCH v3 0/7] perf python: Add missing infra pieces for counting Gautam Menghani
  7 siblings, 0 replies; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

From: Gautam Menghani <gautam@linux.ibm.com>

Add support for the evlist close function.

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
 tools/perf/util/python.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 281e706e102d..cca744d51349 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1210,6 +1210,16 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
 	return Py_None;
 }
 
+static PyObject *pyrf_evlist__close(struct pyrf_evlist *pevlist)
+{
+	struct evlist *evlist = &pevlist->evlist;
+
+	evlist__close(evlist);
+
+	Py_INCREF(Py_None);
+	return Py_None;
+}
+
 static PyObject *pyrf_evlist__config(struct pyrf_evlist *pevlist)
 {
 	struct record_opts opts = {
@@ -1268,6 +1278,12 @@ static PyMethodDef pyrf_evlist__methods[] = {
 		.ml_flags = METH_VARARGS | METH_KEYWORDS,
 		.ml_doc	  = PyDoc_STR("open the file descriptors.")
 	},
+	{
+		.ml_name  = "close",
+		.ml_meth  = (PyCFunction)pyrf_evlist__close,
+		.ml_flags = METH_NOARGS,
+		.ml_doc	  = PyDoc_STR("close the file descriptors.")
+	},
 	{
 		.ml_name  = "poll",
 		.ml_meth  = (PyCFunction)pyrf_evlist__poll,
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
                   ` (5 preceding siblings ...)
  2025-05-19 19:51 ` [PATCH v3 6/7] perf python: Add evlist close support Ian Rogers
@ 2025-05-19 19:51 ` Ian Rogers
  2025-05-21 14:46   ` Arnaldo Carvalho de Melo
  2025-05-21 12:27 ` [PATCH v3 0/7] perf python: Add missing infra pieces for counting Gautam Menghani
  7 siblings, 1 reply; 20+ messages in thread
From: Ian Rogers @ 2025-05-19 19:51 UTC (permalink / raw)
  To: Gautam Menghani, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users
  Cc: maddy

From: Gautam Menghani <gautam@linux.ibm.com>

Add counting.py - a python version of counting.c to demonstrate
measuring and reading of counts for given perf events.

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
Ian modified from v2 to make the API take a CPU and thread then
compute from these the appropriate indices. This was discussed as the
preferred API with Arnaldo:
https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
The use of a thread_map and cpu_map was also removed to make the code
cleaner, instead the cpus and threads of the parsed evsel are
used. Support for command line events is also added. The indent is
reduced from 8 to 4 to match the preferred python PEP8 indent.
---
 tools/perf/python/counting.py | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100755 tools/perf/python/counting.py

diff --git a/tools/perf/python/counting.py b/tools/perf/python/counting.py
new file mode 100755
index 000000000000..02121d2bb11d
--- /dev/null
+++ b/tools/perf/python/counting.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# -*- python -*-
+# -*- coding: utf-8 -*-
+
+import argparse
+import perf
+
+def main(event: str):
+    evlist = perf.parse_events(event)
+
+    for evsel in evlist:
+        evsel.read_format = perf.FORMAT_TOTAL_TIME_ENABLED | perf.FORMAT_TOTAL_TIME_RUNNING
+
+    evlist.open()
+    evlist.enable()
+
+    count = 100000
+    while count > 0:
+        count -= 1
+
+    evlist.disable()
+
+    for evsel in evlist:
+        for cpu in evsel.cpus():
+            for thread in evsel.threads():
+                counts = evsel.read(cpu, thread)
+                print(f"For {evsel} val: {counts.val} enable: {counts.ena} run: {counts.run}")
+
+    evlist.close()
+
+if __name__ == '__main__':
+    ap = argparse.ArgumentParser()
+    ap.add_argument('-e', '--event', help="Events to open", default="cpu-clock,task-clock")
+    args = ap.parse_args()
+    main(args.event)
-- 
2.49.0.1101.gccaa498523-goog


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 0/7] perf python: Add missing infra pieces for counting
  2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
                   ` (6 preceding siblings ...)
  2025-05-19 19:51 ` [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events Ian Rogers
@ 2025-05-21 12:27 ` Gautam Menghani
  7 siblings, 0 replies; 20+ messages in thread
From: Gautam Menghani @ 2025-05-21 12:27 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Adrian Hunter, Kan Liang, Howard Chu, linux-kernel,
	linux-perf-users, maddy

Hi Ian,

Thanks for adding the API related changes, LGTM

Thanks,
Gautam

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 5/7] perf python: Add evsel read method
  2025-05-19 19:51 ` [PATCH v3 5/7] perf python: Add evsel read method Ian Rogers
@ 2025-05-21 13:18   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-21 13:18 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Mon, May 19, 2025 at 12:51:42PM -0700, Ian Rogers wrote:
> From: Gautam Menghani <gautam@linux.ibm.com>
> 
> Add the evsel read method to enable python to read counter data for the
> given evsel.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
> Ian modified from v2 to make the API take a CPU and thread then

I'll add this note right before your Signed-off-by, with a link to the
discussion, i.e. as:

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
Link: https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
[ make the API take a CPU and thread then compute from these the appropriate indices.
Signed-off-by: Ian Rogers <irogers@google.com>

> compute from these the appropriate indices. This was discussed as the
> preferred API with Arnaldo:
> https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
> ---
>  tools/perf/util/python.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 1cbddfe77c7c..281e706e102d 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -888,6 +888,37 @@ static PyObject *pyrf_evsel__threads(struct pyrf_evsel *pevsel)
>  	return (PyObject *)pthread_map;
>  }
>  
> +static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
> +				  PyObject *args, PyObject *kwargs)
> +{
> +	struct evsel *evsel = &pevsel->evsel;
> +	int cpu = 0, cpu_idx, thread = 0, thread_idx;
> +	struct perf_counts_values counts;
> +	struct pyrf_counts_values *count_values = PyObject_New(struct pyrf_counts_values,
> +							       &pyrf_counts_values__type);
> +
> +	if (!count_values)
> +		return NULL;
> +
> +	if (!PyArg_ParseTuple(args, "ii", &cpu, &thread))
> +		return NULL;
> +
> +	cpu_idx = perf_cpu_map__idx(evsel->core.cpus, (struct perf_cpu){.cpu = cpu});
> +	if (cpu_idx < 0) {
> +		PyErr_Format(PyExc_TypeError, "CPU %d is not part of evsel's CPUs", cpu);
> +		return NULL;
> +	}
> +	thread_idx = perf_thread_map__idx(evsel->core.threads, thread);
> +	if (cpu_idx < 0) {
> +		PyErr_Format(PyExc_TypeError, "Thread %d is not part of evsel's threads",
> +			     thread);
> +		return NULL;
> +	}
> +	perf_evsel__read(&(evsel->core), cpu_idx, thread_idx, &counts);
> +	count_values->values = counts;
> +	return (PyObject *)count_values;
> +}
> +
>  static PyObject *pyrf_evsel__str(PyObject *self)
>  {
>  	struct pyrf_evsel *pevsel = (void *)self;
> @@ -918,6 +949,12 @@ static PyMethodDef pyrf_evsel__methods[] = {
>  		.ml_flags = METH_NOARGS,
>  		.ml_doc	  = PyDoc_STR("threads the event is to be used with.")
>  	},
> +	{
> +		.ml_name  = "read",
> +		.ml_meth  = (PyCFunction)pyrf_evsel__read,
> +		.ml_flags = METH_VARARGS | METH_KEYWORDS,
> +		.ml_doc	  = PyDoc_STR("read counters")
> +	},
>  	{ .ml_name = NULL, }
>  };
>  
> -- 
> 2.49.0.1101.gccaa498523-goog
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-19 19:51 ` [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data Ian Rogers
@ 2025-05-21 13:20   ` Arnaldo Carvalho de Melo
  2025-05-21 13:56     ` Ian Rogers
  2025-05-22 22:20   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-21 13:20 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> From: Gautam Menghani <gautam@linux.ibm.com>
> 
> Add support for perf_counts_values struct to enable the python
> bindings to read and return the counter data.

This (and another one in this series) are coming from Ian, that didn't
modify them, so we need a Signed-off-by: Ian, as per:

Documentation/process/submitting-patches.rst

---
Any further SoBs (Signed-off-by:'s) following the author's SoB are from
people handling and transporting the patch, but were not involved in its
development. SoB chains should reflect the **real** route a patch took
as it was propagated to the maintainers and ultimately to Linus, with
the first SoB entry signalling primary authorship of a single author.
---

I'm adding them to my local branch, please ack this,

Thanks,

- Arnaldo
 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index ead3afd2d996..1cbddfe77c7c 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
>  	return PyType_Ready(&pyrf_thread_map__type);
>  }
>  
> +struct pyrf_counts_values {
> +	PyObject_HEAD
> +
> +	struct perf_counts_values values;
> +};
> +
> +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> +
> +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> +{
> +	Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> +}
> +
> +#define counts_values_member_def(member, ptype, help) \
> +	{ #member, ptype, \
> +	  offsetof(struct pyrf_counts_values, values.member), \
> +	  0, help }
> +
> +static PyMemberDef pyrf_counts_values_members[] = {
> +	counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> +	counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> +	counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> +	counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> +	counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> +	{NULL}
> +};
> +
> +static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
> +{
> +	PyObject *vals = PyList_New(5);
> +
> +	if (!vals)
> +		return NULL;
> +	for (int i = 0; i < 5; i++)
> +		PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
> +
> +	return vals;
> +}
> +
> +static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
> +					 void *closure)
> +{
> +	Py_ssize_t size;
> +	PyObject *item = NULL;
> +
> +	if (!PyList_Check(list)) {
> +		PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
> +		return -1;
> +	}
> +
> +	size = PyList_Size(list);
> +	for (Py_ssize_t i = 0; i < size; i++) {
> +		item = PyList_GetItem(list, i);
> +		if (!PyLong_Check(item)) {
> +			PyErr_SetString(PyExc_TypeError, "List members should be numbers");
> +			return -1;
> +		}
> +		self->values.values[i] = PyLong_AsLong(item);
> +	}
> +
> +	return 0;
> +}
> +
> +static PyGetSetDef pyrf_counts_values_getset[] = {
> +	{"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
> +		"Name field", NULL},
> +	{NULL}
> +};
> +
> +static PyTypeObject pyrf_counts_values__type = {
> +	PyVarObject_HEAD_INIT(NULL, 0)
> +	.tp_name	= "perf.counts_values",
> +	.tp_basicsize	= sizeof(struct pyrf_counts_values),
> +	.tp_dealloc	= (destructor)pyrf_counts_values__delete,
> +	.tp_flags	= Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
> +	.tp_doc		= pyrf_counts_values__doc,
> +	.tp_members	= pyrf_counts_values_members,
> +	.tp_getset	= pyrf_counts_values_getset,
> +};
> +
> +static int pyrf_counts_values__setup_types(void)
> +{
> +	pyrf_counts_values__type.tp_new = PyType_GenericNew;
> +	return PyType_Ready(&pyrf_counts_values__type);
> +}
> +
>  struct pyrf_evsel {
>  	PyObject_HEAD
>  
> @@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
>  	    pyrf_evlist__setup_types() < 0 ||
>  	    pyrf_evsel__setup_types() < 0 ||
>  	    pyrf_thread_map__setup_types() < 0 ||
> -	    pyrf_cpu_map__setup_types() < 0)
> +	    pyrf_cpu_map__setup_types() < 0 ||
> +	    pyrf_counts_values__setup_types() < 0)
>  		return module;
>  
>  	/* The page_size is placed in util object. */
> @@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
>  	Py_INCREF(&pyrf_cpu_map__type);
>  	PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
>  
> +	Py_INCREF(&pyrf_counts_values__type);
> +	PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
> +
>  	dict = PyModule_GetDict(module);
>  	if (dict == NULL)
>  		goto error;
> -- 
> 2.49.0.1101.gccaa498523-goog

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-21 13:20   ` Arnaldo Carvalho de Melo
@ 2025-05-21 13:56     ` Ian Rogers
  2025-05-21 17:26       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Rogers @ 2025-05-21 13:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Wed, May 21, 2025 at 6:20 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > From: Gautam Menghani <gautam@linux.ibm.com>
> >
> > Add support for perf_counts_values struct to enable the python
> > bindings to read and return the counter data.
>
> This (and another one in this series) are coming from Ian, that didn't
> modify them, so we need a Signed-off-by: Ian, as per:
>
> Documentation/process/submitting-patches.rst
>
> ---
> Any further SoBs (Signed-off-by:'s) following the author's SoB are from
> people handling and transporting the patch, but were not involved in its
> development. SoB chains should reflect the **real** route a patch took
> as it was propagated to the maintainers and ultimately to Linus, with
> the first SoB entry signalling primary authorship of a single author.
> ---
>
> I'm adding them to my local branch, please ack this,

Ack. Thanks and sorry for the work, will try to do better next time.

Ian

> Thanks,
>
> - Arnaldo
>
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > ---
> >  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 91 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index ead3afd2d996..1cbddfe77c7c 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
> >       return PyType_Ready(&pyrf_thread_map__type);
> >  }
> >
> > +struct pyrf_counts_values {
> > +     PyObject_HEAD
> > +
> > +     struct perf_counts_values values;
> > +};
> > +
> > +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> > +
> > +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> > +{
> > +     Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> > +}
> > +
> > +#define counts_values_member_def(member, ptype, help) \
> > +     { #member, ptype, \
> > +       offsetof(struct pyrf_counts_values, values.member), \
> > +       0, help }
> > +
> > +static PyMemberDef pyrf_counts_values_members[] = {
> > +     counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> > +     counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> > +     counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> > +     counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> > +     counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> > +     {NULL}
> > +};
> > +
> > +static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
> > +{
> > +     PyObject *vals = PyList_New(5);
> > +
> > +     if (!vals)
> > +             return NULL;
> > +     for (int i = 0; i < 5; i++)
> > +             PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
> > +
> > +     return vals;
> > +}
> > +
> > +static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
> > +                                      void *closure)
> > +{
> > +     Py_ssize_t size;
> > +     PyObject *item = NULL;
> > +
> > +     if (!PyList_Check(list)) {
> > +             PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
> > +             return -1;
> > +     }
> > +
> > +     size = PyList_Size(list);
> > +     for (Py_ssize_t i = 0; i < size; i++) {
> > +             item = PyList_GetItem(list, i);
> > +             if (!PyLong_Check(item)) {
> > +                     PyErr_SetString(PyExc_TypeError, "List members should be numbers");
> > +                     return -1;
> > +             }
> > +             self->values.values[i] = PyLong_AsLong(item);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static PyGetSetDef pyrf_counts_values_getset[] = {
> > +     {"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
> > +             "Name field", NULL},
> > +     {NULL}
> > +};
> > +
> > +static PyTypeObject pyrf_counts_values__type = {
> > +     PyVarObject_HEAD_INIT(NULL, 0)
> > +     .tp_name        = "perf.counts_values",
> > +     .tp_basicsize   = sizeof(struct pyrf_counts_values),
> > +     .tp_dealloc     = (destructor)pyrf_counts_values__delete,
> > +     .tp_flags       = Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
> > +     .tp_doc         = pyrf_counts_values__doc,
> > +     .tp_members     = pyrf_counts_values_members,
> > +     .tp_getset      = pyrf_counts_values_getset,
> > +};
> > +
> > +static int pyrf_counts_values__setup_types(void)
> > +{
> > +     pyrf_counts_values__type.tp_new = PyType_GenericNew;
> > +     return PyType_Ready(&pyrf_counts_values__type);
> > +}
> > +
> >  struct pyrf_evsel {
> >       PyObject_HEAD
> >
> > @@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
> >           pyrf_evlist__setup_types() < 0 ||
> >           pyrf_evsel__setup_types() < 0 ||
> >           pyrf_thread_map__setup_types() < 0 ||
> > -         pyrf_cpu_map__setup_types() < 0)
> > +         pyrf_cpu_map__setup_types() < 0 ||
> > +         pyrf_counts_values__setup_types() < 0)
> >               return module;
> >
> >       /* The page_size is placed in util object. */
> > @@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
> >       Py_INCREF(&pyrf_cpu_map__type);
> >       PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
> >
> > +     Py_INCREF(&pyrf_counts_values__type);
> > +     PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
> > +
> >       dict = PyModule_GetDict(module);
> >       if (dict == NULL)
> >               goto error;
> > --
> > 2.49.0.1101.gccaa498523-goog

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events
  2025-05-19 19:51 ` [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events Ian Rogers
@ 2025-05-21 14:46   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-21 14:46 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Mon, May 19, 2025 at 12:51:44PM -0700, Ian Rogers wrote:
> From: Gautam Menghani <gautam@linux.ibm.com>
> 
> Add counting.py - a python version of counting.c to demonstrate
> measuring and reading of counts for given perf events.
> 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> Signed-off-by: Ian Rogers <irogers@google.com>

Tested and applied:

Committer testing:

Build perf and make the generated python binding somewhere you can point
to to avoid using the one in the distro python3-perf (fedora, may be
different in other distros):

  $ make -k O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin

Copy /tmp/build/perf-tools-next/python/perf.cpython-313-x86_64-linux-gnu.so to
somewhere outside this toolbox container and then use it with root:

  # export PYTHONPATH=/root/python/
  # ls -la /root/python/
  total 10640
  drwxr-xr-x. 1 root root       72 May 21 11:40 .
  dr-xr-x---. 1 root root      574 May 21 11:40 ..
  -rwxr-xr-x. 1 acme acme 10894360 May 21 11:40 perf.cpython-313-x86_64-linux-gnu.so
  # tools/perf/python/counting.py | head -5
  For evsel(software/cpu-clock/) val: 2930946 enable: 2932479 run: 2932479
  For evsel(software/cpu-clock/) val: 2924975 enable: 2926267 run: 2926267
  For evsel(software/cpu-clock/) val: 2921017 enable: 2922430 run: 2922430
  For evsel(software/cpu-clock/) val: 2914966 enable: 2916549 run: 2916549
  For evsel(software/cpu-clock/) val: 2910027 enable: 2911589 run: 2911589
  #

It would be nice to have something that compares the output for some
envent obtained from both 'perf stat' and using these new python
counting classes, but that can be done later.

Applied to perf-tools-next,

- Arnaldo

> ---
> Ian modified from v2 to make the API take a CPU and thread then
> compute from these the appropriate indices. This was discussed as the
> preferred API with Arnaldo:
> https://lore.kernel.org/linux-perf-users/20250512055748.479786-1-gautam@linux.ibm.com/
> The use of a thread_map and cpu_map was also removed to make the code
> cleaner, instead the cpus and threads of the parsed evsel are
> used. Support for command line events is also added. The indent is
> reduced from 8 to 4 to match the preferred python PEP8 indent.
> ---
>  tools/perf/python/counting.py | 36 +++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100755 tools/perf/python/counting.py
> 
> diff --git a/tools/perf/python/counting.py b/tools/perf/python/counting.py
> new file mode 100755
> index 000000000000..02121d2bb11d
> --- /dev/null
> +++ b/tools/perf/python/counting.py
> @@ -0,0 +1,36 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +# -*- python -*-
> +# -*- coding: utf-8 -*-
> +
> +import argparse
> +import perf
> +
> +def main(event: str):
> +    evlist = perf.parse_events(event)
> +
> +    for evsel in evlist:
> +        evsel.read_format = perf.FORMAT_TOTAL_TIME_ENABLED | perf.FORMAT_TOTAL_TIME_RUNNING
> +
> +    evlist.open()
> +    evlist.enable()
> +
> +    count = 100000
> +    while count > 0:
> +        count -= 1
> +
> +    evlist.disable()
> +
> +    for evsel in evlist:
> +        for cpu in evsel.cpus():
> +            for thread in evsel.threads():
> +                counts = evsel.read(cpu, thread)
> +                print(f"For {evsel} val: {counts.val} enable: {counts.ena} run: {counts.run}")
> +
> +    evlist.close()
> +
> +if __name__ == '__main__':
> +    ap = argparse.ArgumentParser()
> +    ap.add_argument('-e', '--event', help="Events to open", default="cpu-clock,task-clock")
> +    args = ap.parse_args()
> +    main(args.event)
> -- 
> 2.49.0.1101.gccaa498523-goog
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-21 13:56     ` Ian Rogers
@ 2025-05-21 17:26       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-21 17:26 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Wed, May 21, 2025 at 06:56:44AM -0700, Ian Rogers wrote:
> On Wed, May 21, 2025 at 6:20 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > > From: Gautam Menghani <gautam@linux.ibm.com>
> > >
> > > Add support for perf_counts_values struct to enable the python
> > > bindings to read and return the counter data.
> >
> > This (and another one in this series) are coming from Ian, that didn't
> > modify them, so we need a Signed-off-by: Ian, as per:
> >
> > Documentation/process/submitting-patches.rst
> >
> > ---
> > Any further SoBs (Signed-off-by:'s) following the author's SoB are from
> > people handling and transporting the patch, but were not involved in its
> > development. SoB chains should reflect the **real** route a patch took
> > as it was propagated to the maintainers and ultimately to Linus, with
> > the first SoB entry signalling primary authorship of a single author.
> > ---
> >
> > I'm adding them to my local branch, please ack this,
> 
> Ack. Thanks and sorry for the work, will try to do better next time.

Thanks for confirming,

- Arnaldo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-19 19:51 ` [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data Ian Rogers
  2025-05-21 13:20   ` Arnaldo Carvalho de Melo
@ 2025-05-22 22:20   ` Arnaldo Carvalho de Melo
  2025-05-22 22:32     ` Ian Rogers
  1 sibling, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 22:20 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> From: Gautam Menghani <gautam@linux.ibm.com>
> 
> Add support for perf_counts_values struct to enable the python
> bindings to read and return the counter data.
> 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index ead3afd2d996..1cbddfe77c7c 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
>  	return PyType_Ready(&pyrf_thread_map__type);
>  }
>  
> +struct pyrf_counts_values {
> +	PyObject_HEAD
> +
> +	struct perf_counts_values values;
> +};
> +
> +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> +
> +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> +{
> +	Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> +}
> +
> +#define counts_values_member_def(member, ptype, help) \
> +	{ #member, ptype, \
> +	  offsetof(struct pyrf_counts_values, values.member), \
> +	  0, help }
> +
> +static PyMemberDef pyrf_counts_values_members[] = {
> +	counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> +	counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> +	counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> +	counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> +	counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> +	{NULL}
> +};

So the above is failing on a aarch64 debian (rpi5):

acme@raspberrypi:~/git/perf-tools-next $ dpkg -S /usr/include/python3.11/structmember.h
libpython3.11-dev:arm64: /usr/include/python3.11/structmember.h
acme@raspberrypi:~/git/perf-tools-next $

Where it only has:

acme@raspberrypi:~/git/perf-tools-next $ grep -r Py_T_ULONG /usr/include/
acme@raspberrypi:~/git/perf-tools-next $ grep -rw Py_T_ULONG /usr/include/
acme@raspberrypi:~/git/perf-tools-next $ grep -rw T_ULONG /usr/include/
/usr/include/python3.11/structmember.h:#define T_ULONG     12
acme@raspberrypi:~/git/perf-tools-next $

while on fedora 42 x86_64:

⬢ [acme@toolbx perf-tools-next]$ grep -rw Py_T_ULONG /usr/include/
/usr/include/python3.13/descrobject.h:#define Py_T_ULONG     12
/usr/include/python3.13/structmember.h:#define T_ULONG     Py_T_ULONG
⬢ [acme@toolbx perf-tools-next]$

So I'm making it use the T_ULONG and others as all the other PyMemberDef
arrays in tools/perf/util/python.c, ok?

- Arnaldo

> +static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
> +{
> +	PyObject *vals = PyList_New(5);
> +
> +	if (!vals)
> +		return NULL;
> +	for (int i = 0; i < 5; i++)
> +		PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
> +
> +	return vals;
> +}
> +
> +static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
> +					 void *closure)
> +{
> +	Py_ssize_t size;
> +	PyObject *item = NULL;
> +
> +	if (!PyList_Check(list)) {
> +		PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
> +		return -1;
> +	}
> +
> +	size = PyList_Size(list);
> +	for (Py_ssize_t i = 0; i < size; i++) {
> +		item = PyList_GetItem(list, i);
> +		if (!PyLong_Check(item)) {
> +			PyErr_SetString(PyExc_TypeError, "List members should be numbers");
> +			return -1;
> +		}
> +		self->values.values[i] = PyLong_AsLong(item);
> +	}
> +
> +	return 0;
> +}
> +
> +static PyGetSetDef pyrf_counts_values_getset[] = {
> +	{"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
> +		"Name field", NULL},
> +	{NULL}
> +};
> +
> +static PyTypeObject pyrf_counts_values__type = {
> +	PyVarObject_HEAD_INIT(NULL, 0)
> +	.tp_name	= "perf.counts_values",
> +	.tp_basicsize	= sizeof(struct pyrf_counts_values),
> +	.tp_dealloc	= (destructor)pyrf_counts_values__delete,
> +	.tp_flags	= Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
> +	.tp_doc		= pyrf_counts_values__doc,
> +	.tp_members	= pyrf_counts_values_members,
> +	.tp_getset	= pyrf_counts_values_getset,
> +};
> +
> +static int pyrf_counts_values__setup_types(void)
> +{
> +	pyrf_counts_values__type.tp_new = PyType_GenericNew;
> +	return PyType_Ready(&pyrf_counts_values__type);
> +}
> +
>  struct pyrf_evsel {
>  	PyObject_HEAD
>  
> @@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
>  	    pyrf_evlist__setup_types() < 0 ||
>  	    pyrf_evsel__setup_types() < 0 ||
>  	    pyrf_thread_map__setup_types() < 0 ||
> -	    pyrf_cpu_map__setup_types() < 0)
> +	    pyrf_cpu_map__setup_types() < 0 ||
> +	    pyrf_counts_values__setup_types() < 0)
>  		return module;
>  
>  	/* The page_size is placed in util object. */
> @@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
>  	Py_INCREF(&pyrf_cpu_map__type);
>  	PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
>  
> +	Py_INCREF(&pyrf_counts_values__type);
> +	PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
> +
>  	dict = PyModule_GetDict(module);
>  	if (dict == NULL)
>  		goto error;
> -- 
> 2.49.0.1101.gccaa498523-goog

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-22 22:20   ` Arnaldo Carvalho de Melo
@ 2025-05-22 22:32     ` Ian Rogers
  2025-05-22 22:36       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Rogers @ 2025-05-22 22:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Thu, May 22, 2025 at 3:20 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > From: Gautam Menghani <gautam@linux.ibm.com>
> >
> > Add support for perf_counts_values struct to enable the python
> > bindings to read and return the counter data.
> >
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > ---
> >  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 91 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index ead3afd2d996..1cbddfe77c7c 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
> >       return PyType_Ready(&pyrf_thread_map__type);
> >  }
> >
> > +struct pyrf_counts_values {
> > +     PyObject_HEAD
> > +
> > +     struct perf_counts_values values;
> > +};
> > +
> > +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> > +
> > +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> > +{
> > +     Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> > +}
> > +
> > +#define counts_values_member_def(member, ptype, help) \
> > +     { #member, ptype, \
> > +       offsetof(struct pyrf_counts_values, values.member), \
> > +       0, help }
> > +
> > +static PyMemberDef pyrf_counts_values_members[] = {
> > +     counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> > +     counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> > +     counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> > +     counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> > +     counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> > +     {NULL}
> > +};
>
> So the above is failing on a aarch64 debian (rpi5):
>
> acme@raspberrypi:~/git/perf-tools-next $ dpkg -S /usr/include/python3.11/structmember.h
> libpython3.11-dev:arm64: /usr/include/python3.11/structmember.h
> acme@raspberrypi:~/git/perf-tools-next $
>
> Where it only has:
>
> acme@raspberrypi:~/git/perf-tools-next $ grep -r Py_T_ULONG /usr/include/
> acme@raspberrypi:~/git/perf-tools-next $ grep -rw Py_T_ULONG /usr/include/
> acme@raspberrypi:~/git/perf-tools-next $ grep -rw T_ULONG /usr/include/
> /usr/include/python3.11/structmember.h:#define T_ULONG     12
> acme@raspberrypi:~/git/perf-tools-next $
>
> while on fedora 42 x86_64:
>
> ⬢ [acme@toolbx perf-tools-next]$ grep -rw Py_T_ULONG /usr/include/
> /usr/include/python3.13/descrobject.h:#define Py_T_ULONG     12
> /usr/include/python3.13/structmember.h:#define T_ULONG     Py_T_ULONG
> ⬢ [acme@toolbx perf-tools-next]$
>
> So I'm making it use the T_ULONG and others as all the other PyMemberDef
> arrays in tools/perf/util/python.c, ok?

The fix makes sense to me. Checking the documentation it seems
Py_T_ULONG is preferred:
https://docs.python.org/3/c-api/structures.html#member-types
perhaps we can add:
```
#ifndef Py_T_ULONG
#define Py_T_ULONG T_ULONG
#endif
```
so that we use the approach matching the documentation while fixing
the RaspberryPi issue.

Thanks,
Ian

> - Arnaldo
>
> > +static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
> > +{
> > +     PyObject *vals = PyList_New(5);
> > +
> > +     if (!vals)
> > +             return NULL;
> > +     for (int i = 0; i < 5; i++)
> > +             PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
> > +
> > +     return vals;
> > +}
> > +
> > +static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
> > +                                      void *closure)
> > +{
> > +     Py_ssize_t size;
> > +     PyObject *item = NULL;
> > +
> > +     if (!PyList_Check(list)) {
> > +             PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
> > +             return -1;
> > +     }
> > +
> > +     size = PyList_Size(list);
> > +     for (Py_ssize_t i = 0; i < size; i++) {
> > +             item = PyList_GetItem(list, i);
> > +             if (!PyLong_Check(item)) {
> > +                     PyErr_SetString(PyExc_TypeError, "List members should be numbers");
> > +                     return -1;
> > +             }
> > +             self->values.values[i] = PyLong_AsLong(item);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static PyGetSetDef pyrf_counts_values_getset[] = {
> > +     {"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
> > +             "Name field", NULL},
> > +     {NULL}
> > +};
> > +
> > +static PyTypeObject pyrf_counts_values__type = {
> > +     PyVarObject_HEAD_INIT(NULL, 0)
> > +     .tp_name        = "perf.counts_values",
> > +     .tp_basicsize   = sizeof(struct pyrf_counts_values),
> > +     .tp_dealloc     = (destructor)pyrf_counts_values__delete,
> > +     .tp_flags       = Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
> > +     .tp_doc         = pyrf_counts_values__doc,
> > +     .tp_members     = pyrf_counts_values_members,
> > +     .tp_getset      = pyrf_counts_values_getset,
> > +};
> > +
> > +static int pyrf_counts_values__setup_types(void)
> > +{
> > +     pyrf_counts_values__type.tp_new = PyType_GenericNew;
> > +     return PyType_Ready(&pyrf_counts_values__type);
> > +}
> > +
> >  struct pyrf_evsel {
> >       PyObject_HEAD
> >
> > @@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
> >           pyrf_evlist__setup_types() < 0 ||
> >           pyrf_evsel__setup_types() < 0 ||
> >           pyrf_thread_map__setup_types() < 0 ||
> > -         pyrf_cpu_map__setup_types() < 0)
> > +         pyrf_cpu_map__setup_types() < 0 ||
> > +         pyrf_counts_values__setup_types() < 0)
> >               return module;
> >
> >       /* The page_size is placed in util object. */
> > @@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
> >       Py_INCREF(&pyrf_cpu_map__type);
> >       PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
> >
> > +     Py_INCREF(&pyrf_counts_values__type);
> > +     PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
> > +
> >       dict = PyModule_GetDict(module);
> >       if (dict == NULL)
> >               goto error;
> > --
> > 2.49.0.1101.gccaa498523-goog

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-22 22:32     ` Ian Rogers
@ 2025-05-22 22:36       ` Arnaldo Carvalho de Melo
  2025-05-22 22:37         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 22:36 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Thu, May 22, 2025 at 03:32:44PM -0700, Ian Rogers wrote:
> On Thu, May 22, 2025 at 3:20 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > > From: Gautam Menghani <gautam@linux.ibm.com>
> > >
> > > Add support for perf_counts_values struct to enable the python
> > > bindings to read and return the counter data.
> > >
> > > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > > ---
> > >  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 91 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > > index ead3afd2d996..1cbddfe77c7c 100644
> > > --- a/tools/perf/util/python.c
> > > +++ b/tools/perf/util/python.c
> > > @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
> > >       return PyType_Ready(&pyrf_thread_map__type);
> > >  }
> > >
> > > +struct pyrf_counts_values {
> > > +     PyObject_HEAD
> > > +
> > > +     struct perf_counts_values values;
> > > +};
> > > +
> > > +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> > > +
> > > +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> > > +{
> > > +     Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> > > +}
> > > +
> > > +#define counts_values_member_def(member, ptype, help) \
> > > +     { #member, ptype, \
> > > +       offsetof(struct pyrf_counts_values, values.member), \
> > > +       0, help }
> > > +
> > > +static PyMemberDef pyrf_counts_values_members[] = {
> > > +     counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> > > +     counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> > > +     counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> > > +     counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> > > +     counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> > > +     {NULL}
> > > +};
> >
> > So the above is failing on a aarch64 debian (rpi5):
> >
> > acme@raspberrypi:~/git/perf-tools-next $ dpkg -S /usr/include/python3.11/structmember.h
> > libpython3.11-dev:arm64: /usr/include/python3.11/structmember.h
> > acme@raspberrypi:~/git/perf-tools-next $
> >
> > Where it only has:
> >
> > acme@raspberrypi:~/git/perf-tools-next $ grep -r Py_T_ULONG /usr/include/
> > acme@raspberrypi:~/git/perf-tools-next $ grep -rw Py_T_ULONG /usr/include/
> > acme@raspberrypi:~/git/perf-tools-next $ grep -rw T_ULONG /usr/include/
> > /usr/include/python3.11/structmember.h:#define T_ULONG     12
> > acme@raspberrypi:~/git/perf-tools-next $
> >
> > while on fedora 42 x86_64:
> >
> > ⬢ [acme@toolbx perf-tools-next]$ grep -rw Py_T_ULONG /usr/include/
> > /usr/include/python3.13/descrobject.h:#define Py_T_ULONG     12
> > /usr/include/python3.13/structmember.h:#define T_ULONG     Py_T_ULONG
> > ⬢ [acme@toolbx perf-tools-next]$
> >
> > So I'm making it use the T_ULONG and others as all the other PyMemberDef
> > arrays in tools/perf/util/python.c, ok?
> 
> The fix makes sense to me. Checking the documentation it seems
> Py_T_ULONG is preferred:
> https://docs.python.org/3/c-api/structures.html#member-types
> perhaps we can add:
> ```
> #ifndef Py_T_ULONG
> #define Py_T_ULONG T_ULONG
> #endif
> ```
> so that we use the approach matching the documentation while fixing
> the RaspberryPi issue.

That can be done as a followup, as there are lots of preexisting usage
for struct method definitions.

- Arnaldo
 
> Thanks,
> Ian
> 
> > - Arnaldo
> >
> > > +static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)
> > > +{
> > > +     PyObject *vals = PyList_New(5);
> > > +
> > > +     if (!vals)
> > > +             return NULL;
> > > +     for (int i = 0; i < 5; i++)
> > > +             PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
> > > +
> > > +     return vals;
> > > +}
> > > +
> > > +static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObject *list,
> > > +                                      void *closure)
> > > +{
> > > +     Py_ssize_t size;
> > > +     PyObject *item = NULL;
> > > +
> > > +     if (!PyList_Check(list)) {
> > > +             PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
> > > +             return -1;
> > > +     }
> > > +
> > > +     size = PyList_Size(list);
> > > +     for (Py_ssize_t i = 0; i < size; i++) {
> > > +             item = PyList_GetItem(list, i);
> > > +             if (!PyLong_Check(item)) {
> > > +                     PyErr_SetString(PyExc_TypeError, "List members should be numbers");
> > > +                     return -1;
> > > +             }
> > > +             self->values.values[i] = PyLong_AsLong(item);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static PyGetSetDef pyrf_counts_values_getset[] = {
> > > +     {"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
> > > +             "Name field", NULL},
> > > +     {NULL}
> > > +};
> > > +
> > > +static PyTypeObject pyrf_counts_values__type = {
> > > +     PyVarObject_HEAD_INIT(NULL, 0)
> > > +     .tp_name        = "perf.counts_values",
> > > +     .tp_basicsize   = sizeof(struct pyrf_counts_values),
> > > +     .tp_dealloc     = (destructor)pyrf_counts_values__delete,
> > > +     .tp_flags       = Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE,
> > > +     .tp_doc         = pyrf_counts_values__doc,
> > > +     .tp_members     = pyrf_counts_values_members,
> > > +     .tp_getset      = pyrf_counts_values_getset,
> > > +};
> > > +
> > > +static int pyrf_counts_values__setup_types(void)
> > > +{
> > > +     pyrf_counts_values__type.tp_new = PyType_GenericNew;
> > > +     return PyType_Ready(&pyrf_counts_values__type);
> > > +}
> > > +
> > >  struct pyrf_evsel {
> > >       PyObject_HEAD
> > >
> > > @@ -1475,7 +1561,8 @@ PyMODINIT_FUNC PyInit_perf(void)
> > >           pyrf_evlist__setup_types() < 0 ||
> > >           pyrf_evsel__setup_types() < 0 ||
> > >           pyrf_thread_map__setup_types() < 0 ||
> > > -         pyrf_cpu_map__setup_types() < 0)
> > > +         pyrf_cpu_map__setup_types() < 0 ||
> > > +         pyrf_counts_values__setup_types() < 0)
> > >               return module;
> > >
> > >       /* The page_size is placed in util object. */
> > > @@ -1520,6 +1607,9 @@ PyMODINIT_FUNC PyInit_perf(void)
> > >       Py_INCREF(&pyrf_cpu_map__type);
> > >       PyModule_AddObject(module, "cpu_map", (PyObject*)&pyrf_cpu_map__type);
> > >
> > > +     Py_INCREF(&pyrf_counts_values__type);
> > > +     PyModule_AddObject(module, "counts_values", (PyObject *)&pyrf_counts_values__type);
> > > +
> > >       dict = PyModule_GetDict(module);
> > >       if (dict == NULL)
> > >               goto error;
> > > --
> > > 2.49.0.1101.gccaa498523-goog

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-22 22:36       ` Arnaldo Carvalho de Melo
@ 2025-05-22 22:37         ` Arnaldo Carvalho de Melo
  2025-05-23  1:28           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-22 22:37 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Thu, May 22, 2025 at 07:36:31PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, May 22, 2025 at 03:32:44PM -0700, Ian Rogers wrote:
> > On Thu, May 22, 2025 at 3:20 PM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > > > From: Gautam Menghani <gautam@linux.ibm.com>
> > > >
> > > > Add support for perf_counts_values struct to enable the python
> > > > bindings to read and return the counter data.
> > > >
> > > > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > > > ---
> > > >  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 91 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > > > index ead3afd2d996..1cbddfe77c7c 100644
> > > > --- a/tools/perf/util/python.c
> > > > +++ b/tools/perf/util/python.c
> > > > @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
> > > >       return PyType_Ready(&pyrf_thread_map__type);
> > > >  }
> > > >
> > > > +struct pyrf_counts_values {
> > > > +     PyObject_HEAD
> > > > +
> > > > +     struct perf_counts_values values;
> > > > +};
> > > > +
> > > > +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> > > > +
> > > > +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> > > > +{
> > > > +     Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> > > > +}
> > > > +
> > > > +#define counts_values_member_def(member, ptype, help) \
> > > > +     { #member, ptype, \
> > > > +       offsetof(struct pyrf_counts_values, values.member), \
> > > > +       0, help }
> > > > +
> > > > +static PyMemberDef pyrf_counts_values_members[] = {
> > > > +     counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> > > > +     counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> > > > +     counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> > > > +     counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> > > > +     counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> > > > +     {NULL}
> > > > +};
> > >
> > > So the above is failing on a aarch64 debian (rpi5):
> > >
> > > acme@raspberrypi:~/git/perf-tools-next $ dpkg -S /usr/include/python3.11/structmember.h
> > > libpython3.11-dev:arm64: /usr/include/python3.11/structmember.h
> > > acme@raspberrypi:~/git/perf-tools-next $
> > >
> > > Where it only has:
> > >
> > > acme@raspberrypi:~/git/perf-tools-next $ grep -r Py_T_ULONG /usr/include/
> > > acme@raspberrypi:~/git/perf-tools-next $ grep -rw Py_T_ULONG /usr/include/
> > > acme@raspberrypi:~/git/perf-tools-next $ grep -rw T_ULONG /usr/include/
> > > /usr/include/python3.11/structmember.h:#define T_ULONG     12
> > > acme@raspberrypi:~/git/perf-tools-next $
> > >
> > > while on fedora 42 x86_64:
> > >
> > > ⬢ [acme@toolbx perf-tools-next]$ grep -rw Py_T_ULONG /usr/include/
> > > /usr/include/python3.13/descrobject.h:#define Py_T_ULONG     12
> > > /usr/include/python3.13/structmember.h:#define T_ULONG     Py_T_ULONG
> > > ⬢ [acme@toolbx perf-tools-next]$
> > >
> > > So I'm making it use the T_ULONG and others as all the other PyMemberDef
> > > arrays in tools/perf/util/python.c, ok?
> > 
> > The fix makes sense to me. Checking the documentation it seems
> > Py_T_ULONG is preferred:
> > https://docs.python.org/3/c-api/structures.html#member-types
> > perhaps we can add:
> > ```
> > #ifndef Py_T_ULONG
> > #define Py_T_ULONG T_ULONG
> > #endif
> > ```
> > so that we use the approach matching the documentation while fixing
> > the RaspberryPi issue.
> 
> That can be done as a followup, as there are lots of preexisting usage
> for struct method definitions.

And there is one other issue:

  LINK    /tmp/build/perf/perf
  GEN     /tmp/build/perf/python/perf.cpython-36m-x86_64-linux-gnu.so
/git/perf-6.15.0-rc7/tools/perf/util/python.c:653:7: error: missing field 'type' initializer [-Werror,-Wmissing-field-initializers]
  653 |         {NULL}
      |              ^
/git/perf-6.15.0-rc7/tools/perf/util/python.c:695:7: error: missing field 'get' initializer [-Werror,-Wmissing-field-initializers]
  695 |         {NULL}
      |              ^
2 errors generated.
error: command 'clang' failed with exit status 1
cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf*.so': No such file or directory


I'll look at this after dinner.

- Arnaldo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-22 22:37         ` Arnaldo Carvalho de Melo
@ 2025-05-23  1:28           ` Arnaldo Carvalho de Melo
  2025-05-26 14:49             ` Gautam Menghani
  0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-23  1:28 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Gautam Menghani, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

On Thu, May 22, 2025 at 07:37:46PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, May 22, 2025 at 07:36:31PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Thu, May 22, 2025 at 03:32:44PM -0700, Ian Rogers wrote:
> > > On Thu, May 22, 2025 at 3:20 PM Arnaldo Carvalho de Melo
> > > <acme@kernel.org> wrote:
> > > >
> > > > On Mon, May 19, 2025 at 12:51:41PM -0700, Ian Rogers wrote:
> > > > > From: Gautam Menghani <gautam@linux.ibm.com>
> > > > >
> > > > > Add support for perf_counts_values struct to enable the python
> > > > > bindings to read and return the counter data.
> > > > >
> > > > > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > > > > ---
> > > > >  tools/perf/util/python.c | 92 +++++++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 91 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > > > > index ead3afd2d996..1cbddfe77c7c 100644
> > > > > --- a/tools/perf/util/python.c
> > > > > +++ b/tools/perf/util/python.c
> > > > > @@ -626,6 +626,92 @@ static int pyrf_thread_map__setup_types(void)
> > > > >       return PyType_Ready(&pyrf_thread_map__type);
> > > > >  }
> > > > >
> > > > > +struct pyrf_counts_values {
> > > > > +     PyObject_HEAD
> > > > > +
> > > > > +     struct perf_counts_values values;
> > > > > +};
> > > > > +
> > > > > +static const char pyrf_counts_values__doc[] = PyDoc_STR("perf counts values object.");
> > > > > +
> > > > > +static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values)
> > > > > +{
> > > > > +     Py_TYPE(pcounts_values)->tp_free((PyObject *)pcounts_values);
> > > > > +}
> > > > > +
> > > > > +#define counts_values_member_def(member, ptype, help) \
> > > > > +     { #member, ptype, \
> > > > > +       offsetof(struct pyrf_counts_values, values.member), \
> > > > > +       0, help }
> > > > > +
> > > > > +static PyMemberDef pyrf_counts_values_members[] = {
> > > > > +     counts_values_member_def(val, Py_T_ULONG, "Value of event"),
> > > > > +     counts_values_member_def(ena, Py_T_ULONG, "Time for which enabled"),
> > > > > +     counts_values_member_def(run, Py_T_ULONG, "Time for which running"),
> > > > > +     counts_values_member_def(id, Py_T_ULONG, "Unique ID for an event"),
> > > > > +     counts_values_member_def(lost, Py_T_ULONG, "Num of lost samples"),
> > > > > +     {NULL}
> > > > > +};
> > > >
> > > > So the above is failing on a aarch64 debian (rpi5):
> > > >
> > > > acme@raspberrypi:~/git/perf-tools-next $ dpkg -S /usr/include/python3.11/structmember.h
> > > > libpython3.11-dev:arm64: /usr/include/python3.11/structmember.h
> > > > acme@raspberrypi:~/git/perf-tools-next $
> > > >
> > > > Where it only has:
> > > >
> > > > acme@raspberrypi:~/git/perf-tools-next $ grep -r Py_T_ULONG /usr/include/
> > > > acme@raspberrypi:~/git/perf-tools-next $ grep -rw Py_T_ULONG /usr/include/
> > > > acme@raspberrypi:~/git/perf-tools-next $ grep -rw T_ULONG /usr/include/
> > > > /usr/include/python3.11/structmember.h:#define T_ULONG     12
> > > > acme@raspberrypi:~/git/perf-tools-next $
> > > >
> > > > while on fedora 42 x86_64:
> > > >
> > > > ⬢ [acme@toolbx perf-tools-next]$ grep -rw Py_T_ULONG /usr/include/
> > > > /usr/include/python3.13/descrobject.h:#define Py_T_ULONG     12
> > > > /usr/include/python3.13/structmember.h:#define T_ULONG     Py_T_ULONG
> > > > ⬢ [acme@toolbx perf-tools-next]$
> > > >
> > > > So I'm making it use the T_ULONG and others as all the other PyMemberDef
> > > > arrays in tools/perf/util/python.c, ok?
> > > 
> > > The fix makes sense to me. Checking the documentation it seems
> > > Py_T_ULONG is preferred:
> > > https://docs.python.org/3/c-api/structures.html#member-types
> > > perhaps we can add:
> > > ```
> > > #ifndef Py_T_ULONG
> > > #define Py_T_ULONG T_ULONG
> > > #endif
> > > ```
> > > so that we use the approach matching the documentation while fixing
> > > the RaspberryPi issue.
> > 
> > That can be done as a followup, as there are lots of preexisting usage
> > for struct method definitions.
> 
> And there is one other issue:
> 
>   LINK    /tmp/build/perf/perf
>   GEN     /tmp/build/perf/python/perf.cpython-36m-x86_64-linux-gnu.so
> /git/perf-6.15.0-rc7/tools/perf/util/python.c:653:7: error: missing field 'type' initializer [-Werror,-Wmissing-field-initializers]
>   653 |         {NULL}
>       |              ^
> /git/perf-6.15.0-rc7/tools/perf/util/python.c:695:7: error: missing field 'get' initializer [-Werror,-Wmissing-field-initializers]
>   695 |         {NULL}
>       |              ^
> 2 errors generated.
> error: command 'clang' failed with exit status 1
> cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf*.so': No such file or directory
> 
> 
> I'll look at this after dinner.

Doing the same thing as for all the other PyMemberDef arrays makes it
pass that hurdle:

⬢ [acme@toolbx perf-tools-next]$ git diff
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index cb5a76674a5f1078..60ff12e90b91f97c 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -650,7 +650,7 @@ static PyMemberDef pyrf_counts_values_members[] = {
        counts_values_member_def(run, T_ULONG, "Time for which running"),
        counts_values_member_def(id, T_ULONG, "Unique ID for an event"),
        counts_values_member_def(lost, T_ULONG, "Num of lost samples"),
-       {NULL}
+       { .name = NULL, },
 };

 static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self, void *closure)

Then there is:

  GEN     /tmp/build/perf/python/perf.cpython-36m-x86_64-linux-gnu.so
/git/perf-6.15.0-rc7/tools/perf/util/python.c:695:8: error: missing field 'get' initializer [-Werror,-Wmissing-field-initializers]
  695 |         {NULL,}
      |               ^
1 error generated.
error: command 'clang' failed with exit status 1

I tried with the ending NULL, but it wasn't enough, so I'm going with:

⬢ [acme@toolbx perf-tools-next]$ git diff
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 5fa113daf4488120..21e2da1ec0c6342c 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -692,7 +692,7 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
 static PyGetSetDef pyrf_counts_values_getset[] = {
        {"values", (getter)pyrf_counts_values_get_values, (setter)pyrf_counts_values_set_values,
                "Name field", NULL},
-       {NULL,}
+       { .name = NULL, },
 };
 
 static PyTypeObject pyrf_counts_values__type = {
⬢ [acme@toolbx perf-tools-next]$ 

- Arnaldo

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data
  2025-05-23  1:28           ` Arnaldo Carvalho de Melo
@ 2025-05-26 14:49             ` Gautam Menghani
  0 siblings, 0 replies; 20+ messages in thread
From: Gautam Menghani @ 2025-05-26 14:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Howard Chu, linux-kernel, linux-perf-users, maddy

Hi Arnaldo,

Thanks for fixing this, I should have looked at the other
initializations in the file.

Thanks,
Gautam

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-05-26 14:50 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-19 19:51 [PATCH v3 0/7] perf python: Add missing infra pieces for counting Ian Rogers
2025-05-19 19:51 ` [PATCH v3 1/7] libperf threadmap: Don't segv for index 0 for the NULL perf_thread_map Ian Rogers
2025-05-19 19:51 ` [PATCH v3 2/7] libperf threadmap: Add perf_thread_map__idx Ian Rogers
2025-05-19 19:51 ` [PATCH v3 3/7] perf python: Add evsel cpus and threads functions Ian Rogers
2025-05-19 19:51 ` [PATCH v3 4/7] perf python: Add support for perf_counts_values to return counter data Ian Rogers
2025-05-21 13:20   ` Arnaldo Carvalho de Melo
2025-05-21 13:56     ` Ian Rogers
2025-05-21 17:26       ` Arnaldo Carvalho de Melo
2025-05-22 22:20   ` Arnaldo Carvalho de Melo
2025-05-22 22:32     ` Ian Rogers
2025-05-22 22:36       ` Arnaldo Carvalho de Melo
2025-05-22 22:37         ` Arnaldo Carvalho de Melo
2025-05-23  1:28           ` Arnaldo Carvalho de Melo
2025-05-26 14:49             ` Gautam Menghani
2025-05-19 19:51 ` [PATCH v3 5/7] perf python: Add evsel read method Ian Rogers
2025-05-21 13:18   ` Arnaldo Carvalho de Melo
2025-05-19 19:51 ` [PATCH v3 6/7] perf python: Add evlist close support Ian Rogers
2025-05-19 19:51 ` [PATCH v3 7/7] perf python: Add counting.py as example for counting perf events Ian Rogers
2025-05-21 14:46   ` Arnaldo Carvalho de Melo
2025-05-21 12:27 ` [PATCH v3 0/7] perf python: Add missing infra pieces for counting Gautam Menghani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).