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
next 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