| Message ID | 20250113032858.63670-1-hao.ge@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/mm/cow: Modify the incorrect checking parameters | expand |
Hello Hao, On 1/13/25 08:58, Hao Ge wrote: > From: Hao Ge <gehao@kylinos.cn> > > In the run_with_memfd_hugetlb function, some error handle > have passed incorrect parameters. > It should be "smem", but it was mistakenly written as "mem". I guess there are couple of more instances where the returned address 'smem' is not getting tested for MAP_FAILED. Hence the commit message here needs to be bit more generic rather than run_with_memfd_hugetlb() specific. --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -1482,7 +1482,7 @@ static void run_with_zeropage(non_anon_test_fn fn, const char *desc) } smem = mmap(NULL, pagesize, PROT_READ, MAP_PRIVATE | MAP_ANON, -1, 0); - if (mem == MAP_FAILED) { + if (smem == MAP_FAILED) { ksft_test_result_fail("mmap() failed\n"); goto munmap; } @@ -1583,7 +1583,7 @@ static void run_with_memfd(non_anon_test_fn fn, const char *desc) goto close; } smem = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); - if (mem == MAP_FAILED) { + if (smem == MAP_FAILED) { ksft_test_result_fail("mmap() failed\n"); goto munmap; } > > Let's fix it. > > Fixes: baa489fabd01 ("selftests/vm: rename selftests/vm to selftests/mm") This commit just renamed the directory from vm/ to mm/ directory. The following commit introduced the problem instead. Please update the Fixes: tag as required. f8664f3c4a08f799 ("selftests/vm: cow: basic COW tests for non-anonymous pages") > Signed-off-by: Hao Ge <gehao@kylinos.cn> > --- > tools/testing/selftests/mm/cow.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c > index 32c6ccc2a6be..7a89680d1566 100644 > --- a/tools/testing/selftests/mm/cow.c > +++ b/tools/testing/selftests/mm/cow.c > @@ -1684,7 +1684,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, > goto close; > } > smem = mmap(NULL, hugetlbsize, PROT_READ, MAP_SHARED, fd, 0); > - if (mem == MAP_FAILED) { > + if (smem == MAP_FAILED) { > ksft_test_result_fail("mmap() failed\n"); > goto munmap; > } > @@ -1696,7 +1696,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, > fn(mem, smem, hugetlbsize); > munmap: > munmap(mem, hugetlbsize); > - if (mem != MAP_FAILED) > + if (smem != MAP_FAILED) > munmap(smem, hugetlbsize); > close: > close(fd);
Hi Anshuman Thanks for your revirew. On 2025/1/13 12:26, Anshuman Khandual wrote: > Hello Hao, > > On 1/13/25 08:58, Hao Ge wrote: >> From: Hao Ge <gehao@kylinos.cn> >> >> In the run_with_memfd_hugetlb function, some error handle >> have passed incorrect parameters. >> It should be "smem", but it was mistakenly written as "mem". > I guess there are couple of more instances where the returned address > 'smem' is not getting tested for MAP_FAILED. Hence the commit message > here needs to be bit more generic rather than run_with_memfd_hugetlb() > specific. Thank you for bringing it to my attention. Upon reviewing, I confirm that you are correct. > > --- a/tools/testing/selftests/mm/cow.c > +++ b/tools/testing/selftests/mm/cow.c > @@ -1482,7 +1482,7 @@ static void run_with_zeropage(non_anon_test_fn fn, const char *desc) > } > > smem = mmap(NULL, pagesize, PROT_READ, MAP_PRIVATE | MAP_ANON, -1, 0); > - if (mem == MAP_FAILED) { > + if (smem == MAP_FAILED) { > ksft_test_result_fail("mmap() failed\n"); > goto munmap; > } > @@ -1583,7 +1583,7 @@ static void run_with_memfd(non_anon_test_fn fn, const char *desc) > goto close; > } > smem = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); > - if (mem == MAP_FAILED) { > + if (smem == MAP_FAILED) { > ksft_test_result_fail("mmap() failed\n"); > goto munmap; > } > > >> Let's fix it. >> >> Fixes: baa489fabd01 ("selftests/vm: rename selftests/vm to selftests/mm") > This commit just renamed the directory from vm/ to mm/ directory. The following > commit introduced the problem instead. Please update the Fixes: tag as required. > > f8664f3c4a08f799 ("selftests/vm: cow: basic COW tests for non-anonymous pages") Yes, I accidentally copied the wrong information. I will resend a v2 to address these issues. Thank you very much Thanks Best Regards Hao >> Signed-off-by: Hao Ge <gehao@kylinos.cn> >> --- >> tools/testing/selftests/mm/cow.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c >> index 32c6ccc2a6be..7a89680d1566 100644 >> --- a/tools/testing/selftests/mm/cow.c >> +++ b/tools/testing/selftests/mm/cow.c >> @@ -1684,7 +1684,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, >> goto close; >> } >> smem = mmap(NULL, hugetlbsize, PROT_READ, MAP_SHARED, fd, 0); >> - if (mem == MAP_FAILED) { >> + if (smem == MAP_FAILED) { >> ksft_test_result_fail("mmap() failed\n"); >> goto munmap; >> } >> @@ -1696,7 +1696,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, >> fn(mem, smem, hugetlbsize); >> munmap: >> munmap(mem, hugetlbsize); >> - if (mem != MAP_FAILED) >> + if (smem != MAP_FAILED) >> munmap(smem, hugetlbsize); >> close: >> close(fd);
diff --git a/tools/testing/selftests/mm/cow.c b/tools/testing/selftests/mm/cow.c index 32c6ccc2a6be..7a89680d1566 100644 --- a/tools/testing/selftests/mm/cow.c +++ b/tools/testing/selftests/mm/cow.c @@ -1684,7 +1684,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, goto close; } smem = mmap(NULL, hugetlbsize, PROT_READ, MAP_SHARED, fd, 0); - if (mem == MAP_FAILED) { + if (smem == MAP_FAILED) { ksft_test_result_fail("mmap() failed\n"); goto munmap; } @@ -1696,7 +1696,7 @@ static void run_with_memfd_hugetlb(non_anon_test_fn fn, const char *desc, fn(mem, smem, hugetlbsize); munmap: munmap(mem, hugetlbsize); - if (mem != MAP_FAILED) + if (smem != MAP_FAILED) munmap(smem, hugetlbsize); close: close(fd);