linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"David Ahern" <dsahern@gmail.com>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Peter Zijlstra" <a.p.zijlstra@chello.nl>,
	"Jaroslav Škarvada" <jskarvad@redhat.com>,
	"Joe Mario" <jmario@redhat.com>
Subject: [PATCH 2/2] perf python: Fix pyrf_evlist__read_on_cpu interface
Date: Fri, 17 Aug 2018 13:45:56 +0200	[thread overview]
Message-ID: <20180817114556.28000-3-jolsa@kernel.org> (raw)
In-Reply-To: <20180817114556.28000-1-jolsa@kernel.org>

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


  parent reply	other threads:[~2018-08-17 11:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Jiri Olsa [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180817114556.28000-3-jolsa@kernel.org \
    --to=jolsa@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dsahern@gmail.com \
    --cc=jmario@redhat.com \
    --cc=jskarvad@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).