* [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface
@ 2018-08-17 11:45 Jiri Olsa
2018-08-17 11:45 ` [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap Jiri Olsa
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jiri Olsa @ 2018-08-17 11:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Alexander Shishkin,
Peter Zijlstra, Jaroslav Škarvada, Joe Mario
hi,
Jaroslav reported errors from valgrind over perf python script,
which led us to discover serious issue with python interface.
It affects tuned daemon, which depends on this interface,
and makes it hang.
thanks,
jirka
---
Jiri Olsa (2):
perf tools: Store real cpu number in the struct perf_mmap
perf python: Fix pyrf_evlist__read_on_cpu interface
tools/perf/util/evlist.c | 2 +-
tools/perf/util/mmap.c | 4 +++-
tools/perf/util/mmap.h | 4 +++-
tools/perf/util/python.c | 20 +++++++++++++++++++-
4 files changed, 26 insertions(+), 4 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap 2018-08-17 11:45 [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa @ 2018-08-17 11:45 ` Jiri Olsa 2018-08-23 8:45 ` [tip:perf/urgent] perf mmap: Store real cpu number in 'struct perf_mmap' tip-bot for Jiri Olsa 2018-08-17 11:45 ` [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa 2018-08-17 14:59 ` [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Arnaldo Carvalho de Melo 2 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2018-08-17 11:45 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Alexander Shishkin, Peter Zijlstra, Jaroslav Škarvada, Joe Mario Storing the real cpu number in the struct perf_mmap, which be used by python interface that allows user to read particular memory map for given cpu. Link: http://lkml.kernel.org/n/tip-a2h9hjs7abv42yiagxiz2sp8@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/evlist.c | 2 +- tools/perf/util/mmap.c | 4 +++- tools/perf/util/mmap.h | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index e7a4b31a84fb..26e1c02440f9 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -803,7 +803,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, if (*output == -1) { *output = fd; - if (perf_mmap__mmap(&maps[idx], mp, *output) < 0) + if (perf_mmap__mmap(&maps[idx], mp, *output, evlist_cpu) < 0) return -1; } else { if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0) diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c index fc832676a798..2eac6dee0b9c 100644 --- a/tools/perf/util/mmap.c +++ b/tools/perf/util/mmap.c @@ -164,7 +164,8 @@ void perf_mmap__munmap(struct perf_mmap *map) auxtrace_mmap__munmap(&map->auxtrace_mmap); } -int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd) +int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, + int fd, int cpu) { /* * The last one will be done at perf_mmap__consume(), so that we @@ -191,6 +192,7 @@ int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd) return -1; } map->fd = fd; + map->cpu = cpu; if (auxtrace_mmap__mmap(&map->auxtrace_mmap, &mp->auxtrace_mp, map->base, fd)) diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h index d82294db1295..cc4fc8cc33ca 100644 --- a/tools/perf/util/mmap.h +++ b/tools/perf/util/mmap.h @@ -18,6 +18,7 @@ struct perf_mmap { void *base; int mask; int fd; + int cpu; refcount_t refcnt; u64 prev; u64 start; @@ -60,7 +61,8 @@ struct mmap_params { struct auxtrace_mmap_params auxtrace_mp; }; -int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd); +int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, + int fd, int cpu); void perf_mmap__munmap(struct perf_mmap *map); void perf_mmap__get(struct perf_mmap *map); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:perf/urgent] perf mmap: Store real cpu number in 'struct perf_mmap' 2018-08-17 11:45 ` [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap Jiri Olsa @ 2018-08-23 8:45 ` tip-bot for Jiri Olsa 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Jiri Olsa @ 2018-08-23 8:45 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, linux-kernel, jolsa, acme, namhyung, dsahern, alexander.shishkin, mingo, hpa, jskarvad, peterz, jmario Commit-ID: 31fb4c0d7b88f036edb96a6a3bd791289ea2f931 Gitweb: https://git.kernel.org/tip/31fb4c0d7b88f036edb96a6a3bd791289ea2f931 Author: Jiri Olsa <jolsa@kernel.org> AuthorDate: Fri, 17 Aug 2018 13:45:55 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Mon, 20 Aug 2018 08:54:59 -0300 perf mmap: Store real cpu number in 'struct perf_mmap' Store the real cpu number in 'struct perf_mmap', which will be used by python interface that allows user to read a particular memory map for given cpu. Signed-off-by: Jiri Olsa <jolsa@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Jaroslav Škarvada <jskarvad@redhat.com> Cc: Joe Mario <jmario@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180817114556.28000-2-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/evlist.c | 2 +- tools/perf/util/mmap.c | 3 ++- tools/perf/util/mmap.h | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index e7a4b31a84fb..be440df29615 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -803,7 +803,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx, if (*output == -1) { *output = fd; - if (perf_mmap__mmap(&maps[idx], mp, *output) < 0) + if (perf_mmap__mmap(&maps[idx], mp, *output, evlist_cpu) < 0) return -1; } else { if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0) diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c index fc832676a798..215f69f41672 100644 --- a/tools/perf/util/mmap.c +++ b/tools/perf/util/mmap.c @@ -164,7 +164,7 @@ void perf_mmap__munmap(struct perf_mmap *map) auxtrace_mmap__munmap(&map->auxtrace_mmap); } -int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd) +int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd, int cpu) { /* * The last one will be done at perf_mmap__consume(), so that we @@ -191,6 +191,7 @@ int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd) return -1; } map->fd = fd; + map->cpu = cpu; if (auxtrace_mmap__mmap(&map->auxtrace_mmap, &mp->auxtrace_mp, map->base, fd)) diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h index d82294db1295..05a6d47c7956 100644 --- a/tools/perf/util/mmap.h +++ b/tools/perf/util/mmap.h @@ -18,6 +18,7 @@ struct perf_mmap { void *base; int mask; int fd; + int cpu; refcount_t refcnt; u64 prev; u64 start; @@ -60,7 +61,7 @@ struct mmap_params { struct auxtrace_mmap_params auxtrace_mp; }; -int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd); +int perf_mmap__mmap(struct perf_mmap *map, struct mmap_params *mp, int fd, int cpu); void perf_mmap__munmap(struct perf_mmap *map); void perf_mmap__get(struct perf_mmap *map); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface 2018-08-17 11:45 [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa 2018-08-17 11:45 ` [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap Jiri Olsa @ 2018-08-17 11:45 ` Jiri Olsa 2018-08-23 8:46 ` [tip:perf/urgent] perf python: Fix pyrf_evlist__read_on_cpu() interface tip-bot for Jiri Olsa 2018-08-17 14:59 ` [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Arnaldo Carvalho de Melo 2 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2018-08-17 11:45 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Alexander Shishkin, Peter Zijlstra, Jaroslav Škarvada, Joe Mario Jaroslav reported errors from valgrind over perf python script: # echo 0 > /sys/devices/system/cpu/cpu4/online # valgrind ./test.py ==7524== Memcheck, a memory error detector ... ==7524== Command: ./test.py ==7524== pid 7526 exited ==7524== Invalid read of size 8 ==7524== at 0xCC2C2B3: perf_mmap__read_forward (evlist.c:780) ==7524== by 0xCC2A681: pyrf_evlist__read_on_cpu (python.c:959) ... ==7524== Address 0x65c4868 is 16 bytes after a block of size 459,36.. ==7524== at 0x4C2B955: calloc (vg_replace_malloc.c:711) ==7524== by 0xCC2F484: zalloc (util.h:35) ==7524== by 0xCC2F484: perf_evlist__alloc_mmap (evlist.c:978) ... The reason for this is in the python interface, that allows script to pass arbitrary cpu number, which is then used to access struct perf_evlist::mmap array. That's obviously wrong and works only when if all cpus are available and fails if some cpu is missing, like in the example above. This patch makes the pyrf_evlist__read_on_cpu search the evlist's maps array for the proper map to access. It's linear search at the moment. Based on the way how is the read_on_cpu used, I don't think we need to be fast in here. But we could add some hash in the middle to make it fast/er. We don't allow python interface to set write_backward event attribute, so it's safe to check only evlist's mmaps. Reported-by: Jaroslav Škarvada <jskarvad@redhat.com> Link: http://lkml.kernel.org/n/tip-nje64txu8bcop0ogjvbt8i54@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/python.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c index f74fbb652a4f..ce501ba14b08 100644 --- a/tools/perf/util/python.c +++ b/tools/perf/util/python.c @@ -11,6 +11,7 @@ #include "cpumap.h" #include "print_binary.h" #include "thread_map.h" +#include "mmap.h" #if PY_MAJOR_VERSION < 3 #define _PyUnicode_FromString(arg) \ @@ -976,6 +977,20 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist, return Py_BuildValue("i", evlist->nr_entries); } +static struct perf_mmap *get_md(struct perf_evlist *evlist, int cpu) +{ + int i; + + for (i = 0; i < evlist->nr_mmaps; i++) { + struct perf_mmap *md = &evlist->mmap[i]; + + if (md->cpu == cpu) + return md; + } + + return NULL; +} + static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist, PyObject *args, PyObject *kwargs) { @@ -990,7 +1005,10 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist, &cpu, &sample_id_all)) return NULL; - md = &evlist->mmap[cpu]; + md = get_md(evlist, cpu); + if (!md) + return NULL; + if (perf_mmap__read_init(md) < 0) goto end; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:perf/urgent] perf python: Fix pyrf_evlist__read_on_cpu() interface 2018-08-17 11:45 ` [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa @ 2018-08-23 8:46 ` tip-bot for Jiri Olsa 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Jiri Olsa @ 2018-08-23 8:46 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, namhyung, jskarvad, jmario, jolsa, hpa, linux-kernel, alexander.shishkin, acme, dsahern, mingo, peterz Commit-ID: 721f0dfc3ce821c6a32820ab63edfb48ed4af075 Gitweb: https://git.kernel.org/tip/721f0dfc3ce821c6a32820ab63edfb48ed4af075 Author: Jiri Olsa <jolsa@kernel.org> AuthorDate: Fri, 17 Aug 2018 13:45:56 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Mon, 20 Aug 2018 08:54:59 -0300 perf python: Fix pyrf_evlist__read_on_cpu() interface Jaroslav reported errors from valgrind over perf python script: # echo 0 > /sys/devices/system/cpu/cpu4/online # valgrind ./test.py ==7524== Memcheck, a memory error detector ... ==7524== Command: ./test.py ==7524== pid 7526 exited ==7524== Invalid read of size 8 ==7524== at 0xCC2C2B3: perf_mmap__read_forward (evlist.c:780) ==7524== by 0xCC2A681: pyrf_evlist__read_on_cpu (python.c:959) ... ==7524== Address 0x65c4868 is 16 bytes after a block of size 459,36.. ==7524== at 0x4C2B955: calloc (vg_replace_malloc.c:711) ==7524== by 0xCC2F484: zalloc (util.h:35) ==7524== by 0xCC2F484: perf_evlist__alloc_mmap (evlist.c:978) ... The reason for this is in the python interface, that allows a script to pass arbitrary cpu number, which is then used to access struct perf_evlist::mmap array. That's obviously wrong and works only when if all cpus are available and fails if some cpu is missing, like in the example above. This patch makes pyrf_evlist__read_on_cpu() search the evlist's maps array for the proper map to access. It's linear search at the moment. Based on the way how is the read_on_cpu used, I don't think we need to be fast in here. But we could add some hash in the middle to make it fast/er. We don't allow python interface to set write_backward event attribute, so it's safe to check only evlist's mmaps. Reported-by: Jaroslav Škarvada <jskarvad@redhat.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Joe Mario <jmario@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180817114556.28000-3-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/python.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c index f74fbb652a4f..ce501ba14b08 100644 --- a/tools/perf/util/python.c +++ b/tools/perf/util/python.c @@ -11,6 +11,7 @@ #include "cpumap.h" #include "print_binary.h" #include "thread_map.h" +#include "mmap.h" #if PY_MAJOR_VERSION < 3 #define _PyUnicode_FromString(arg) \ @@ -976,6 +977,20 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist, return Py_BuildValue("i", evlist->nr_entries); } +static struct perf_mmap *get_md(struct perf_evlist *evlist, int cpu) +{ + int i; + + for (i = 0; i < evlist->nr_mmaps; i++) { + struct perf_mmap *md = &evlist->mmap[i]; + + if (md->cpu == cpu) + return md; + } + + return NULL; +} + static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist, PyObject *args, PyObject *kwargs) { @@ -990,7 +1005,10 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist, &cpu, &sample_id_all)) return NULL; - md = &evlist->mmap[cpu]; + md = get_md(evlist, cpu); + if (!md) + return NULL; + if (perf_mmap__read_init(md) < 0) goto end; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface 2018-08-17 11:45 [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa 2018-08-17 11:45 ` [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap Jiri Olsa 2018-08-17 11:45 ` [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa @ 2018-08-17 14:59 ` Arnaldo Carvalho de Melo 2 siblings, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-08-17 14:59 UTC (permalink / raw) To: Jiri Olsa Cc: lkml, Ingo Molnar, Namhyung Kim, David Ahern, Alexander Shishkin, Peter Zijlstra, Jaroslav Škarvada, Joe Mario Em Fri, Aug 17, 2018 at 01:45:54PM +0200, Jiri Olsa escreveu: > hi, > Jaroslav reported errors from valgrind over perf python script, > which led us to discover serious issue with python interface. > > It affects tuned daemon, which depends on this interface, > and makes it hang. Thanks, applied. - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-23 8:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-17 11:45 [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa 2018-08-17 11:45 ` [PATCH 1/2] perf tools: Store real cpu number in the struct perf_mmap Jiri Olsa 2018-08-23 8:45 ` [tip:perf/urgent] perf mmap: Store real cpu number in 'struct perf_mmap' tip-bot for Jiri Olsa 2018-08-17 11:45 ` [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface Jiri Olsa 2018-08-23 8:46 ` [tip:perf/urgent] perf python: Fix pyrf_evlist__read_on_cpu() interface tip-bot for Jiri Olsa 2018-08-17 14:59 ` [PATCH 0/2] perf tools: Fix pyrf_evlist__read_on_cpu interface Arnaldo Carvalho de Melo
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).