All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Donnefort <vdonnefort@google.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-trace-kernel@vger.kernel.org,
	mathieu.desnoyers@efficios.com, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] ring-buffer: Prevent resizing of persistent ring buffer
Date: Mon, 10 Aug 2026 09:56:53 +0100	[thread overview]
Message-ID: <anmSVUVPVQLPzcaa@google.com> (raw)
In-Reply-To: <20260810173137.e8c5337fef0f989faed995e4@kernel.org>

On Mon, Aug 10, 2026 at 05:31:37PM +0900, Masami Hiramatsu wrote:
> On Fri, 7 Aug 2026 15:26:41 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Fri, 7 Aug 2026 16:45:23 +0100
> > Vincent Donnefort <vdonnefort@google.com> wrote:
> > 
> > > free_reserved_page() would do actually. But then it is definitive. 
> > 
> > It's not always a reserved page.
> > 
> > > 
> > > Happy to implement something like that. That also means that the instance can be
> > > actually freed?
> > 
> > They can be freed now. Try a rmdir on one.

I meant free_buffer_page() skips the memory for the persistent buffer.

> > 
> > Note implementing this is not straight forward. What I would suggest is

Yeah, I started looking at it and quickly understood :)

> > that because the persistent ring buffers are contiguous, to resize, you
> > basically need to remap to the new size. That means each of the buffers
> > will still be attached to each other.
> > 
> > What would need to be done is:
> > 
> >   1. calculate the new size needed to accommodate all the CPU buffers.
> >   2. Split them up within the new size region.
> >   3. Then free the remaining pages.
> > 
> > Obviously, access to the buffer from readers and writers will need to be
> > prohibited while this is happening.

That would mean losing all the data on the buffer? Actually the trace_remote
needs to teardown the whole buffer before the size is modified. Perhaps if
something similar is done for the persistent buffer, trace_remote could benefit.

> 
> Hmm, I think we also need to record the size of persistent ring buffer at
> initialization. The buffer size (number of pages are calculated by the
> size of reserved memory, which is defined in the kernel cmdline.

Could we store in ring_buffer_cpu_meta::buffers that the page is gone and must
be skipped to re-create the ring-buffer?

> 
> So if we have `reserve_mem=12M:4096:trace trace_instance=boot_map@trace` on
> the kernel cmdline, and we write the buffer_size = 1024(KB) on 2CPU machine,
> it can be shrinked down to ~2MB on the reserved memory. However, when we
> reboot the machine, the kernel calculates the size as 12MB again, and may
> get a validation failure.
> 
> So we also need to add nr_pages and nr_cpus (maybe) on ring_buffer_meta and
> calculate the ring buffer size from it, instead of using the reserved memory
> size (we also need to check the calculated size is smaller than that.)
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

-- 
Vincent

  reply	other threads:[~2026-08-10  8:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 21:13 [PATCH 0/6] ring-buffer: Fixes for subbuf resizing and persistent buffers Vincent Donnefort
2026-08-06 21:13 ` [PATCH 1/6] ring-buffer: Prevent resizing of persistent ring buffer Vincent Donnefort
2026-08-07  2:18   ` Masami Hiramatsu
2026-08-07  8:14     ` Vincent Donnefort
2026-08-07  9:43       ` Vincent Donnefort
2026-08-07 10:25         ` Vincent Donnefort
2026-08-07 14:45         ` Steven Rostedt
2026-08-07 15:25           ` Vincent Donnefort
2026-08-07 15:45             ` Vincent Donnefort
2026-08-07 19:26               ` Steven Rostedt
2026-08-10  8:31                 ` Masami Hiramatsu
2026-08-10  8:56                   ` Vincent Donnefort [this message]
2026-08-10 14:13                     ` Steven Rostedt
2026-08-06 21:13 ` [PATCH 2/6] ring-buffer: Prevent subbuf order change when resizing is disabled Vincent Donnefort
2026-08-06 21:13 ` [PATCH 3/6] ring-buffer: Initialise reader page order in rb_allocate_cpu_buffer() Vincent Donnefort
2026-08-06 21:13 ` [PATCH 4/6] ring-buffer: Fix subbuf resize concurrency Vincent Donnefort
2026-08-08 18:58   ` Steven Rostedt
2026-08-10  7:54     ` Vincent Donnefort
2026-08-10 13:26       ` Steven Rostedt
2026-08-10 13:38         ` Vincent Donnefort
2026-08-06 21:13 ` [PATCH 5/6] ring-buffer: Dynamically calculate max_data_size Vincent Donnefort
2026-08-06 21:13 ` [PATCH 6/6] ring-buffer: Remove ring_buffer_per_cpu::mapped Vincent Donnefort

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=anmSVUVPVQLPzcaa@google.com \
    --to=vdonnefort@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.