> Add a new GUP selftest which uses kselftest_harness.h. Cover
> 12 mapping configurations: THP enabled, THP disabled and
> HugeTLB, each across private/shared mappings and with/without
> FOLL_WRITE. Run 7 testcases for every variant: get_user_pages,
> get_user_pages_fast, pin_user_pages, pin_user_pages_fast,
> pin_user_pages_longterm, and DUMP_USER_PAGES_TEST using both
> get and pin.
>
> Sweep four nr_pages_per_call values for each test: 1, 512, 123 and
> all pages. This preserves the coverage previously provided by
> run_gup_matrix(): 12 mapping combinations x 5 GUP/PUP operations x 4
> batch sizes, for 240 ioctl calls. The two dump modes add another 96
> calls.
>
> Preserve the previous sparse dump coverage with a standalone test for
> pages 0, 19 and 0x1000. In total the selftest reports 85 TAP
> cases and issues 337 ioctls.
>
> Add the new gup binary to the selftests/mm build, .gitignore,
> run_vmtests.sh and MAINTAINERS. Update
> Documentation/core-api/pin_user_pages.rst for the new test.
>
> Suggested-by: David Hildenbrand (Arm) <[email protected]>
> Signed-off-by: Sarthak Sharma <[email protected]>
>
> +
> +FIXTURE_SETUP(gup_test)
> +{
> + int mmap_flags = MAP_PRIVATE;
> + int zero_fd;
> + char *p;
> +
> + /* zero_fd has to be >= 0. Already checked in main() */
> + zero_fd = open("/dev/zero", O_RDWR);
> + ASSERT_GE(zero_fd, 0);
> +
> + /* gup_fd has to be >= 0. Already checked in main() */
> + self->gup_fd = open(GUP_TEST_FILE, O_RDWR);
> + ASSERT_GE(self->gup_fd, 0);
> +
> + self->size = variant->hugetlb ? 256 * MB : 128 * MB;
I'd derive the hugetbl variant size from the size of a huge page and
predefined number of huge pages.
> +
> + if (variant->hugetlb) {
> + unsigned long hp_size = default_huge_page_size();
> +
> + if (!hp_size) {
> + close(zero_fd);
> + close(self->gup_fd);
You can move hugetlb setup after opening those and save the headache of
closing them.
And in any rate prefer
goto err_do_cleanup
to
if (something_failed) {
cleanup1();
cleanup2();
}
if (something_else_failed) {
cleanup1();
cleanup2();
cleanup3();
}
> + SKIP(return, "HugeTLB not available\n");
> + }
> +
> + self->size = (self->size + hp_size - 1) & ~(hp_size - 1);
> + if (!hugetlb_setup_default(self->size / hp_size)) {
> + hugetlb_restore_settings();
No need to call restore() here.
Also, if you use a constant number of huge pages you can just add
HUGETLB_SETUP_DEFAULT_PAGES(NR_HUGE_PAGES) somewhere in the begining and
you won't need to setup and teardown hugetlb explicitly for every
hugetlb test.
> + close(zero_fd);
> + close(self->gup_fd);
> + SKIP(return, "Not enough huge pages\n");
> + }
> +
> + mmap_flags |= (MAP_HUGETLB | MAP_ANONYMOUS);
> + }
> +
> + if (variant->shared)
> + mmap_flags = (mmap_flags & ~MAP_PRIVATE) | MAP_SHARED;
> +
> + self->addr = mmap(NULL, self->size, PROT_READ | PROT_WRITE,
> + mmap_flags, zero_fd, 0);
> +
> + ASSERT_NE(self->addr, MAP_FAILED) {
> + int err = errno;
> +
> + close(zero_fd);
> + close(self->gup_fd);
> + if (variant->hugetlb)
> + hugetlb_restore_settings();
> + TH_LOG("mmap failed: %s", strerror(err));
> + }
> + close(zero_fd);
> +
> + if (variant->thp)
> + madvise(self->addr, self->size, MADV_HUGEPAGE);
> + else if (!variant->hugetlb)
> + madvise(self->addr, self->size, MADV_NOHUGEPAGE);
> +
> + for (p = self->addr; (unsigned long)p < (unsigned long)self->addr
> + + self->size; p += psize())
> + p[0] = 0;
> +}
> +
> +FIXTURE_TEARDOWN(gup_test)
> +{
> + munmap(self->addr, self->size);
> + close(self->gup_fd);
> +
> + if (variant->hugetlb)
> + hugetlb_restore_settings();
> +}
> +
> +static void run_gup_cmd(struct __test_metadata *_metadata,
> + FIXTURE_DATA(gup_test) *self,
> + const FIXTURE_VARIANT(gup_test) *variant,
> + unsigned long command,
> + unsigned int test_flags,
> + unsigned int which_page)
> +{
> + int i;
> +
> + for (i = 0; i < (int)ARRAY_SIZE(nr_pages_list); i++) {
> + struct gup_test gup = {
> + .addr = (unsigned long)self->addr,
> + .size = self->size,
> + .nr_pages_per_call = nr_pages_list[i] < 0 ?
> + self->size / psize() : nr_pages_list[i],
> + .test_flags = test_flags,
> + };
> +
> + if (variant->write)
> + gup.gup_flags |= FOLL_WRITE;
> +
> + gup.which_pages[0] = which_page;
Can't these to go to the static initialization?
--
Sincerely yours,
Mike.
