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,
Masami Hiramatsu <mhiramat@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 9/9] ring-buffer: Show persistent buffer dropped events in trace_pipe file
Date: Tue, 26 May 2026 15:06:40 +0900 [thread overview]
Message-ID: <20260526150640.69d957c8339fd15774b79e71@kernel.org> (raw)
In-Reply-To: <20260522171052.156419479@kernel.org>
On Fri, 22 May 2026 13:09:06 -0400
Steven Rostedt <rostedt@kernel.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When the persistent ring buffer is validated on boot up, if a subbuffer is
> deemed invalid, it resets the buffer and continues. Have the code preserve
> the RB_MISSED_EVENTS flag in the commit portion of the subbuffer header
> and pass that back so that the trace_pipe file can show the missed events
> like the trace file does.
>
> For example:
>
> <...>-1242 [005] d.... 4429.120116: page_fault_user: address=0x7ffaebb6e728 ip=0x7ffaeb9d4960 error_code=0x7
> <...>-1242 [005] ..... 4429.120124: mm_page_alloc: page=00000000055254f3 pfn=0x1373bd order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE|__GFP_COMP
> <...>-1242 [005] d..2. 4429.120132: tlb_flush: pages:1 reason:local MM shootdown (3)
> CPU:5 [LOST EVENTS]
> <...>-1242 [005] d.... 4429.120661: page_fault_user: address=0x55ba7c2d0944 ip=0x55ba7c20cd02 error_code=0x7
> <...>-1242 [005] ..... 4429.120669: mm_page_alloc: page=0000000005a02500 pfn=0x12b6e4 order=0 migratetype=1 gfp_flags=GFP_HIGHUSER_MOVABLE|__GFP_COMP
> <...>-1242 [005] d..2. 4429.120680: tlb_flush: pages:1 reason:local MM shootdown (3)
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> Changes since v20: https://patch.msgid.link/20260520185018.470465795@kernel.org
>
Looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> - Removed left over printk() (Masami Hiramatsu)
>
> kernel/trace/ring_buffer.c | 56 +++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 988915f035c7..910f6b3adf74 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -5801,6 +5801,7 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
> struct buffer_page *reader = NULL;
> unsigned long overwrite;
> unsigned long flags;
> + int missed_events = 0;
> int nr_loops = 0;
> bool ret;
>
> @@ -5901,6 +5902,9 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
> if (!ret)
> goto spin;
>
> + if (rb_page_commit(reader) & RB_MISSED_EVENTS)
> + missed_events = -1;
> +
> if (cpu_buffer->ring_meta)
> rb_update_meta_reader(cpu_buffer, reader);
>
> @@ -5965,6 +5969,8 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
> */
> smp_rmb();
>
> + if (!cpu_buffer->lost_events)
> + cpu_buffer->lost_events = missed_events;
>
> return reader;
> }
> @@ -7066,6 +7072,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> struct buffer_page *reader;
> long missed_events;
> unsigned int commit;
> + unsigned int size;
> unsigned int read;
> u64 save_timestamp;
> bool force_memcpy;
> @@ -7101,7 +7108,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> event = rb_reader_event(cpu_buffer);
>
> read = reader->read;
> - commit = rb_page_size(reader);
> + commit = rb_page_commit(reader);
> + size = rb_page_size(reader);
>
> /* Check if any events were dropped */
> missed_events = cpu_buffer->lost_events;
> @@ -7115,13 +7123,14 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> * we must copy the data from the page to the buffer.
> * Otherwise, we can simply swap the page with the one passed in.
> */
> - if (read || (len < (commit - read)) ||
> + if (read || (len < (size - read)) ||
> cpu_buffer->reader_page == cpu_buffer->commit_page ||
> force_memcpy) {
> struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
> unsigned int rpos = read;
> unsigned int pos = 0;
> - unsigned int size;
> + unsigned int event_size;
> + unsigned int flags = 0;
>
> /*
> * If a full page is expected, this can still be returned
> @@ -7130,19 +7139,22 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> * the reader page.
> */
> if (full &&
> - (!read || (len < (commit - read)) ||
> + (!read || (len < (size - read)) ||
> cpu_buffer->reader_page == cpu_buffer->commit_page))
> return -1;
>
> - if (len > (commit - read))
> - len = (commit - read);
> + if (len > (size - read))
> + len = (size - read);
>
> /* Always keep the time extend and data together */
> - size = rb_event_ts_length(event);
> + event_size = rb_event_ts_length(event);
>
> - if (len < size)
> + if (len < event_size)
> return -1;
>
> + if (commit & RB_MISSED_EVENTS)
> + flags = RB_MISSED_EVENTS;
> +
> /* save the current timestamp, since the user will need it */
> save_timestamp = cpu_buffer->read_stamp;
>
> @@ -7154,25 +7166,25 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> * one or two events.
> * We have already ensured there's enough space if this
> * is a time extend. */
> - size = rb_event_length(event);
> - memcpy(dpage->data + pos, rpage->data + rpos, size);
> + event_size = rb_event_length(event);
> + memcpy(dpage->data + pos, rpage->data + rpos, event_size);
>
> - len -= size;
> + len -= event_size;
>
> rb_advance_reader(cpu_buffer);
> rpos = reader->read;
> - pos += size;
> + pos += event_size;
>
> - if (rpos >= commit)
> + if (rpos >= event_size)
> break;
>
> event = rb_reader_event(cpu_buffer);
> /* Always keep the time extend and data together */
> - size = rb_event_ts_length(event);
> - } while (len >= size);
> + event_size = rb_event_ts_length(event);
> + } while (len >= event_size);
>
> /* update dpage */
> - local_set(&dpage->commit, pos);
> + local_set(&dpage->commit, pos | flags);
> dpage->time_stamp = save_timestamp;
>
> /* we copied everything to the beginning */
> @@ -7204,7 +7216,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>
> cpu_buffer->lost_events = 0;
>
> - commit = rb_data_page_commit(dpage);
> + size = rb_data_page_size(dpage);
> /*
> * Set a flag in the commit field if we lost events
> */
> @@ -7214,11 +7226,11 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> * missed events, then record it there.
> */
> if (missed_events > 0 &&
> - buffer->subbuf_size - commit >= sizeof(missed_events)) {
> - memcpy(&dpage->data[commit], &missed_events,
> + buffer->subbuf_size - size >= sizeof(missed_events)) {
> + memcpy(&dpage->data[size], &missed_events,
> sizeof(missed_events));
> local_add(RB_MISSED_STORED, &dpage->commit);
> - commit += sizeof(missed_events);
> + size += sizeof(missed_events);
> }
> local_add(RB_MISSED_EVENTS, &dpage->commit);
> }
> @@ -7226,8 +7238,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> /*
> * This page may be off to user land. Zero it out here.
> */
> - if (commit < buffer->subbuf_size)
> - memset(&dpage->data[commit], 0, buffer->subbuf_size - commit);
> + if (size < buffer->subbuf_size)
> + memset(&dpage->data[size], 0, buffer->subbuf_size - size);
>
> return read;
> }
> --
> 2.53.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
next prev parent reply other threads:[~2026-05-26 6:06 UTC|newest]
Thread overview: 17+ 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-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 [this message]
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=20260526150640.69d957c8339fd15774b79e71@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