Skip to content

Commit 8ba6e86

Browse files
CmdrMoozytorvalds
authored andcommitted
userfaultfd/selftests: reinitialize test context in each test
Currently, the context (fds, mmap-ed areas, etc.) are global. Each test mutates this state in some way, in some cases really "clobbering it" (e.g., the events test mremap-ing area_dst over the top of area_src, or the minor faults tests overwriting the count_verify values in the test areas). We run the tests in a particular order, each test is careful to make the right assumptions about its starting state, etc. But, this is fragile. It's better for a test's success or failure to not depend on what some other prior test case did to the global state. To that end, clear and reinitialize the test context at the start of each test case, so whatever prior test cases did doesn't affect future tests. This is particularly relevant to this series because the events test's mremap of area_dst screws up assumptions the minor fault test was relying on. This wasn't a problem for hugetlb, as we don't mremap in that case. [[email protected]: fix conflict between this patch and the uffd pagemap series] Link: https://lkml.kernel.org/r/YKQqKrl+/cQ1utrb@t490s Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Axel Rasmussen <[email protected]> Signed-off-by: Peter Xu <[email protected]> Reviewed-by: Peter Xu <[email protected]> Cc: Alexander Viro <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: Brian Geffon <[email protected]> Cc: "Dr . David Alan Gilbert" <[email protected]> Cc: Hugh Dickins <[email protected]> Cc: Jerome Glisse <[email protected]> Cc: Joe Perches <[email protected]> Cc: Kirill A. Shutemov <[email protected]> Cc: Lokesh Gidra <[email protected]> Cc: Mike Kravetz <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Mina Almasry <[email protected]> Cc: Oliver Upton <[email protected]> Cc: Shaohua Li <[email protected]> Cc: Shuah Khan <[email protected]> Cc: Stephen Rothwell <[email protected]> Cc: Wang Qing <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 5bb23ed commit 8ba6e86

File tree

1 file changed

+117
-105
lines changed

1 file changed

+117
-105
lines changed

tools/testing/selftests/vm/userfaultfd.c

