On 02.01.23 16:49, Lorenzo Stoakes wrote:
On Mon, Jan 02, 2023 at 04:34:14PM +0100, David Hildenbrand wrote:
On 02.01.23 15:44, Lorenzo Stoakes wrote:
Add a test to assert that we can mremap() and expand a mapping starting
from an offset within an existing mapping. We unmap the last page in a 3
page mapping to ensure that the remap should always succeed, before
remapping from the 2nd page.
This is additionally a regression test for the issue solved in "mm, mremap:
fix mremap() expanding vma with addr inside vma" and confirmed to fail
prior to the change and pass after it.
Finally, this patch updates the existing mremap expand merge test to check
error conditions and reduce code duplication between the two tests.
Signed-off-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>
---
tools/testing/selftests/vm/mremap_test.c | 115 ++++++++++++++++++-----
1 file changed, 93 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
...
+
+ start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+ if (start == MAP_FAILED) {
+ ksft_print_msg("mmap failed: %s\n", strerror(errno));
I'd
ksft_test_result_fail(...)
return;
+ goto out;
+ }
+
+ munmap(start + page_size, page_size);
+ remap = mremap(start, page_size, 2 * page_size, 0);
+ if (remap == MAP_FAILED) {
+ ksft_print_msg("mremap failed: %s\n", strerror(errno));
+ munmap(start, page_size);
+ munmap(start + 2 * page_size, page_size);
+ goto out;
dito
ksft_test_result_fail(...)
...
return;
+ }
+
+ success = is_range_mapped(maps_fp, start, start + 3 * page_size);
+ munmap(start, 3 * page_size);
+
+out:
then you can drop the out label.
I have to disagree on this, to be consistent with the other tests the
failure messages should include the test name, and putting the
ksft_test_result_fail("test name") in each branch as well as the error
message would just be wilful duplication.
I do think it's a pity C lacks mechanisms such that gotos are sometimes
necessary, but I can only right so many wrongs in this patch :)
Let's agree to disagree ;) Too bad we don't have prefix push/pop
functionality as we have in other similar testing frameworks -- to avoid
exactly that duplication.
[...]
I'd simply use a global variable, same applies for page_size. But passing it
around is also ok.
I am trying to keep things consistent with what's gone before in this code,
and given page_size is being passed around I think the 'when in Rome'
principle applies equally to passing the fp around I think.
Other tests we have handle it "easier". :)
--
Thanks,
David / dhildenb
