linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Cc: Vincent Donnefort <vdonnefort@google.com>
Subject: [PATCH v2] kbuffer: Always walk the events to calculate timestamp in kbuffer_read_buffer()
Date: Mon, 8 Jan 2024 12:34:20 -0500	[thread overview]
Message-ID: <20240108123420.5e227075@gandalf.local.home> (raw)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If the buffer to read the kbuffer subbuffer is big enough to hold the
entire buffer, it was simply copied. But this is not good enough, as the
next read should include the events after what was copied. That means the
timestamps need to be calculated.

Just copying the data is not enough as the timestamp of any event is an
offset of the previous event. To know the event timestamp after what was
copied, the timestamps of the events before it need to be read, to know
what the offset is against of the following event.

Link: https://lore.kernel.org/linux-trace-devel/ZZwFvyGvm0C38eBh@google.com/

Fixes: 05821189 ("kbuffer: Add kbuffer_read_buffer()")
Reported-by: Vincent Donnefort <vdonnefort@google.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes from v1: https://lore.kernel.org/linux-trace-devel/20240105194015.253165-3-rostedt@goodmis.org
 (which was originally: kbuffer: Add event if the buffer just fits in kbuffer_read_buffer()

 - Do not just fix the copying by the off-by-one error, but instead always
   read all events to copy to calculate the timestamp (Vincent Donnefort)

 src/kbuffer-parse.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/src/kbuffer-parse.c b/src/kbuffer-parse.c
index d43fe5d972fd..691d53678f5f 100644
--- a/src/kbuffer-parse.c
+++ b/src/kbuffer-parse.c
@@ -947,19 +947,12 @@ kbuffer_raw_get(struct kbuffer *kbuf, void *subbuf, struct kbuffer_raw_info *inf
  */
 int kbuffer_read_buffer(struct kbuffer *kbuf, void *buffer, int len)
 {
-	int subbuf_size = kbuf->start + kbuf->size;
 	unsigned long long ts;
 	unsigned int type_len_ts;
 	bool do_swap = false;
 	int last_next;
 	int save_curr;
 
-	if (!kbuf->curr && len >= subbuf_size) {
-		memcpy(buffer, kbuf->subbuffer, subbuf_size);
-		set_curr_to_end(kbuf);
-		return kbuf->size;
-	}
-
 	/* Are we at the end of the buffer */
 	if (kbuf->curr >= kbuf->size)
 		return 0;
@@ -982,24 +975,13 @@ int kbuffer_read_buffer(struct kbuffer *kbuf, void *buffer, int len)
 
 	save_curr = kbuf->curr;
 
-	/* Copy the rest of the buffer if it fits */
-	if (len >= kbuf->size - kbuf->curr) {
-		set_curr_to_end(kbuf);
-		last_next = kbuf->size;
-	} else {
-		/*
-		 * The length doesn't hold the rest,
-		 * need to find the last that fits
-		 */
+	/* Due to timestamps, we must save the current next to use */
+	last_next = kbuf->next;
 
-		/* Due to timestamps, we must save the current next to use */
+	while (len >= kbuf->next - save_curr) {
 		last_next = kbuf->next;
-
-		while (len > kbuf->next - save_curr) {
-			last_next = kbuf->next;
-			if (!kbuffer_next_event(kbuf, &ts))
-				break;
-		}
+		if (!kbuffer_next_event(kbuf, &ts))
+			break;
 	}
 
 	len = last_next - save_curr;
-- 
2.43.0


                 reply	other threads:[~2024-01-08 17:33 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20240108123420.5e227075@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=vdonnefort@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;
as well as URLs for NNTP newsgroup(s).