Lines changed: 117 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ static int shm_fd;
8989
static int huge_fd;
9090
static char *huge_fd_off0;
9191
static unsigned long long *count_verify;
92-
static int uffd, uffd_flags, finished, *pipefd;
92+
static int uffd = -1;
93+
static int uffd_flags, finished, *pipefd;
9394
static char *area_src, *area_src_alias, *area_dst, *area_dst_alias;
9495
static char *zeropage;
9596
pthread_attr_t attr;
@@ -342,6 +343,111 @@ static struct uffd_test_ops hugetlb_uffd_test_ops = {
342343

343344
static struct uffd_test_ops *uffd_test_ops;
344345

346+
static void userfaultfd_open(uint64_t *features)
347+
{
348+
struct uffdio_api uffdio_api;
349+
350+
uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
351+
if (uffd < 0)
352+
err("userfaultfd syscall not available in this kernel");
353+
uffd_flags = fcntl(uffd, F_GETFD, NULL);
354+
355+
uffdio_api.api = UFFD_API;
356+
uffdio_api.features = *features;
357+
if (ioctl(uffd, UFFDIO_API, &uffdio_api))
358+
err("UFFDIO_API failed.\nPlease make sure to "
359+
"run with either root or ptrace capability.");
360+
if (uffdio_api.api != UFFD_API)
361+
err("UFFDIO_API error: %" PRIu64, (uint64_t)uffdio_api.api);
362+
363+
*features = uffdio_api.features;
364+
}
365+
366+
static inline void munmap_area(void **area)
367+
{
368+
if (*area)
369+
if (munmap(*area, nr_pages * page_size))
370+
err("munmap");
371+
372+
*area = NULL;
373+
}
374+
375+
static void uffd_test_ctx_clear(void)
376+
{
377+
size_t i;
378+
379+
if (pipefd) {
380+
for (i = 0; i < nr_cpus * 2; ++i) {
381+
if (close(pipefd[i]))
382+
err("close pipefd");
383+
}
384+
free(pipefd);
385+
pipefd = NULL;
386+
}
387+
388+
if (count_verify) {
389+
free(count_verify);
390+
count_verify = NULL;
391+
}
392+
393+
if (uffd != -1) {
394+
if (close(uffd))
395+
err("close uffd");
396+
uffd = -1;
397+
}
398+
399+
huge_fd_off0 = NULL;
400+
munmap_area((void **)&area_src);
401+
munmap_area((void **)&area_src_alias);
402+
munmap_area((void **)&area_dst);
403+
munmap_area((void **)&area_dst_alias);
404+
}
405+
406+
static void uffd_test_ctx_init_ext(uint64_t *features)
407+
{
408+
unsigned long nr, cpu;
409+
410+
uffd_test_ctx_clear();
411+
412+
uffd_test_ops->allocate_area((void **)&area_src);
413+
uffd_test_ops->allocate_area((void **)&area_dst);
414+
415+
uffd_test_ops->release_pages(area_src);
416+
uffd_test_ops->release_pages(area_dst);
417+
418+
userfaultfd_open(features);
419+
420+
count_verify = malloc(nr_pages * sizeof(unsigned long long));
421+
if (!count_verify)
422+
err("count_verify");
423+
424+
for (nr = 0; nr < nr_pages; nr++) {
425+
*area_mutex(area_src, nr) =
426+
(pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;
427+
count_verify[nr] = *area_count(area_src, nr) = 1;
428+
/*
429+
* In the transition between 255 to 256, powerpc will
430+
* read out of order in my_bcmp and see both bytes as
431+
* zero, so leave a placeholder below always non-zero
432+
* after the count, to avoid my_bcmp to trigger false
433+
* positives.
434+
*/
435+
*(area_count(area_src, nr) + 1) = 1;
436+
}
437+
438+
pipefd = malloc(sizeof(int) * nr_cpus * 2);
439+
if (!pipefd)
440+
err("pipefd");
441+
for (cpu = 0; cpu < nr_cpus; cpu++)
442+
if (pipe2(&pipefd[cpu * 2], O_CLOEXEC | O_NONBLOCK))
443+
err("pipe");
444+
}
445+
446+
static inline void uffd_test_ctx_init(uint64_t features)
447+
{
448+
uffd_test_ctx_init_ext(&features);
449+
}
450+
345451
static int my_bcmp(char *str1, char *str2, size_t n)
346452
{
347453
unsigned long i;
@@ -726,40 +832,6 @@ static int stress(struct uffd_stats *uffd_stats)
726832
return 0;
727833
}
728834

729-
static int userfaultfd_open_ext(uint64_t *features)
730-
{
731-
struct uffdio_api uffdio_api;
732-
733-
uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
734-
if (uffd < 0) {
735-
fprintf(stderr,
736-
"userfaultfd syscall not available in this kernel\n");
737-
return 1;
738-
}
739-
uffd_flags = fcntl(uffd, F_GETFD, NULL);
740-
741-
uffdio_api.api = UFFD_API;
742-
uffdio_api.features = *features;
743-
if (ioctl(uffd, UFFDIO_API, &uffdio_api)) {
744-
fprintf(stderr, "UFFDIO_API failed.\nPlease make sure to "
745-
"run with either root or ptrace capability.\n");
746-
return 1;
747-
}
748-
if (uffdio_api.api != UFFD_API) {
749-
fprintf(stderr, "UFFDIO_API error: %" PRIu64 "\n",
750-
(uint64_t)uffdio_api.api);
751-
return 1;
752-
}
753-
754-
*features = uffdio_api.features;
755-
return 0;
756-
}
757-
758-
static int userfaultfd_open(uint64_t features)
759-
{
760-
return userfaultfd_open_ext(&features);
761-
}
762-
763835
sigjmp_buf jbuf, *sigbuf;
764836

765837
static void sighndl(int sig, siginfo_t *siginfo, void *ptr)
@@ -868,6 +940,8 @@ static int faulting_process(int signal_test)
868940
MREMAP_MAYMOVE | MREMAP_FIXED, area_src);
869941
if (area_dst == MAP_FAILED)
870942
err("mremap");
943+
/* Reset area_src since we just clobbered it */
944+
area_src = NULL;
871945

872946
for (; nr < nr_pages; nr++) {
873947
count = *area_count(area_dst, nr);
@@ -961,10 +1035,8 @@ static int userfaultfd_zeropage_test(void)
9611035
printf("testing UFFDIO_ZEROPAGE: ");
9621036
fflush(stdout);
9631037

964-
uffd_test_ops->release_pages(area_dst);
1038+
uffd_test_ctx_init(0);
9651039

966-
if (userfaultfd_open(0))
967-
return 1;
9681040
uffdio_register.range.start = (unsigned long) area_dst;
9691041
uffdio_register.range.len = nr_pages * page_size;
9701042
uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
@@ -981,7 +1053,6 @@ static int userfaultfd_zeropage_test(void)
9811053
if (my_bcmp(area_dst, zeropage, page_size))
9821054
err("zeropage is not zero");
9831055

984-
close(uffd);
9851056
printf("done.\n");
9861057
return 0;
9871058
}
@@ -999,12 +1070,10 @@ static int userfaultfd_events_test(void)
9991070
printf("testing events (fork, remap, remove): ");
10001071
fflush(stdout);
10011072

1002-
uffd_test_ops->release_pages(area_dst);
1003-
10041073
features = UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_EVENT_REMAP |
10051074
UFFD_FEATURE_EVENT_REMOVE;
1006-
if (userfaultfd_open(features))
1007-
return 1;
1075+
uffd_test_ctx_init(features);
1076+
10081077
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
10091078

10101079
uffdio_register.range.start = (unsigned long) area_dst;
@@ -1037,8 +1106,6 @@ static int userfaultfd_events_test(void)
10371106
if (pthread_join(uffd_mon, NULL))
10381107
return 1;
10391108

1040-
close(uffd);
1041-
10421109
uffd_stats_report(&stats, 1);
10431110

10441111
return stats.missing_faults != nr_pages;
@@ -1058,11 +1125,9 @@ static int userfaultfd_sig_test(void)
10581125
printf("testing signal delivery: ");
10591126
fflush(stdout);
10601127

1061-
uffd_test_ops->release_pages(area_dst);
1062-
10631128
features = UFFD_FEATURE_EVENT_FORK|UFFD_FEATURE_SIGBUS;
1064-
if (userfaultfd_open(features))
1065-
return 1;
1129+
uffd_test_ctx_init(features);
1130+
10661131
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
10671132

10681133
uffdio_register.range.start = (unsigned long) area_dst;
@@ -1103,7 +1168,6 @@ static int userfaultfd_sig_test(void)
11031168
printf("done.\n");
11041169
if (userfaults)
11051170
err("Signal test failed, userfaults: %ld", userfaults);
1106-
close(uffd);
11071171

11081172
return userfaults != 0;
11091173
}
@@ -1126,10 +1190,7 @@ static int userfaultfd_minor_test(void)
11261190
printf("testing minor faults: ");
11271191
fflush(stdout);
11281192

