On Mon, 12 Jan 2026 16:15:56 -0800
Guenter Roeck <[email protected]> wrote:
> > v3: Restore the code calculating the number of unused pages due to skipped
> > entries. Use the actual number of entries per page group when
> > calculating the number of entries in unused page groups.
> > v2: Have ftrace_allocate_pages() return the number of allocated pages,
> > and drop the page count calculation code as well as the associated
> > warnings from ftrace_process_locs().
>
> Any additional feedback / comments / suggestions ?
Yeah, sorry got caught up in other work.
> > -static int ftrace_allocate_records(struct ftrace_page *pg, int count)
> > +static int ftrace_allocate_records(struct ftrace_page *pg, int count,
> > + unsigned long *num_pages)
> > {
> > int order;
> > int pages;
> > @@ -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:
> > @@ -3859,6 +3859,7 @@ static int ftrace_allocate_records(struct ftrace_page
> > *pg, int count)
> > }
> >
> > ftrace_number_of_pages += 1 << order;
> > + *num_pages += 1 << order;
> > ftrace_number_of_groups++;
> >
> > cnt = (PAGE_SIZE << order) / ENTRY_SIZE;
> > @@ -3887,12 +3888,14 @@ static void ftrace_free_pages(struct ftrace_page
> > *pages)
> > }
> >
> > static struct ftrace_page *
> > -ftrace_allocate_pages(unsigned long num_to_init)
> > +ftrace_allocate_pages(unsigned long num_to_init, unsigned long *pages)
Small nit.
Can you rename this to num_pages? That way it is consistent with the above.
Both this function and ftrace_allocate_records() has this as a pointer,
whereas below it's an unsigned long. I rather have the pointer names be
consistent than having "pages" be a pointer here and an unsigned long where
it is called.
Thanks,
-- Steve
> > {
> > struct ftrace_page *start_pg;
> > struct ftrace_page *pg;
> > int cnt;
> >
> > + *pages = 0;
> > +
> > if (!num_to_init)
> > return NULL;
> >
> > @@ -3906,7 +3909,7 @@ ftrace_allocate_pages(unsigned long num_to_init)
> > * waste as little space as possible.
> > */
> > for (;;) {
> > - cnt = ftrace_allocate_records(pg, num_to_init);
> > + cnt = ftrace_allocate_records(pg, num_to_init, pages);
> > if (cnt < 0)
> > goto free_pages;
> >
> > @@ -7192,8 +7195,6 @@ static int ftrace_process_locs(struct module *mod,
> > if (!count)
> > return 0;
> >
> > - pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> > -
> > /*
> > * Sorting mcount in vmlinux at build time depend on
> > * CONFIG_BUILDTIME_MCOUNT_SORT, while mcount loc in
> > @@ -7206,7 +7207,7 @@ static int ftrace_process_locs(struct module *mod,
> > test_is_sorted(start, count);
> > }
> >
> > - start_pg = ftrace_allocate_pages(count);
> > + start_pg = ftrace_allocate_pages(count, &pages);
> > if (!start_pg)
> > return -ENOMEM;
> >
> > @@ -7305,27 +7306,27 @@ static int ftrace_process_locs(struct module *mod,
> > /* We should have used all pages unless we skipped some */
> > if (pg_unuse) {
> > unsigned long pg_remaining, remaining = 0;
> > - unsigned long skip;
> > + 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")) {
> >
> > skip = skipped - pg_remaining;
> >
> > - for (pg = pg_unuse; pg; pg = pg->next)
> > + for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
> > remaining += 1 << pg->order;
> > + skip -= (PAGE_SIZE << pg->order) / ENTRY_SIZE;
> > + }
> >
> > pages -= remaining;
> >
> > - 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.
> > */
> > - WARN(skip != remaining, "Extra allocated pages for
> > ftrace: %lu with %lu skipped",
> > + WARN(pg || skip > 0, "Extra allocated pages for ftrace:
> > %lu with %lu skipped",
> > remaining, skipped);
> > }
> > /* Need to synchronize with ftrace_location_range() */
