perf python: Do not force closing original perf descriptor in evlist.get_pollfd

PATCH Applied Jiri Olsa 4 participants 5 messages Dec 26, 2018 – Jan 3, 2019 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2018-12-26 11:21 UTC #0 lore raw

PATCH 1/1 perf python: Do not force closing original perf descriptor in evlist.get_pollfd

Ondřej reported that when compiled with python3, the python extension regress in evlist.get_pollfd function behaviour.

The evlist.get_pollfd creates file objects from evlist's fds and returns them in the list. The python3 version also sets them to 'close the original descriptor' when the object die (is closed), by passing True via 'closefd' arg in PyFile_FromFd call.

The python's closefd doc says:
If closefd is False, the underlying file descriptor will be kept open when the file is closed.

That's why following line in python3 closes all evlist fds:
evlist.get_pollfd()

the returned list is immediately destroyed and that takes down the original events fds.

Passing closefd as False to PyFile_FromFd to fix this.

Reported-by Ondřej Lysoněk <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tools/perf/util/python.c +2 -1

index 47628e85c5eb..dda0ac978b1e 100644
@@ -939,7 +939,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 		file = PyFile_FromFile(fp, "perf", "r", NULL);
 #else
-		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, NULL, NULL, NULL, 1);
+		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1,
+				     NULL, NULL, NULL, 0);
 #endif
 		if (file == NULL)
 			goto free_list;

Signature · 1 line

2.17.2

Arnaldo Carvalho de Melo

maintainer Applied

2018-12-26 19:47 UTC #1 lore raw

Em Wed, Dec 26, 2018 at 12:21:21PM +0100, Jiri Olsa escreveu:

Quoted 13 lines

Ondřej reported that when compiled with python3, the python extension regress in evlist.get_pollfd function behaviour.
The evlist.get_pollfd creates file objects from evlist's fds and returns them in the list. The python3 version also sets them to 'close the original descriptor' when the object die (is closed), by passing True via 'closefd' arg in PyFile_FromFd call.
The python's closefd doc says:
If closefd is False, the underlying file descriptor will be kept open when the file is closed.

Quoted 7 lines

That's why following line in python3 closes all evlist fds:
evlist.get_pollfd()
the returned list is immediately destroyed and that takes down the original events fds.
Passing closefd as False to PyFile_FromFd to fix this.

Applied.

- Arnaldo

Quoted 23 lines

Reported-by Ondřej Lysoněk <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tools/perf/util/python.c +2 -1

index 47628e85c5eb..dda0ac978b1e 100644
@@ -939,7 +939,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 		file = PyFile_FromFile(fp, "perf", "r", NULL);
 #else
-		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, NULL, NULL, NULL, 1);
+		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1,
+				     NULL, NULL, NULL, 0);
 #endif
 		if (file == NULL)
 			goto free_list;

Signature · 1 line

2.17.2

Signature · 1 line

- Arnaldo

Jiri Olsa

2018-12-27 08:06 UTC #2 lore raw

In reply to Jiri Olsa

Quoted 16 lines · Jiri Olsa

Ondřej reported that when compiled with python3, the python extension regress in evlist.get_pollfd function behaviour.
The evlist.get_pollfd creates file objects from evlist's fds and returns them in the list. The python3 version also sets them to 'close the original descriptor' when the object die (is closed), by passing True via 'closefd' arg in PyFile_FromFd call.
The python's closefd doc says:
If closefd is False, the underlying file descriptor will be kept open when the file is closed.
That's why following line in python3 closes all evlist fds:
evlist.get_pollfd()

Quoted 8 lines

the returned list is immediately destroyed and that takes down the original events fds.
Passing closefd as False to PyFile_FromFd to fix this.
Reported-by Ondřej Lysoněk <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>

oops, forgot to add.. and cc-ing Jaroslav Škarvada

Fixes 66dfdff03d19 ("perf tools: Add Python 3 support")

jirka

Quoted 21 lines

---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tools/perf/util/python.c +2 -1

index 47628e85c5eb..dda0ac978b1e 100644
@@ -939,7 +939,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 		file = PyFile_FromFile(fp, "perf", "r", NULL);
 #else
-		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, NULL, NULL, NULL, 1);
+		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1,
+				     NULL, NULL, NULL, 0);
 #endif
 		if (file == NULL)
 			goto free_list;

Signature · 1 line

2.17.2

Arnaldo Carvalho de Melo

maintainer

2018-12-27 12:54 UTC #3 lore raw

Em Thu, Dec 27, 2018 at 09:06:38AM +0100, Jiri Olsa escreveu:

Quoted 22 lines · Jiri Olsa

Quoted 21 lines
Ondřej reported that when compiled with python3, the python extension regress in evlist.get_pollfd function behaviour.
The evlist.get_pollfd creates file objects from evlist's fds and returns them in the list. The python3 version also sets them to 'close the original descriptor' when the object die (is closed), by passing True via 'closefd' arg in PyFile_FromFd call.
The python's closefd doc says:
If closefd is False, the underlying file descriptor will be kept open when the file is closed.
That's why following line in python3 closes all evlist fds:
evlist.get_pollfd()
the returned list is immediately destroyed and that takes down the original events fds.
Passing closefd as False to PyFile_FromFd to fix this.

Quoted 7 lines

Quoted 3 lines
Reported-by Ondřej Lysoněk <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
oops, forgot to add.. and cc-ing Jaroslav Škarvada
Fixes 66dfdff03d19 ("perf tools: Add Python 3 support")

Thanks, added the Fixes: and Cc: Jaroslav,

- Arnaldo

tip-bot for Jiri Olsa

merge bot

2019-01-03 13:13 UTC #4 lore raw

[tip:perf/urgent] perf python: Do not force closing original perf descriptor in evlist.get_pollfd()

In reply to Jiri Olsa

perf python: Do not force closing original perf descriptor in evlist.get_pollfd()

Commit-ID:  a389aece97938966616ce0336466b98b0351ef10
Gitweb:     https://git.kernel.org/tip/a389aece97938966616ce0336466b98b0351ef10
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 26 Dec 2018 12:21:21 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 28 Dec 2018 16:33:02 -0300

perf python: Do not force closing original perf descriptor in evlist.get_pollfd()

Ondřej reported that when compiled with python3, the python extension regresses in evlist.get_pollfd function behaviour.

The evlist.get_pollfd function creates file objects from evlist's fds and returns them in a list. The python3 version also sets them to 'close the original descriptor' when the object dies (is closed), by passing True via the 'closefd' arg in the PyFile_FromFd call.

The python's closefd doc says:

If closefd is False, the underlying file descriptor will be kept open when the file is closed.

That's why the following line in python3 closes all evlist fds:

evlist.get_pollfd()

the returned list is immediately destroyed and that takes down the original events fds.

Passing closefd as False to PyFile_FromFd to fix this.

Reported-by Ondřej Lysoněk <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Jaroslav Škarvada <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes 66dfdff03d19 ("perf tools: Add Python 3 support")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

tools/perf/util/python.c +2 -1

index 47628e85c5eb..dda0ac978b1e 100644
@@ -939,7 +939,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 		file = PyFile_FromFile(fp, "perf", "r", NULL);
 #else
-		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, NULL, NULL, NULL, 1);
+		file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1,
+				     NULL, NULL, NULL, 0);
 #endif
 		if (file == NULL)
 			goto free_list;