Linux kernel -stable discussions
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: <gregkh@linuxfoundation.org>
Cc: mark.rutland@arm.com, mathieu.desnoyers@efficios.com,
	mhiramat@kernel.org, torvalds@linux-foundation.org,
	<stable@vger.kernel.org>
Subject: Re: FAILED: patch "[PATCH] ring-buffer: Fix slowpath of interrupted event" failed to apply to 6.1-stable tree
Date: Sat, 30 Dec 2023 17:20:19 -0500	[thread overview]
Message-ID: <20231230172019.261b6059@gandalf.local.home> (raw)
In-Reply-To: <20231230165156.4fd2eef6@gandalf.local.home>

On Sat, 30 Dec 2023 16:51:56 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Below is the patch for 6.1 of this change, and it depends on:
> 
> 20231230164736.3b8c86c4@gandalf.local.home

And this is for 5.15.

-- Steve


---
 kernel/trace/ring_buffer.c |   77 ++++++++++++++-------------------------------
 1 file changed, 24 insertions(+), 53 deletions(-)

Index: test-linux.git/kernel/trace/ring_buffer.c
===================================================================
--- test-linux.git.orig/kernel/trace/ring_buffer.c	2023-12-30 16:51:29.022248800 -0500
+++ test-linux.git/kernel/trace/ring_buffer.c	2023-12-30 16:51:47.278447539 -0500
@@ -691,44 +691,6 @@ rb_time_read_cmpxchg(local_t *l, unsigne
 	return ret == expect;
 }
 
-static int rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set)
-{
-	unsigned long cnt, top, bottom;
-	unsigned long cnt2, top2, bottom2;
-	u64 val;
-
-	/* Any interruptions in this function should cause a failure */
-	cnt = local_read(&t->cnt);
-
-	/* The cmpxchg always fails if it interrupted an update */
-	 if (!__rb_time_read(t, &val, &cnt2))
-		 return false;
-
-	 if (val != expect)
-		 return false;
-
-	 if ((cnt & 3) != cnt2)
-		 return false;
-
-	 cnt2 = cnt + 1;
-
-	 rb_time_split(val, &top, &bottom);
-	 top = rb_time_val_cnt(top, cnt);
-	 bottom = rb_time_val_cnt(bottom, cnt);
-
-	 rb_time_split(set, &top2, &bottom2);
-	 top2 = rb_time_val_cnt(top2, cnt2);
-	 bottom2 = rb_time_val_cnt(bottom2, cnt2);
-
-	if (!rb_time_read_cmpxchg(&t->cnt, cnt, cnt2))
-		return false;
-	if (!rb_time_read_cmpxchg(&t->top, top, top2))
-		return false;
-	if (!rb_time_read_cmpxchg(&t->bottom, bottom, bottom2))
-		return false;
-	return true;
-}
-
 #else /* 64 bits */
 
 /* local64_t always succeeds */
@@ -742,13 +704,6 @@ static void rb_time_set(rb_time_t *t, u6
 {
 	local64_set(&t->time, val);
 }
-
-static bool rb_time_cmpxchg(rb_time_t *t, u64 expect, u64 set)
-{
-	u64 val;
-	val = local64_cmpxchg(&t->time, expect, set);
-	return val == expect;
-}
 #endif
 
 /*
@@ -3562,20 +3517,36 @@ __rb_reserve_next(struct ring_buffer_per
 	} else {
 		u64 ts;
 		/* SLOW PATH - Interrupted between A and C */
-		a_ok = rb_time_read(&cpu_buffer->write_stamp, &info->after);
-		/* Was interrupted before here, write_stamp must be valid */
+
+		/* Save the old before_stamp */
+		a_ok = rb_time_read(&cpu_buffer->before_stamp, &info->before);
 		RB_WARN_ON(cpu_buffer, !a_ok);
+
+		/*
+		 * Read a new timestamp and update the before_stamp to make
+		 * the next event after this one force using an absolute
+		 * timestamp. This is in case an interrupt were to come in
+		 * between E and F.
+		 */
 		ts = rb_time_stamp(cpu_buffer->buffer);
+		rb_time_set(&cpu_buffer->before_stamp, ts);
+
+		barrier();
+ /*E*/		a_ok = rb_time_read(&cpu_buffer->write_stamp, &info->after);
+		/* Was interrupted before here, write_stamp must be valid */
+		RB_WARN_ON(cpu_buffer, !a_ok);
 		barrier();
- /*E*/		if (write == (local_read(&tail_page->write) & RB_WRITE_MASK) &&
-		    info->after < ts &&
-		    rb_time_cmpxchg(&cpu_buffer->write_stamp,
-				    info->after, ts)) {
-			/* Nothing came after this event between C and E */
+ /*F*/		if (write == (local_read(&tail_page->write) & RB_WRITE_MASK) &&
+		    info->after == info->before && info->after < ts) {
+			/*
+			 * Nothing came after this event between C and F, it is
+			 * safe to use info->after for the delta as it
+			 * matched info->before and is still valid.
+			 */
 			info->delta = ts - info->after;
 		} else {
 			/*
-			 * Interrupted between C and E:
+			 * Interrupted between C and F:
 			 * Lost the previous events time stamp. Just set the
 			 * delta to zero, and this will be the same time as
 			 * the event this event interrupted. And the events that

  reply	other threads:[~2023-12-30 22:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-30 11:00 FAILED: patch "[PATCH] ring-buffer: Fix slowpath of interrupted event" failed to apply to 6.1-stable tree gregkh
2023-12-30 21:51 ` Steven Rostedt
2023-12-30 22:20   ` Steven Rostedt [this message]
2024-01-03 10:25     ` Greg KH

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=20231230172019.261b6059@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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