1129-
uffd_test_ops->release_pages(area_dst);
1130-
1131-
if (userfaultfd_open_ext(&features))
1132-
return 1;
1193+
uffd_test_ctx_init_ext(&features);
11331194
/* If kernel reports the feature isn't supported, skip the test. */
11341195
if (!(features & UFFD_FEATURE_MINOR_HUGETLBFS)) {
11351196
printf("skipping test due to lack of feature support\n");
@@ -1183,8 +1244,6 @@ static int userfaultfd_minor_test(void)
11831244
if (pthread_join(uffd_mon, NULL))
11841245
return 1;
11851246

1186-
close(uffd);
1187-
11881247
uffd_stats_report(&stats, 1);
11891248

11901249
return stats.missing_faults != 0 || stats.minor_faults != nr_pages;
@@ -1267,7 +1326,7 @@ static void userfaultfd_pagemap_test(unsigned int test_pgsize)
12671326
/* Flush so it doesn't flush twice in parent/child later */
12681327
fflush(stdout);
12691328

1270-
uffd_test_ops->release_pages(area_dst);
1329+
uffd_test_ctx_init(0);
12711330

12721331
if (test_pgsize > page_size) {
12731332
/* This is a thp test */
@@ -1279,9 +1338,6 @@ static void userfaultfd_pagemap_test(unsigned int test_pgsize)
12791338
err("madvise(MADV_NOHUGEPAGE) failed");
12801339
}
12811340

1282-
if (userfaultfd_open(0))
1283-
err("userfaultfd_open");
1284-
12851341
uffdio_register.range.start = (unsigned long) area_dst;
12861342
uffdio_register.range.len = nr_pages * page_size;
12871343
uffdio_register.mode = UFFDIO_REGISTER_MODE_WP;
@@ -1324,7 +1380,6 @@ static void userfaultfd_pagemap_test(unsigned int test_pgsize)
13241380
pagemap_check_wp(value, false);
13251381

13261382
close(pagemap_fd);
1327-
close(uffd);
13281383
printf("done\n");
13291384
}
13301385

