oprofile: remove double ring buffering
PATCH 1/1 oprofile: remove double ring buffering
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
-Andi
---
oprofile: remove double ring buffering
oprofile used a double buffer scheme for its cpu event buffer to avoid races on reading with the old locked ring buffer.
But that is obsolete now with the new ring buffer, so simply use a single buffer. This greatly simplifies the code and avoids a lot of sample drops on large runs, especially with call graph.
Based on suggestions from Steven Rostedt
drivers/oprofile/cpu_buffer.c +13 -50
@@ -30,23 +30,7 @@#define OP_BUFFER_FLAGS 0 -/* -* Read and write access is using spin locking. Thus, writing to the -* buffer by NMI handler (x86) could occur also during critical -* sections when reading the buffer. To avoid this, there are 2 -* buffers for independent read and write access. Read access is in -* process context only, write access only in the NMI handler. If the -* read buffer runs empty, both buffers are swapped atomically. There -* is potentially a small window during swapping where the buffers are -* disabled and samples could be lost. -* -* Using 2 buffers is a little bit overhead, but the solution is clear -* and does not require changes in the ring buffer implementation. It -* can be changed to a single buffer solution when the ring buffer -* access is implemented as non-locking atomic code. -*/ -static struct ring_buffer *op_ring_buffer_read; -static struct ring_buffer *op_ring_buffer_write; +static struct ring_buffer *op_ring_buffer; DEFINE_PER_CPU(struct oprofile_cpu_buffer, op_cpu_buffer);static void wq_sync_buffer (struct work_struct *work);
@@ -68,12 +52,9 @@ void oprofile_cpu_buffer_inc_smpl_lost(void)void free_cpu_buffers (void ) { -if (op_ring_buffer_read) -ring_buffer_free (op_ring_buffer_read); - op_ring_buffer_read =NULL ; -if (op_ring_buffer_write) -ring_buffer_free (op_ring_buffer_write); - op_ring_buffer_write =NULL ; +if (op_ring_buffer) +ring_buffer_free (op_ring_buffer); + op_ring_buffer =NULL ; }#define RB_EVENT_HDR_SIZE 4
@@ -86,11 +67,8 @@ int alloc_cpu_buffers(void)unsigned long byte_size = buffer_size * (sizeof (struct op_sample ) +RB_EVENT_HDR_SIZE ); - op_ring_buffer_read =ring_buffer_alloc (byte_size,OP_BUFFER_FLAGS ); -if (!op_ring_buffer_read) - goto fail; - op_ring_buffer_write =ring_buffer_alloc (byte_size,OP_BUFFER_FLAGS ); -if (!op_ring_buffer_write) + op_ring_buffer =ring_buffer_alloc (byte_size,OP_BUFFER_FLAGS ); +if (!op_ring_buffer) goto fail;for_each_possible_cpu (i) {
@@ -162,16 +140,11 @@ struct op_sample *op_cpu_buffer_write_reserve(struct op_entry *entry, unsigned long size) { entry->event =ring_buffer_lock_reserve - (op_ring_buffer_write,sizeof (struct op_sample ) + + (op_ring_buffer,sizeof (struct op_sample ) + size *sizeof (entry->sample->data[0 ])); -if (entry->event) - entry->sample =ring_buffer_event_data (entry->event); -else - entry->sample =NULL ; - -if (!entry->sample) +if (!entry->event)return NULL ; - + entry->sample =ring_buffer_event_data (entry->event); entry->size = size; entry->data = entry->sample->data;
@@ -180,25 +153,16 @@ struct op_sampleint op_cpu_buffer_write_commit (struct op_entry *entry) { -return ring_buffer_unlock_commit (op_ring_buffer_write, entry->event); +return ring_buffer_unlock_commit (op_ring_buffer, entry->event); }struct op_sample *op_cpu_buffer_read_entry (struct op_entry *entry,int cpu) {struct ring_buffer_event *e; - e =ring_buffer_consume (op_ring_buffer_read, cpu,NULL ); -if (e) - goto event; -if (ring_buffer_swap_cpu (op_ring_buffer_read, - op_ring_buffer_write, - cpu)) + e =ring_buffer_consume (op_ring_buffer, cpu,NULL ); +if (!e)return NULL ; - e =ring_buffer_consume (op_ring_buffer_read, cpu,NULL ); -if (e) - goto event; -return NULL ; -event: entry->event = e; entry->sample =ring_buffer_event_data (e); entry->size = (ring_buffer_event_length (e) -sizeof (struct op_sample ))
@@ -209,8 +173,7 @@ event:unsigned long op_cpu_buffer_entries (int cpu) { -return ring_buffer_entries_cpu (op_ring_buffer_read, cpu) - +ring_buffer_entries_cpu (op_ring_buffer_write, cpu); +return ring_buffer_entries_cpu (op_ring_buffer, cpu); } static int
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
For automatic inclusion in stable trees when the patch goes into Linus's tree, just add:
Cc: stable <[email protected]>
to the Signed-off-by: area of the patch and we will be automatically notified of it and you will not have to do anything else (and I will not have to dig through the git trees to see if it has been commited or not, and if so, what the id is...)
thanks,
greg k-h
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
Thanks Andi, will take a look at this next week.
-Robert
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Quoted 7 lines · Robert Richter
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.Thanks Andi, will take a look at this next week.
Note, I just sent out a patch set yesterday which will conflict with this change.
I Cc'd both you and Andi on that patch.
-- Steve
Quoted 7 lines · Steven Rostedt
Quoted 6 lines · Robert Richter
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
Quoted 6 lines
Quoted 1 line
Thanks Andi, will take a look at this next week.
Note, I just sent out a patch set yesterday which will conflict with this change.
I Cc'd both you and Andi on that patch.
Steve, is your ring buffer patch intended for stable?
I plan to submit the oprofile patch for stable and if your is not it would be more convenient if your patch was based on mine instead of the other way round. If yours is for stable I can rebase on top of yours.
Thanks,
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only.
Quoted 11 lines · Andi Kleen
Quoted 4 lines
Note, I just sent out a patch set yesterday which will conflict with this change.
I Cc'd both you and Andi on that patch.
Steve, is your ring buffer patch intended for stable?
I plan to submit the oprofile patch for stable and if your is not it would be more convenient if your patch was based on mine instead of the other way round. If yours is for stable I can rebase on top of yours.
No, my patch is not intended for stable, and in fact, is intended for 35.
When yours gets into Linus's tree, we can fix the conflict. It should be trivial to fix, since the conflict is just me adding another parameter (NULL) to a function you changed.
I just wanted you to be aware of the change, you don't need to do anything about it.
-- Steve
Quoted 7 lines · Robert Richter
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.Thanks Andi, will take a look at this next week.
I was wondering what the state of this patch is. I don't see it in linux-2.6 or linux-next. or ingit://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git
Thanks,
John
Quoted 7 lines · On Thu, 22 Apr 2010 10:33:56 -0400 John Villalovos <[email protected]> wrote:
Quoted 6 lines · Robert Richter
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
Quoted 6 lines
Quoted 1 line
Thanks Andi, will take a look at this next week.
I was wondering what the state of this patch is. I don't see it in linux-2.6 or linux-next. or in
git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git
I queued it in my for-2.6.34-via-subsystem-maintainer pile. I'll start sending those things out for everyone to ignore early next week I guess.
Quoted 7 lines · Andrew Morton
Quoted 4 lines
I was wondering what the state of this patch is. I don't see it in linux-2.6 or linux-next. or in
git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.gitI queued it in my for-2.6.34-via-subsystem-maintainer pile. I'll start sending those things out for everyone to ignore early next week I guess.
I just make my tree ready for v2.6.35 and will put it in there.
The patch looks good, will run some tests.
-Robert
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Quoted 3 lines · Robert Richter
I just make my tree ready for v2.6.35 and will put it in there.
The patch looks good, will run some tests.
The patch is now in the oprofile tree. Sorry for this late response.
-Robert
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Quoted 4 lines · Andi Kleen
Got confirmation now this patch fixes most of the dropped oprofile samples problem on a particular intensive test on a large system. So now submitting the previous RFC formally.
.34 / Stable candidate.
Patch applied to
git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
Thanks Andi and sorry for this late response.
-Robert
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]