Re: [PATCH v2 1/2] tracing: add ring-buffer memory usage statistics in tracefs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Persistent memory-mapped trace buffers are incorrectly accounted as page-allocator memory, leading to double-counting in lost RAM calculations.
- [Low] Evaluation of a struct type as a boolean condition causes a hard compilation error.
--

commit 339a95b19ea8f13ff179992c9814bfee94fa38b3
Author: Xiang Gao <gxxa03070307@gmail.com>

tracing: add ring-buffer memory usage statistics in tracefs

This commit adds memory usage reporting for tracing ring buffers to calculate
memory footprint more accurately than relying on buffer capacity. It exposes
a new memory_usage_kb file in tracefs which sums up the page allocator memory
used by the data pages and reader pages.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 9c03a555a6ba2..73e79c86035c2 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> @@ -6560,6 +6560,47 @@ unsigned long ring_buffer_size(struct trace_buffer *buffer, int cpu)
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_size);
>  
> +/**
> + * ring_buffer_memory_size - return the memory used by the buffer (in bytes)
[ ... ]
> + */
> +unsigned long ring_buffer_memory_size(struct trace_buffer *buffer, int cpu)
> +{
> +	struct ring_buffer_per_cpu *cpu_buffer;
> +	unsigned long subbuf_size;
> +	unsigned long size;
> +
> +	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> +		return 0;
> +
> +	/* Remote buffers use externally owned memory. */
> +	if (buffer->remote)
> +		return 0;

[Severity: Medium]
Does this check also need to exclude persistent memory-mapped trace buffers
where buffer->range_addr_start is set? Since those buffers also use
externally owned or reserved memory bypassing the page-allocator, not
skipping them here might result in double-counting their memory in lost
RAM calculations.

> +
> +	cpu_buffer = buffer->buffers[cpu];
> +	subbuf_size = PAGE_SIZE << READ_ONCE(buffer->subbuf_order);
> +
> +	/* Data sub-buffers plus the reader page. */
> +	size = (cpu_buffer->nr_pages + 1) * subbuf_size;
> +
> +	/* The cached read page, if present, is a full sub-buffer page. */
> +	if (cpu_buffer->free_page)
> +		size += subbuf_size;

[Severity: Low]
Does evaluating cpu_buffer->free_page directly as a boolean condition
cause a compiler error here, since free_page is an embedded struct
(struct buffer_data_read_page) rather than a pointer? Should this check its
internal data pointer instead, such as if (cpu_buffer->free_page.data)?

> +
> +	/* The mmap metadata page is a single system page. */
> +	if (cpu_buffer->meta_page)
> +		size += PAGE_SIZE;
> +
> +	return size;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911155017.3377254-1-gaoxiang17@xiaomi.com?part=1