Linux Trace Kernel
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ian Rogers <irogers@google.com>
Subject: Re: [PATCH v21 8/9] ring-buffer: Show persistent buffer dropped events in trace file
Date: Fri, 29 May 2026 00:15:00 +0900	[thread overview]
Message-ID: <20260529001500.14178455a046a5cbc6180861@kernel.org> (raw)
In-Reply-To: <20260527093507.00ac35d8@gandalf.local.home>

On Wed, 27 May 2026 09:35:07 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> On Wed, 27 May 2026 12:47:21 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Yeah, for the persistent ring buffer, it does not happen.
> > But there seems RB_MISSED_EVENTS bit can be cleared in
> > "else" path (after applying 1-8 patches)?
> 
> Note, *only* the persistent ring buffer adds RB_MISSED_EVENTS to the pages
> in the write buffer. In the normal buffer, these bits are only set by this
> function. That is, they would not be set from the swap of pages.
> 

Ah, OK. For normal ring buffers, it is only set by reader.

> > 
> > ----------
> > 	if (read || (len < (commit - read)) ||
> > 	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
> > 	    force_memcpy) { // <-- persistent ring buffer sets force_memcpy = true.
> > [...]
> > 	} else {
> > 		/* update the entry counter */
> > [...]
> > 		if (!missed_events && rb_data_page_commit(dpage) & RB_MISSED_EVENTS)
> > 			missed_events = -1;	
> > 			//^-- we check RB_MISSED_EVENTS bit on @dpage->commit and set missed_events = -1.
> > 
> > 		/*
> > 		 * Use the real_end for the data size,
> > 		 * This gives us a chance to store the lost events
> > 		 * on the page.
> > 		 */
> > 		if (reader->real_end)
> > 			local_set(&dpage->commit, reader->real_end);
> >                         // ^- only if @reader->real_end, RB_MISSED_EVENTS bit is dropped.
> 
> Because this isn't a persistent ring buffer (if it was, as you noted,
> force_memcpy would be true and we wouldn't enter the else path), the
> RB_MISSED_EVENTS bit in the commit would never be set here. It is *only* set
> by the verifier of the persistent ring buffer logic.

OK, I got it.

Thanks for confirmation!

> 
> > 	}
> > 
> > 	cpu_buffer->lost_events = 0;
> > 
> > 	commit = rb_data_page_commit(dpage);
> > 	/*
> > 	 * Set a flag in the commit field if we lost events
> > 	 */
> > 	if (missed_events) {
> > 		/*
> > 		 * If there is room at the end of the page to save the
> > 		 * missed events, then record it there.
> > 		 */
> > 		if (missed_events > 0 &&
> > 		    buffer->subbuf_size - commit >= sizeof(missed_events)) {
> > 			memcpy(&dpage->data[commit], &missed_events,
> > 			       sizeof(missed_events));
> > 			local_add(RB_MISSED_STORED, &dpage->commit);
> > 			commit += sizeof(missed_events);
> > 		}
> > 		local_add(RB_MISSED_EVENTS, &dpage->commit); // <-- @dpage->commit is updated.
> > 	}
> 
> And this is the first place it would get set.
> 
> But yeah, it is very confusing and needs better comments.

Yeah, the relationship between the persistent ring buffer (which is mapped)
and the RB_MISSED_EVENTS is a bit unclear.

Thanks, 


> 
> Thanks,
> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2026-05-28 15:15 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22 17:08 [PATCH v21 0/9] ring-buffer: Making persistent ring buffers robust Steven Rostedt
2026-05-22 17:08 ` [PATCH v21 1/9] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer Steven Rostedt
2026-05-22 17:08 ` [PATCH v21 2/9] ring-buffer: Skip invalid sub-buffers when rewinding " Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 3/9] ring-buffer: Add persistent ring buffer invalid-page inject test Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 4/9] ring-buffer: Show commit numbers in buffer_meta file Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 5/9] ring-buffer: Cleanup persistent ring buffer validation Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 6/9] ring-buffer: Cleanup buffer_data_page related code Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 7/9] ring-buffer: Have dropped subbuffers be persistent across reboots Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 8/9] ring-buffer: Show persistent buffer dropped events in trace file Steven Rostedt
2026-05-26  5:06   ` Masami Hiramatsu
2026-05-26 17:41     ` Steven Rostedt
2026-05-27  3:47       ` Masami Hiramatsu
2026-05-27 13:35         ` Steven Rostedt
2026-05-28 15:15           ` Masami Hiramatsu [this message]
2026-05-28 15:26             ` Steven Rostedt
2026-05-22 17:09 ` [PATCH v21 9/9] ring-buffer: Show persistent buffer dropped events in trace_pipe file Steven Rostedt
2026-05-26  6:06   ` Masami Hiramatsu
2026-05-26  5:17 ` [PATCH v21 0/9] ring-buffer: Making persistent ring buffers robust Masami Hiramatsu
2026-05-26 17:42   ` Steven Rostedt
2026-05-27  3:57     ` Masami Hiramatsu

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=20260529001500.14178455a046a5cbc6180861@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=rostedt@kernel.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