Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Low] A hardcoded minimum page limit was missed during refactoring in 
trace_rb_cpu_prepare().
--

commit 699b707360c7ac89e4a8f2b996c220e5cb22ea93
Author: Vincent Donnefort <[email protected]>

ring-buffer: Unify ring buffer minimum page calculations

The commit introduces the RING_BUFFER_MIN_NR_PAGES macro to formalize the
requirement of having at least two pages in a ring buffer. It updates multiple
sites across the codebase to use this new unified calculation instead of
scattered hardcoded values.

> diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
> index 7a1a92f876503..a9401b8ebc721 100644
> --- a/include/linux/ring_buffer.h
> +++ b/include/linux/ring_buffer.h
> @@ -99,6 +99,13 @@ struct trace_buffer *__ring_buffer_alloc_range(unsigned 
> long size, unsigned flag
>  
>  void *ring_buffer_meta_scratch(struct trace_buffer *buffer, unsigned int 
> *size);
>  
> +/*
> + * A minimum of 2 pages ensures we can always swap a page out of the ring
> + * buffer. This does not account for the reader page or any other page
> + * containing meta data used by static ring buffers.
> + */
> +#define RING_BUFFER_MIN_NR_PAGES 2UL
> +

[Severity: Low]
Was a hardcoded minimum page limit missed during this refactoring?

Looking at kernel/trace/ring_buffer.c:trace_rb_cpu_prepare(), when a newly
hotplugged CPU is brought online and detects varying subbuffer sizes across
existing CPUs (!nr_pages_same), it still uses a hardcoded fallback limit:

kernel/trace/ring_buffer.c:trace_rb_cpu_prepare() {
    ...
        /* allocate minimum pages, user can later expand it */
        if (!nr_pages_same)
                nr_pages = 2;
    ...
}

Should this fallback allocation use the newly introduced
RING_BUFFER_MIN_NR_PAGES macro instead to avoid a maintenance hazard if the
minimum requirement changes in the future?

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to