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

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

[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()

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 <olysonek@redhat.com>
Link: http://lkml.kernel.org/n/tip-ru9hmsaliew8p01kr0050mvg@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 47628e85c5eb..dda0ac978b1e 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -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;
-- 
2.17.2

Re: [PATCH] perf python: Do not force closing original perf descriptor in evlist.get_pollfd

[Arnaldo Carvalho de Melo]

Em Wed, Dec 26, 2018 at 12:21:21PM +0100, Jiri Olsa escreveu:
> 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.

Applied.

- Arnaldo
 
> Reported-by: Ondřej Lysoněk <olysonek@redhat.com>
> Link: http://lkml.kernel.org/n/tip-ru9hmsaliew8p01kr0050mvg@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/python.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 47628e85c5eb..dda0ac978b1e 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -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;
> -- 
> 2.17.2

-- 

- Arnaldo

Re: [PATCH] perf python: Do not force closing original perf descriptor in evlist.get_pollfd

[Jiri Olsa]

On Wed, Dec 26, 2018 at 12:21:21PM +0100, Jiri Olsa wrote:
> 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 <olysonek@redhat.com>
> Link: http://lkml.kernel.org/n/tip-ru9hmsaliew8p01kr0050mvg@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

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

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

jirka

> ---
>  tools/perf/util/python.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 47628e85c5eb..dda0ac978b1e 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -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;
> -- 
> 2.17.2
> 

Re: [PATCH] perf python: Do not force closing original perf descriptor in evlist.get_pollfd

[Arnaldo Carvalho de Melo]

Em Thu, Dec 27, 2018 at 09:06:38AM +0100, Jiri Olsa escreveu:
> On Wed, Dec 26, 2018 at 12:21:21PM +0100, Jiri Olsa wrote:
> > 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 <olysonek@redhat.com>
> > Link: http://lkml.kernel.org/n/tip-ru9hmsaliew8p01kr0050mvg@git.kernel.org
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> 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:perf/urgent] perf python: Do not force closing original perf descriptor in evlist.get_pollfd()

[tip-bot for Jiri Olsa]

Commit-ID:  a389aece97938966616ce0336466b98b0351ef10
Gitweb:     https://git.kernel.org/tip/a389aece97938966616ce0336466b98b0351ef10
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 26 Dec 2018 12:21:21 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <olysonek@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jaroslav Škarvada <jskarvad@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 66dfdff03d19 ("perf tools: Add Python 3 support")
Link: http://lkml.kernel.org/r/20181226112121.5285-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 47628e85c5eb..dda0ac978b1e 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -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;