All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 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.