Messages in this thread | | | Date | Thu, 9 Jun 2011 18:40:25 +0200 | From | Frederic Weisbecker <> | Subject | Re: [GIT PULL] perf update |
| |
On Thu, Jun 09, 2011 at 03:17:32PM +0200, Ingo Molnar wrote: > > > > > > - 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.
For the same reasons I don't like the rename you did of the perf buffer into struct ring_buffer, I think we shouldn't do this.
This is a generic naming that belongs to some very universal datatype. There is no good reason to find that generic concept naming exported from a subsystem that is not a generic implementation of that datatype.
And it's not because it doesn't yet clash with existing names that it's a good choice.
Why not "struct perf_ring_buffer" ? And then perf_ring_buffer_begin()? or perf_rb_begin() if it's too long.
> > Thanks, > > Ingo
| |