public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Sharp <dhsharp@google.com>
To: rostedt@goodmis.org, linux-kernel@vger.kernel.org
Cc: vnagarnaik@google.com, David Sharp <dhsharp@google.com>
Subject: [PATCH] ring-buffer: fix uninitialized read_stamp
Date: Thu,  7 Jun 2012 16:27:22 -0700	[thread overview]
Message-ID: <1339111642-869-1-git-send-email-dhsharp@google.com> (raw)

This fixes a scenario in which trace_pipe_raw will return events with invalid
timestamps when events are copied out one at a time (instead of swapping out
the reader page with a spare page). In this scenario, ring_buffer_read_page()
uses cpu_buffer->read_stamp to keep track of the time of the earliest event.
However, cpu_buffer->read_stamp was not always updated. The only function that
sets read_stamp to a new valid value is rb_reset_reader_page(), which is called
only by rb_get_reader_page(). rb_reset_reader_page() was not called when there
is data immediately available on the page to read (read < rb_page_size()). This
is the bug.

Setting the read_stamp on the first read from a page fixes the bug.

Tested: On certain lightly-loaded machines, repetitive reads from
trace_pipe_raw without using splice() would sometimes result in invalid
timestamps. Poisoning read_stamp in rb_init_page() with a negative value makes
the problem more visible. After this fix, the invalid timstamps disappear.

Google-Bug-Id: 6410455
Signed-off-by: David Sharp <dhsharp@google.com>
---
 kernel/trace/ring_buffer.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1d0f6a8..ad0239b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3226,8 +3226,15 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 	reader = cpu_buffer->reader_page;
 
 	/* If there's more to read, return this page */
-	if (cpu_buffer->reader_page->read < rb_page_size(reader))
+	if (cpu_buffer->reader_page->read < rb_page_size(reader)) {
+		/*
+		 * If this is the first read from this page,
+		 * initialize the read timestamp.
+		 */
+		if (cpu_buffer->reader_page->read == 0)
+			cpu_buffer->read_stamp = reader->page->time_stamp;
 		goto out;
+	}
 
 	/* Never should we have an index greater than the size */
 	if (RB_WARN_ON(cpu_buffer,
-- 
1.7.7.3


             reply	other threads:[~2012-06-07 23:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-07 23:27 David Sharp [this message]
2012-06-07 23:34 ` [PATCH] ring-buffer: fix uninitialized read_stamp David Sharp
2012-06-18 23:02   ` David Sharp
2012-06-21 14:52     ` Steven Rostedt
2012-06-21 15:46     ` Steven Rostedt
2012-06-21 15:56       ` Steven Rostedt
2012-06-22 20:50       ` David Sharp
2012-06-22 23:31         ` Steven Rostedt
2012-06-23  1:27           ` David Sharp
2012-06-27  0:35             ` David Sharp
2012-06-27 12:46               ` Steven Rostedt
2012-06-27 18:00                 ` David Sharp
2012-06-27 18:05                   ` Steven Rostedt
2012-07-06 10:49     ` [tip:perf/core] ring-buffer: Fix " tip-bot for Steven Rostedt
2012-07-06 11:30     ` [tip:perf/urgent] " tip-bot for Steven Rostedt

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=1339111642-869-1-git-send-email-dhsharp@google.com \
    --to=dhsharp@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=vnagarnaik@google.com \
    /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