From: Ingo Molnar <mingo@elte.hu>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
LKML <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@alien8.de>,
Stephane Eranian <eranian@google.com>
Subject: Re: [GIT PULL] perf update
Date: Thu, 9 Jun 2011 15:17:32 +0200 [thread overview]
Message-ID: <20110609131732.GA21100@elte.hu> (raw)
In-Reply-To: <20110609123519.GF25771@somewhere.redhat.com>
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Thu, Jun 09, 2011 at 01:43:13PM +0200, Ingo Molnar wrote:
> >
> > * Ingo Molnar <mingo@elte.hu> wrote:
> >
> > >
> > > * Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > >
> > > > On Tue, 2011-06-07 at 16:56 +0200, Frederic Weisbecker wrote:
> > > >
> > > > > Frederic Weisbecker (1):
> > > > > perf: Split up buffer handling from core code
> > > > >
> > > > >
> > > > > kernel/events/Makefile | 2 +-
> > > > > kernel/events/buffer.c | 400 ++++++++++++++++++++++++++++++++++++++++
> > > > > kernel/events/core.c | 458 ++--------------------------------------------
> > > > > kernel/events/internal.h | 70 +++++++
> > > > > 4 files changed, 487 insertions(+), 443 deletions(-)
> > > >
> > > > Looks about right.
> > > >
> > > > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > >
> > > thanks, i'll pull it it and test it.
> >
> > Note, i ended up applying it by hand:
> >
> > - fixed a build error,
> >
> > - streamlined the renaming: we really want this to be ring_buffer.c
> > (most of the complexity comes from this not being a simple buffer
> > but a ring-buffer)
> >
> > - i streamlined the naming around it: struct ring_buffer
> > internalized via internal.h (it does not clash with ftrace's
> > ring-buffer)
> >
> > It all looks and reads much nicer now, but please double check the
> > commit as well :-)
> >
> > One other rename i'd like to do is:
> >
> > struct perf_output_handle => struct rb_handle
> >
> > perf_output_begin() => rb_open()
> > perf_output_copy() => rb_write()
> > perf_output_sample() => rb_write_sample()
> > perf_output_end() => rb_close()
> >
> > Which really makes it a lot more apparent that it's a regular
> > input/output flow defined over the ring-buffer!
> >
> > I can do this if this is fine with everyone. There will be no change
> > in functionality.
>
> I feel more comfortable if we keep the perf_outpout_*() naming, having some
> global rb_* would pollute the global namespace.
Hm, using the rb_ prefix is not good due to the (conceptual) clash
with rbtree.h primitives.
> perf_rb_* namespace would be fine as well.
How about:
struct perf_output_handle => struct ring_buffer_handle
perf_output_begin() => ring_buffer_open()
perf_output_copy() => ring_buffer_write()
perf_output_sample() => ring_buffer_write_sample()
perf_output_end() => ring_buffer_close()
?
It doesn't clash with existing names.
Thanks,
Ingo
next prev parent reply other threads:[~2011-06-09 13:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-07 14:56 [GIT PULL] perf update Frederic Weisbecker
2011-06-09 9:13 ` Peter Zijlstra
2011-06-09 9:19 ` Ingo Molnar
2011-06-09 11:43 ` Ingo Molnar
2011-06-09 12:35 ` Frederic Weisbecker
2011-06-09 13:17 ` Ingo Molnar [this message]
2011-06-09 16:40 ` Frederic Weisbecker
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=20110609131732.GA21100@elte.hu \
--to=mingo@elte.hu \
--cc=a.p.zijlstra@chello.nl \
--cc=bp@alien8.de \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.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