On Tue, 6 Jan 2026 12:35:33 -0800
Guenter Roeck <[email protected]> wrote:
> The pg_remaining calculation in ftrace_process_locs() assumes that
> ENTRIES_PER_PAGE multiplied by 2^order equals the actual capacity of the
> allocated page group. However, ENTRIES_PER_PAGE is PAGE_SIZE / ENTRY_SIZE
> (integer division). When PAGE_SIZE is not a multiple of ENTRY_SIZE (e.g.
> 4096 / 24 = 170 with remainder 16), high-order allocations (like 256 pages)
> have significantly more capacity than 256 * 170. This leads to pg_remaining
> being underestimated, which in turn makes skip (derived from skipped -
> pg_remaining) larger than expected, causing the WARN(skip != remaining)
> to trigger.
Nice catch! I guess you have a machine that allows much higher order
allocations than I do ;-)
>
> Extra allocated pages for ftrace: 2 with 654 skipped
> WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7295
> ftrace_process_locs+0x5bf/0x5e0
>
> A similar problem in ftrace_allocate_records() can result in allocating
> too many pages. This can trigger the second warning in
> ftrace_process_locs().
>
> Extra allocated pages for ftrace
> WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7276
> ftrace_process_locs+0x548/0x580
>
> Use the actual capacity of a page group to determine if too many pages
> have been allocated to solve the problem. Also use the actual capacity
> of a page group to determine the number of pages needed to avoid over-
> allocations in ftrace_allocate_records().
>
> Fixes: 4a3efc6baff93 ("ftrace: Update the mcount_loc check of skipped
> entries")
> Cc: Steven Rostedt <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>
> ---
> kernel/trace/ftrace.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index ef2d5dca6f70..211ec7a04f7e 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3844,7 +3844,7 @@ static int ftrace_allocate_records(struct ftrace_page
> *pg, int count)
> return -EINVAL;
>
> /* We want to fill as much as possible, with no empty pages */
> - pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> + pages = DIV_ROUND_UP(count * ENTRY_SIZE, PAGE_SIZE);
> order = fls(pages) - 1;
>
> again:
> @@ -7308,24 +7308,33 @@ static int ftrace_process_locs(struct module *mod,
> unsigned long skip;
>
> /* Count the number of entries unused and compare it to
> skipped. */
> - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> + pg_remaining = (PAGE_SIZE << pg->order) / ENTRY_SIZE -
> pg->index;
>
> if (!WARN(skipped < pg_remaining, "Extra allocated pages for
> ftrace")) {
> + unsigned long space = 0;
>
> skip = skipped - pg_remaining;
>
> - for (pg = pg_unuse; pg; pg = pg->next)
> + for (pg = pg_unuse; pg; pg = pg->next) {
> remaining += 1 << pg->order;
> + /*
> + * The capacity of a page group is
> + * (PAGE_SIZE << order) / ENTRY_SIZE
> + * Accumulate the total capacity of unused
> pages.
> + */
> + space += (PAGE_SIZE << pg->order) / ENTRY_SIZE;
> + }
>
> pages -= remaining;
I think pages is meaningless here, as it was set in the beginning with:
pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
Which is incorrect. I wonder if we should set it via:
/*
* Use ftrace_number_of_pages to determine how many pages were
* allocated
*/
pages = ftrace_number_of_pages;
start_pg = ftrace_allocate_pages(count);
if (!start_pg)
return -ENOMEM;
/* ftrace_allocate_pages() increments ftrace_number_of_pages */
pages = ftrace_number_of_pages - pages;
This will make pages equal the number of pages that were allocated. Then
I'm not sure we need this extra logic.
-- Steve
>
> - skip = DIV_ROUND_UP(skip, ENTRIES_PER_PAGE);
> -
> /*
> - * Check to see if the number of pages remaining would
> - * just fit the number of entries skipped.
> + * Check to see if extra pages have been allocated.
> + * Only warn if the number of unused entries is larger
> + * than the number of entries per page to avoid false
> + * positives due to rounding.
> */
> - WARN(skip != remaining, "Extra allocated pages for
> ftrace: %lu with %lu skipped",
> + WARN(space - skip > ENTRIES_PER_PAGE,
> + "Extra allocated pages for ftrace: %lu with %lu
> skipped",
> remaining, skipped);
> }
> /* Need to synchronize with ftrace_location_range() */
