On Tue, Jan 06, 2026 at 02:26:23PM -0800, Guenter Roeck wrote: > > > > > > 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: > > > I know, I just didn't want to make more changes than absolutely > necessary. > > > /* > > * 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; > > > > That might work, assuming that the code updating ftrace_number_of_pages > is (mutex) protected. I don't immediately see that, and the > "mutex_lock(&ftrace_lock);" right after the above code makes me a bit > concerned. >
One way to avoid the locking problem without potentially risky code changes would be to pass a pointer to pages to ftrace_allocate_pages() and to ftrace_allocate_records(), and to update it from there. I tested that and confirmed that it works. Guenter
