Re: [PATCH 4/7] tracing: account ring buffer pages and trace_array metadata to memcg

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 11 Nov 2026 16:47:38 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:

> On Fri, Nov 06, 2026 at 09:18:26PM +0800, Tao Cui wrote:
> > From: Tao Cui <cuitao@kylinos.cn>
> > 
> > Creating an instance immediately allocates two full sets of per-cpu
> > ring buffers, array buffer plus snapshot, about 1.375MB per cpu per set
> > on the default config, plus the trace_array and its name.  None of it
> > is charged: neither trace.c nor ring_buffer.c passes __GFP_ACCOUNT,
> > while user_events in the same directory accounts everything it
> > allocates.

As the name suggests, "user_events" are for users and are special.

> > 
> > Instance creation is currently gated by tracefs file permissions
> > alone, so a
> > rootful container with the host tracefs bound in can create instances
> > until the host runs out of memory.

You mean, "works as expected"?

>  Charge the ring buffer pages and
> > the trace_array metadata to the creating cgroup.
> > 
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> > ---
> >  kernel/trace/ring_buffer.c | 8 +++++++-
> >  kernel/trace/trace.c       | 6 +++---
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 1fd28a72e6ee..676c059c112c 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -441,7 +441,13 @@ static struct buffer_data_page *alloc_cpu_data(int cpu, int order)
> >  	 * gracefully without invoking oom-killer and the system is not
> >  	 * destabilized.
> >  	 */
> > -	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO;
> > +	/*
> > +	 * The subbuffers are charged to the memcg of whoever caused the
> > +	 * allocation: the creator of a trace instance, or root at boot for
> > +	 * the global trace.
> > +	 */
> > +	mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO |
> > +		  __GFP_ACCOUNT;
> >  
> >  	page = alloc_pages_node(cpu_to_node(cpu), mflags, order);
> >  	if (!page)  
> 
> Hum, by default tracefs is root only. 
> 
> And even if made accessible to other users: 
> 
>   1. The life span of a trace instance is not tied to the caller.
>   2. The ring buffer is shared by multiple users.

Right. Is there a use case out there that allows non-admin to create an
instance? Tracing ring buffers can expose a lot and should not be given
to unprivileged users.

What exactly is this trying to achieve?

-- Steve


> 
> Overall I am not convinced charging the creator makes much sense.
> 
> Eventually a few allocations for iterators could be GFP_ACCOUNT.
>