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 <[email protected]> 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/[email protected]?part=1
