From: Ingo Molnar <mingo@kernel.org>
To: Robert Richter <rric@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>,
Jiri Olsa <jolsa@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 00/14] perf, persistent: Kernel updates for perf tool integration
Date: Wed, 26 Jun 2013 13:45:38 +0200 [thread overview]
Message-ID: <20130626114538.GA4117@gmail.com> (raw)
In-Reply-To: <20130626101132.GC21788@rric.localhost>
* Robert Richter <rric@kernel.org> wrote:
> On 26.06.13 10:24:08, Borislav Petkov wrote:
> > On Wed, Jun 26, 2013 at 10:12:23AM +0200, Robert Richter wrote:
> > > We get a new fd by opening the persistent event with the syscall.
> > > There would be 2 new ioctls:
> > >
> > > ioctl(fd, PERF_EVENT_IOC_DETACH, 0);
> > > ioctl(fd, PERF_EVENT_IOC_ATTACH, 0);
> > >
> > > This would be fine and reuses existing infrastructure.
> >
> > Well, how are you going to say that you want to open an already existing
> > persistent event or your want to create exactly the same persistent
> > event? Are we even going to allow identical persistent events to
> > coexist?
>
> Here is the scenario:
Looks mostly good - with a few suggestions:
>
> Creating a persistent event from userspace:
>
> * A process opens a system-wide event with the syscall and gets a fd.
Should this really be limited to system-wide events?
> * The process mmaps the buffer.
> * The process does an ioctl to detach the process which increases the
> events and buffers refcount. The event is listed as 'persistent' in
> sysfs with a unique id.
> * The process closes the fd. Event and buffer remain in the system
> since the refcounts are not zero.
>
> Opening a persistent event:
>
> * A process scans sysfs for persistent events.
> * To open the event it sets up the event attr according to sysfs.
Basically it would just put some ID (found in sysfs) into the attr and set
attr.persistent=1 - not any other information, right?
If it knows the ID straight away (the user told it, or it remembers it
from some other file such as a temporary file, etc.) then it does not even
have to scan sysfs.
[ How about to additional logic: attr.persistent=1 && attr.config==0 means
a new persistent event is created straight away - no ioctl is needed to
detach it explicitly. ]
> * The persistent event is opened with the syscall, the process gets a
> new fd of the event.
> * The process attaches to the event buffer with mmap.
Yes. And gets the pre-existing event and mmap buffer.
> Releasing a persistent event:
>
> * A process opens a persistent event and gets a fd.
> * The process does an ioctl to attach the process which decreases the
> refcounts. The sysfs entry is removed.
> * The process closes the fd.
> * After all processes that are tied to the event closed their event's
> fds, the persistent event and its buffer is released.
>
> Sounds like a plan?
It does :-)
I'm sure there will be some details going down that path, but it looks
workable at first glance.
Note, for tracing the PERF_FLAG_FD_OUTPUT method of multiplexing multiple
events onto a single mmap buffers is probably useful (also usable via the
PERF_EVENT_IOC_SET_OUTPUT ioctl()), so please make sure the scheme works
naturally with that model as well, not just with 1:1 event+buffer
mappings.
See the uses of PERF_EVENT_IOC_SET_OUTPUT in tools/perf/.
Thanks,
Ingo
next prev parent reply other threads:[~2013-06-26 11:45 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-11 16:42 [PATCH v2 00/14] perf, persistent: Kernel updates for perf tool integration Robert Richter
2013-06-11 16:42 ` [PATCH v2 01/14] perf, ring_buffer: Use same prefix Robert Richter
2013-06-11 16:42 ` [PATCH v2 02/14] perf: Add persistent events Robert Richter
2013-06-24 9:28 ` Peter Zijlstra
2013-06-24 19:24 ` Borislav Petkov
2013-06-25 8:46 ` Robert Richter
2013-06-11 16:42 ` [PATCH v2 03/14] perf: Add persistent event facilities Robert Richter
2013-06-14 2:15 ` Namhyung Kim
2013-06-14 7:20 ` Robert Richter
2013-06-24 9:32 ` Peter Zijlstra
2013-06-25 8:47 ` Robert Richter
2013-06-24 9:44 ` Peter Zijlstra
2013-06-25 8:41 ` Robert Richter
2013-06-24 9:48 ` Peter Zijlstra
2013-06-24 19:26 ` Borislav Petkov
2013-06-25 7:44 ` Peter Zijlstra
2013-06-25 9:24 ` Robert Richter
2013-06-25 9:37 ` Borislav Petkov
2013-06-25 10:51 ` Robert Richter
2013-06-25 15:29 ` Borislav Petkov
2013-06-25 16:14 ` Robert Richter
2013-06-11 16:42 ` [PATCH v2 04/14] MCE: Enable persistent event Robert Richter
2013-06-11 16:42 ` [PATCH v2 05/14] perf, persistent: Rework struct pers_event_desc Robert Richter
2013-06-11 16:42 ` [PATCH v2 06/14] perf, persistent: Remove rb_put() Robert Richter
2013-06-11 16:42 ` [PATCH v2 07/14] perf, persistent: Introduce get_persistent_event() Robert Richter
2013-06-11 16:42 ` [PATCH v2 08/14] perf, persistent: Reworking perf_get_persistent_event_fd() Robert Richter
2013-06-11 16:42 ` [PATCH v2 09/14] perf, persistent: Protect event lists with mutex Robert Richter
2013-06-11 16:42 ` [PATCH v2 10/14] perf, persistent: Avoid adding identical events Robert Richter
2013-06-11 16:42 ` [PATCH v2 11/14] perf, persistent: Implementing a persistent pmu Robert Richter
2013-06-11 16:42 ` [PATCH v2 12/14] perf, persistent: Name each persistent event Robert Richter
2013-06-11 16:42 ` [PATCH v2 13/14] perf, persistent: Exposing persistent events using sysfs Robert Richter
2013-06-14 2:36 ` Namhyung Kim
2013-06-14 8:57 ` Robert Richter
2013-06-11 16:42 ` [PATCH v2 14/14] perf, persistent: Allow multiple users for an event Robert Richter
2013-06-24 10:08 ` [PATCH v2 00/14] perf, persistent: Kernel updates for perf tool integration Peter Zijlstra
2013-06-25 10:46 ` Robert Richter
2013-06-24 10:22 ` Peter Zijlstra
2013-06-25 16:56 ` Robert Richter
2013-06-24 10:24 ` Peter Zijlstra
2013-06-24 15:25 ` Peter Zijlstra
2013-06-24 19:45 ` Ingo Molnar
2013-06-25 17:57 ` Robert Richter
2013-06-25 19:16 ` Borislav Petkov
2013-06-26 8:12 ` Robert Richter
2013-06-26 8:24 ` Borislav Petkov
2013-06-26 9:46 ` Ingo Molnar
2013-06-26 9:56 ` Borislav Petkov
2013-06-26 10:11 ` Robert Richter
2013-06-26 11:45 ` Ingo Molnar [this message]
2013-06-26 12:25 ` Ingo Molnar
2013-06-26 12:44 ` Robert Richter
2013-06-27 5:46 ` Namhyung Kim
2013-06-27 8:35 ` Borislav Petkov
2013-06-27 8:50 ` Ingo Molnar
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=20130626114538.GA4117@gmail.com \
--to=mingo@kernel.org \
--cc=acme@infradead.org \
--cc=bp@alien8.de \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rric@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).