@@ -1334,50 +1389,9 @@ static int userfaultfd_stress(void)
13341389
char *tmp_area;
13351390
unsigned long nr;
13361391
struct uffdio_register uffdio_register;
1337-
unsigned long cpu;
13381392
struct uffd_stats uffd_stats[nr_cpus];
13391393

1340-
uffd_test_ops->allocate_area((void **)&area_src);
1341-
if (!area_src)
1342-
return 1;
1343-
uffd_test_ops->allocate_area((void **)&area_dst);
1344-
if (!area_dst)
1345-
return 1;
1346-
1347-
if (userfaultfd_open(0))
1348-
return 1;
1349-
1350-
count_verify = malloc(nr_pages * sizeof(unsigned long long));
1351-
if (!count_verify) {
1352-
perror("count_verify");
1353-
return 1;
1354-
}
1355-
1356-
for (nr = 0; nr < nr_pages; nr++) {
1357-
*area_mutex(area_src, nr) = (pthread_mutex_t)
1358-
PTHREAD_MUTEX_INITIALIZER;
1359-
count_verify[nr] = *area_count(area_src, nr) = 1;
1360-
/*
1361-
* In the transition between 255 to 256, powerpc will
1362-
* read out of order in my_bcmp and see both bytes as
1363-
* zero, so leave a placeholder below always non-zero
1364-
* after the count, to avoid my_bcmp to trigger false
1365-
* positives.
1366-
*/
1367-
*(area_count(area_src, nr) + 1) = 1;
1368-
}
1369-
1370-
pipefd = malloc(sizeof(int) * nr_cpus * 2);
1371-
if (!pipefd) {
1372-
perror("pipefd");
1373-
return 1;
1374-
}
1375-
for (cpu = 0; cpu < nr_cpus; cpu++) {
1376-
if (pipe2(&pipefd[cpu*2], O_CLOEXEC | O_NONBLOCK)) {
1377-
perror("pipe");
1378-
return 1;
1379-
}
1380-
}
1394+
uffd_test_ctx_init(0);
13811395

13821396
if (posix_memalign(&area, page_size, page_size))
13831397
err("out of memory");
@@ -1498,8 +1512,6 @@ static int userfaultfd_stress(void)
14981512
uffd_stats_report(uffd_stats, nr_cpus);
14991513
}
15001514

1501-
close(uffd);
1502-
15031515
if (test_type == TEST_ANON) {
15041516
/*
15051517
* shmem/hugetlb won't be able to run since they have different

0 commit comments

Comments
 (0)