[PATCH] tracing: Update entries counter when removing pages

[PATCH] tracing: Update entries counter when removing pages

[Vaibhav Nagarnaik]

When removing pages from the ring buffer, its state is not reset. This
means that the counters need to be correctly updated to account for the
pages removed.

Update the entries counter to reflect the removed events from the
pages just like entries_bytes counter.

Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
---
 kernel/trace/ring_buffer.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 96c2dd1..a2bec4c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1346,10 +1346,10 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages)
 			 * If something was added to this page, it was full
 			 * since it is not the tail page. So we deduct the
 			 * bytes consumed in ring buffer from here.
-			 * No need to update overruns, since this page is
-			 * deleted from ring buffer and its entries are
-			 * already accounted for.
+			 * No need to update overruns, since updating
+			 * 'entries' accounts for that.
 			 */
+			local_sub(page_entries, &cpu_buffer->entries);
 			local_sub(BUF_PAGE_SIZE, &cpu_buffer->entries_bytes);
 		}
 
-- 
1.7.7.3

Re: [PATCH] tracing: Update entries counter when removing pages

[Steven Rostedt]

On Mon, 2012-06-18 at 12:12 -0700, Vaibhav Nagarnaik wrote:
> When removing pages from the ring buffer, its state is not reset. This
> means that the counters need to be correctly updated to account for the
> pages removed.
> 
> Update the entries counter to reflect the removed events from the
> pages just like entries_bytes counter.
> 
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
> ---
>  kernel/trace/ring_buffer.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 96c2dd1..a2bec4c 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1346,10 +1346,10 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages)
>  			 * If something was added to this page, it was full
>  			 * since it is not the tail page. So we deduct the
>  			 * bytes consumed in ring buffer from here.
> -			 * No need to update overruns, since this page is
> -			 * deleted from ring buffer and its entries are
> -			 * already accounted for.
> +			 * No need to update overruns, since updating
> +			 * 'entries' accounts for that.
>  			 */
> +			local_sub(page_entries, &cpu_buffer->entries);

Actually, I think it would be better to increment overrun instead. As
these are lost events, and that is what overrun counts. I just tested it
with: local_add(page_entries, &cpu_buffer->overrun); and it works well.
Showing the lost events.

-- Steve

>  			local_sub(BUF_PAGE_SIZE, &cpu_buffer->entries_bytes);
>  		}
>  

Re: [PATCH] tracing: Update entries counter when removing pages

[Steven Rostedt]

On Fri, 2012-06-29 at 08:25 -0400, Steven Rostedt wrote:
> On Mon, 2012-06-18 at 12:12 -0700, Vaibhav Nagarnaik wrote:
> > When removing pages from the ring buffer, its state is not reset. This
> > means that the counters need to be correctly updated to account for the
> > pages removed.
> > 
> > Update the entries counter to reflect the removed events from the
> > pages just like entries_bytes counter.
> > 
> > Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
> > ---
> >  kernel/trace/ring_buffer.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 96c2dd1..a2bec4c 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -1346,10 +1346,10 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages)
> >  			 * If something was added to this page, it was full
> >  			 * since it is not the tail page. So we deduct the
> >  			 * bytes consumed in ring buffer from here.
> > -			 * No need to update overruns, since this page is
> > -			 * deleted from ring buffer and its entries are
> > -			 * already accounted for.
> > +			 * No need to update overruns, since updating
> > +			 * 'entries' accounts for that.
> >  			 */
> > +			local_sub(page_entries, &cpu_buffer->entries);
> 
> Actually, I think it would be better to increment overrun instead. As
> these are lost events, and that is what overrun counts. I just tested it
> with: local_add(page_entries, &cpu_buffer->overrun); and it works well.
> Showing the lost events.

Can you make this change and repost the patch. I'll start testing it if
you can ASAP. Otherwise it will have to wait till next week.

-- Steve

> >  			local_sub(BUF_PAGE_SIZE, &cpu_buffer->entries_bytes);
> >  		}
> >  
> 

Re: [PATCH] tracing: Update entries counter when removing pages

[Vaibhav Nagarnaik]

On Fri, Jun 29, 2012 at 12:17 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 2012-06-29 at 08:25 -0400, Steven Rostedt wrote:
>> Actually, I think it would be better to increment overrun instead. As
>> these are lost events, and that is what overrun counts. I just tested it
>> with: local_add(page_entries, &cpu_buffer->overrun); and it works well.
>> Showing the lost events.
>
> Can you make this change and repost the patch. I'll start testing it if
> you can ASAP. Otherwise it will have to wait till next week.
>

I was just testing the change. I am sending the new patch.


Vaibhav Nagarnaik