From: Robert Richter <rric@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Jean Pihet <jean.pihet@linaro.org>,
Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@kernel.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>,
Jiri Olsa <jolsa@redhat.com>,
linux-kernel@vger.kernel.org, Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH 04/16] perf, mmap: Factor out perf_get_fd()
Date: Tue, 29 Apr 2014 17:34:01 +0200 [thread overview]
Message-ID: <20140429153401.GK32718@rric.localhost> (raw)
In-Reply-To: <20140425145205.GV13658@twins.programming.kicks-ass.net>
On 25.04.14 16:52:05, Peter Zijlstra wrote:
> But no, I don't think that helps, its still true that the moment you get
> a fd another thread can immediately close(). That would drop the last
> ref and free it, meanwhile perf_event_open() is happily poking at it.
>
> Now I think you could cure this by adding an extra ref before calling
> your perf_get_fd() and dropping that extra ref at the end, where we used
> to have fd_install().
Yes, right. I have a solution now which increments the event's ref
count before creating the file descriptor using try_get_event()/
put_event().
The patch also does not remove get_unused_fd_flags() and the err_fd
error handler.
Have an update already of a rebase version but still need to test it.
Would it be ok to split the patch set and send in a first step only
the first 4 patches that refactor the perf mmap code?
Thanks,
-Robert
next prev parent reply other threads:[~2014-04-29 15:34 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-07 15:04 [PATCH v4 00/16] perf, persistent: Add persistent events Jean Pihet
2014-04-07 15:04 ` [PATCH 01/16] perf, mmap: Factor out ring_buffer_detach_all() Jean Pihet
2014-04-07 15:04 ` [PATCH 02/16] perf, mmap: Factor out try_get_event()/put_event() Jean Pihet
2014-04-07 15:04 ` [PATCH 03/16] perf, mmap: Factor out perf_alloc/free_rb() Jean Pihet
2014-04-22 14:25 ` Peter Zijlstra
2014-04-07 15:04 ` [PATCH 04/16] perf, mmap: Factor out perf_get_fd() Jean Pihet
2014-04-22 14:27 ` Peter Zijlstra
2014-04-25 13:54 ` Robert Richter
2014-04-25 14:43 ` Peter Zijlstra
2014-04-25 14:52 ` Peter Zijlstra
2014-04-29 15:34 ` Robert Richter [this message]
2014-04-07 15:04 ` [PATCH 05/16] perf: Add persistent events Jean Pihet
2014-04-07 15:04 ` [PATCH 06/16] mce, x86: Enable " Jean Pihet
2014-04-07 15:04 ` [PATCH 07/16] perf, persistent: Implementing a persistent pmu Jean Pihet
2014-04-07 15:04 ` [PATCH 08/16] perf, persistent: Exposing persistent events using sysfs Jean Pihet
2014-04-07 15:04 ` [PATCH 09/16] perf, persistent: Use unique event ids Jean Pihet
2014-04-07 15:04 ` [PATCH 10/16] perf, persistent: Implement reference counter for events Jean Pihet
2014-04-07 15:04 ` [PATCH 11/16] perf, persistent: Dynamically resize list of sysfs entries Jean Pihet
2014-04-07 15:04 ` [PATCH 12/16] perf, persistent: ioctl functions to control persistency Jean Pihet
2014-04-07 15:04 ` [PATCH 13/16] perf tools: Rename flex conditions to avoid name conflicts Jean Pihet
2014-04-07 15:04 ` [PATCH 14/16] perf tools: Modify event parser to update event attribute by index Jean Pihet
2014-04-07 15:04 ` [PATCH 15/16] perf tools: Add attr<num> syntax to event parser Jean Pihet
2014-04-07 15:04 ` [PATCH 16/16] perf tools: Retry mapping buffers readonly on EACCES Jean Pihet
2014-04-17 12:44 ` [PATCH v4 00/16] perf, persistent: Add persistent events Jean Pihet
2014-04-17 12:50 ` Borislav Petkov
2014-04-17 13:17 ` Jean Pihet
2014-04-17 13:21 ` Borislav Petkov
2014-04-22 8:20 ` Jean Pihet
2014-04-22 10:07 ` Jean Pihet
2014-04-22 10:20 ` Borislav Petkov
2014-05-06 12:39 ` Robert Richter
2014-05-06 18:50 ` Borislav Petkov
2014-05-06 18:53 ` Borislav Petkov
2014-05-07 16:44 ` Robert Richter
2014-05-08 18:23 ` Borislav Petkov
2014-05-09 9:17 ` Robert Richter
2014-05-06 18:58 ` Borislav Petkov
2014-05-07 17:01 ` Robert Richter
2014-05-08 18:36 ` Borislav Petkov
2014-05-09 8:52 ` Robert Richter
2014-05-09 10:17 ` Borislav Petkov
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=20140429153401.GK32718@rric.localhost \
--to=rric@kernel.org \
--cc=acme@infradead.org \
--cc=bp@alien8.de \
--cc=jean.pihet@linaro.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=viro@ZenIV.linux.org.uk \
/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