This is a note to let you know that I've just added the patch titled
ring-buffer: Do not try to put back write_stamp
to the 6.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ring-buffer-do-not-try-to-put-back-write_stamp.patch
and it can be found in the queue-6.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From dd939425707898da992e59ab0fcfae4652546910 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
Date: Thu, 14 Dec 2023 22:29:21 -0500
Subject: ring-buffer: Do not try to put back write_stamp
From: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
commit dd939425707898da992e59ab0fcfae4652546910 upstream.
If an update to an event is interrupted by another event between the time
the initial event allocated its buffer and where it wrote to the
write_stamp, the code try to reset the write stamp back to the what it had
just overwritten. It knows that it was overwritten via checking the
before_stamp, and if it didn't match what it wrote to the before_stamp
before it allocated its space, it knows it was overwritten.
To put back the write_stamp, it uses the before_stamp it read. The problem
here is that by writing the before_stamp to the write_stamp it makes the
two equal again, which means that the write_stamp can be considered valid
as the last timestamp written to the ring buffer. But this is not
necessarily true. The event that interrupted the event could have been
interrupted in a way that it was interrupted as well, and can end up
leaving with an invalid write_stamp. But if this happens and returns to
this context that uses the before_stamp to update the write_stamp again,
it can possibly incorrectly make it valid, causing later events to have in
correct time stamps.
As it is OK to leave this function with an invalid write_stamp (one that
doesn't match the before_stamp), there's no reason to try to make it valid
again in this case. If this race happens, then just leave with the invalid
write_stamp and the next event to come along will just add a absolute
timestamp and validate everything again.
Bonus points: This gets rid of another cmpxchg64!
Link: https://lore.kernel.org/linux-trace-kernel/20231214222921.193037a7@xxxxxxxxxxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx
Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
Cc: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx>
Cc: Vincent Donnefort <vdonnefort@xxxxxxxxxx>
Fixes: a389d86f7fd09 ("ring-buffer: Have nested events still record running time stamp")
Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
kernel/trace/ring_buffer.c | 29 ++++++-----------------------
1 file changed, 6 insertions(+), 23 deletions(-)
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3611,14 +3611,14 @@ __rb_reserve_next(struct ring_buffer_per
}
if (likely(tail == w)) {
- u64 save_before;
- bool s_ok;
-
/* Nothing interrupted us between A and C */
/*D*/ rb_time_set(&cpu_buffer->write_stamp, info->ts);
- barrier();
- /*E*/ s_ok = rb_time_read(&cpu_buffer->before_stamp, &save_before);
- RB_WARN_ON(cpu_buffer, !s_ok);
+ /*
+ * If something came in between C and D, the write stamp
+ * may now not be in sync. But that's fine as the before_stamp
+ * will be different and then next event will just be forced
+ * to use an absolute timestamp.
+ */
if (likely(!(info->add_timestamp &
(RB_ADD_STAMP_FORCE | RB_ADD_STAMP_ABSOLUTE))))
/* This did not interrupt any time update */
@@ -3626,24 +3626,7 @@ __rb_reserve_next(struct ring_buffer_per
else
/* Just use full timestamp for interrupting event */
info->delta = info->ts;
- barrier();
check_buffer(cpu_buffer, info, tail);
- if (unlikely(info->ts != save_before)) {
- /* SLOW PATH - Interrupted between C and E */
-
- a_ok = rb_time_read(&cpu_buffer->write_stamp, &info->after);
- RB_WARN_ON(cpu_buffer, !a_ok);
-
- /* Write stamp must only go forward */
- if (save_before > info->after) {
- /*
- * We do not care about the result, only that
- * it gets updated atomically.
- */
- (void)rb_time_cmpxchg(&cpu_buffer->write_stamp,
- info->after, save_before);
- }
- }
} else {
u64 ts;
/* SLOW PATH - Interrupted between A and C */
Patches currently in stable-queue which might be from rostedt@xxxxxxxxxxx are
queue-6.1/ring-buffer-do-not-update-before-stamp-when-switching-sub-buffers.patch
queue-6.1/ring-buffer-have-saved-event-hold-the-entire-event.patch
queue-6.1/ring-buffer-have-rb_time_cmpxchg-set-the-msb-counter-too.patch
queue-6.1/ring-buffer-do-not-try-to-put-back-write_stamp.patch
queue-6.1/ring-buffer-fix-memory-leak-of-free-page.patch
queue-6.1/ring-buffer-fix-a-race-in-rb_time_cmpxchg-for-32-bit-archs.patch
queue-6.1/tracing-update-snapshot-buffer-on-resize-if-it-is-allocated.patch
queue-6.1/ring-buffer-fix-writing-to-the-buffer-with-max_data_size.patch
