All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@kernel.org>
Cc: lkml <linux-kernel@vger.kernel.org>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Peter Zijlstra" <a.p.zijlstra@chello.nl>,
	"Ondřej Lysoněk" <olysonek@redhat.com>
Subject: Re: [PATCH] perf python: Do not force closing original perf descriptor in evlist.get_pollfd
Date: Wed, 26 Dec 2018 16:47:10 -0300	[thread overview]
Message-ID: <20181226194710.GB19471@kernel.org> (raw)
In-Reply-To: <20181226112121.5285-1-jolsa@kernel.org>

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

  reply	other threads:[~2018-12-26 19:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-26 11:21 [PATCH] perf python: Do not force closing original perf descriptor in evlist.get_pollfd Jiri Olsa
2018-12-26 19:47 ` Arnaldo Carvalho de Melo [this message]
2018-12-27  8:06 ` Jiri Olsa
2018-12-27 12:54   ` Arnaldo Carvalho de Melo
2019-01-03 13:13 ` [tip:perf/urgent] perf python: Do not force closing original perf descriptor in evlist.get_pollfd() tip-bot for Jiri Olsa

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=20181226194710.GB19471@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=olysonek@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.