On 7/22/26 06:38, Sarthak Sharma wrote: > Hi David! > > On 7/21/26 7:22 PM, David Hildenbrand (Arm) wrote: >> On 7/16/26 14:32, Sarthak Sharma wrote: >>> Add a command-line tool for benchmarking get_user_pages fast-path >>> (GUP_FAST), pin_user_pages fast-path (PIN_FAST), and pin_user_pages >>> longterm (PIN_LONGTERM) via the CONFIG_GUP_TEST debugfs interface. >>> >>> When invoked without arguments, gup_bench runs the benchmark subset >>> of configurations covered by run_gup_matrix() in run_vmtests.sh: all >> >> To we really need the matrix in there? The matrix was good for selftests, but >> not when using it as a benchmarking tool, no? > > Yeah makes sense, I'll remove the matrix part. > >> >>> three GUP commands across read/write, private/shared mappings, and a >>> range of page counts, with THP on/off for regular mappings and hugetlb >>> for huge page mappings. Restore HugeTLB settings after each hugetlb >>> benchmark run. Validate numeric command-line arguments instead of >>> relying on atoi(). >>> >>> This tool is a mix of reused and new logic. The mapping/setup path comes >>> from selftests/mm/gup_test.c, while the default benchmark matrix matches >>> the benchmark subset of run_gup_matrix() in run_vmtests.sh. The standalone >>> CLI and tools/mm integration are added here and the HugeTLB setup code is >>> shared via tools/lib/mm. >>> >>> Add gup_bench to BUILD_TARGETS and INSTALL_TARGETS in tools/mm/Makefile, >>> link it against the shared tools/lib/mm hugepage helpers, and ignore the >>> resulting binary in tools/mm/.gitignore. While here, also add the >>> missing thp_swap_allocator_test entry to .gitignore. >>> >>> Add tools/mm/gup_bench.c to the GUP entry in MAINTAINERS. >>> >>> Suggested-by: David Hildenbrand (Arm) <[email protected]> >>> Signed-off-by: Sarthak Sharma <[email protected]> >>> --- >>> MAINTAINERS | 1 + >>> tools/mm/.gitignore | 2 + >>> tools/mm/Makefile | 10 +- >>> tools/mm/gup_bench.c | 482 +++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 492 insertions(+), 3 deletions(-) >>> create mode 100644 tools/mm/gup_bench.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 26b06d80ab45..7d121dee9f0f 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -17024,6 +17024,7 @@ T: git >>> git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm >>> F: mm/gup.c >>> F: mm/gup_test.c >>> F: mm/gup_test.h >>> +F: tools/mm/gup_bench.c >>> F: tools/testing/selftests/mm/gup_longterm.c >>> F: tools/testing/selftests/mm/gup_test.c >>> >>> diff --git a/tools/mm/.gitignore b/tools/mm/.gitignore >>> index 922879f93fc8..154d740be02e 100644 >>> --- a/tools/mm/.gitignore >>> +++ b/tools/mm/.gitignore >>> @@ -2,3 +2,5 @@ >>> slabinfo >>> page-types >>> page_owner_sort >>> +thp_swap_allocator_test >> >> ^ Does that belong into this patch? > > No it's just that when I was working on v1 of this patchset, I saw that > this binary hadn't been added to .gitignore so I added this, since I was > already modifying .gitignore to add gup_bench. It has stuck around since > then :). Will drop this from here and send a patch separately for this.
Yes please, thanks! > >> >>> +gup_bench >> >> I wonder if it makes sense to "git mv" the existing tool (only slightly >> fixing >> header inclusion to make it compile), essentially removing the old benchmark. >> >> So it would be >> >> 1) Move tools, fixing up includes and removing the gup testing logic+matrix >> to >> make it compile >> >> 2) Extend tool by XYZ >> >> 3) Add completely new selftest >> >> Because looking at patch #5, there is not really any code reuse there, but I >> suspect there should be some in this patch here when moving instead of >> copying. >> >> That would make this a lot easier to review. >> > > I agree that this would make it easier to review, but then there would > be an intermediate commit where there is no gup_test selftest. Is this > fine from a bisectability point of view? Perfectly fine I think! -- Cheers, David
