2017-07-25 06:38:28

by Anshuman Khandual

[permalink] [raw]
Subject: [PATCH V2] selftests/vm: Add tests to validate mirror functionality with mremap

This adds two tests to validate mirror functionality with mremap()
system call on shared and private anon mappings.

Suggested-by: Mike Kravetz <[email protected]>
Signed-off-by: Anshuman Khandual <[email protected]>
---
Changes in V2:

- Added a test for private anon mappings
- Used sysconf(_SC_PAGESIZE) instead of hard coding page size
- Used MREMAP_MAYMOVE instead of hard coding the flag value 1

tools/testing/selftests/vm/Makefile | 2 +
.../selftests/vm/mremap_mirror_private_anon.c | 49 ++++++++++++++++++
.../selftests/vm/mremap_mirror_shared_anon.c | 58 ++++++++++++++++++++++
3 files changed, 109 insertions(+)
create mode 100644 tools/testing/selftests/vm/mremap_mirror_private_anon.c
create mode 100644 tools/testing/selftests/vm/mremap_mirror_shared_anon.c

diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index cbb29e4..6401f91 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -17,6 +17,8 @@ TEST_GEN_FILES += transhuge-stress
TEST_GEN_FILES += userfaultfd
TEST_GEN_FILES += mlock-random-test
TEST_GEN_FILES += virtual_address_range
+TEST_GEN_FILES += mremap_mirror_shared_anon
+TEST_GEN_FILES += mremap_mirror_private_anon

TEST_PROGS := run_vmtests

diff --git a/tools/testing/selftests/vm/mremap_mirror_private_anon.c b/tools/testing/selftests/vm/mremap_mirror_private_anon.c
new file mode 100644
index 0000000..3106809
--- /dev/null
+++ b/tools/testing/selftests/vm/mremap_mirror_private_anon.c
@@ -0,0 +1,49 @@
+/*
+ * Test to verify mirror functionality with mremap() system
+ * call for private anon mappings. The 'mirrored' buffer is
+ * a separate distinct unrelated mapping and different from
+ * that of the original one.
+ *
+ * Copyright (C) 2017 Anshuman Khandual, IBM Corporation
+ *
+ * Licensed under GPL V2
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+
+#define PATTERN 0xbe
+#define NR_PAGES 10
+
+int main(int argc, char *argv[])
+{
+ unsigned long alloc_size, i;
+ char *ptr, *mirror_ptr;
+
+ alloc_size = sysconf(_SC_PAGESIZE) * NR_PAGES;
+ ptr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (ptr == MAP_FAILED) {
+ perror("map() failed");
+ return -1;
+ }
+ memset(ptr, PATTERN, alloc_size);
+
+ mirror_ptr = (char *) mremap(ptr, 0, alloc_size, MREMAP_MAYMOVE);
+ if (mirror_ptr == MAP_FAILED) {
+ perror("mremap() failed");
+ return -1;
+ }
+
+ for (i = 0; i < alloc_size; i++) {
+ if (ptr[i] == mirror_ptr[i]) {
+ printf("Mirror buffer elements matched at %lu\n", i);
+ return 1;
+ }
+ }
+ return 0;
+}
diff --git a/tools/testing/selftests/vm/mremap_mirror_shared_anon.c b/tools/testing/selftests/vm/mremap_mirror_shared_anon.c
new file mode 100644
index 0000000..f775698
--- /dev/null
+++ b/tools/testing/selftests/vm/mremap_mirror_shared_anon.c
@@ -0,0 +1,58 @@
+/*
+ * Test to verify mirror functionality with mremap() system
+ * call for shared anon mappings. The 'mirrored' buffer will
+ * match element to element with that of the original one.
+ *
+ * Copyright (C) 2017 Anshuman Khandual, IBM Corporation
+ *
+ * Licensed under GPL V2
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+
+#define PATTERN 0xbe
+#define NR_PAGES 10
+
+int test_mirror(char *old, char *new, unsigned long size)
+{
+ unsigned long i;
+
+ for (i = 0; i < size; i++) {
+ if (new[i] != old[i]) {
+ printf("Mismatch at new[%lu] expected "
+ "%d received %d\n", i, old[i], new[i]);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ unsigned long alloc_size;
+ char *ptr, *mirror_ptr;
+
+ alloc_size = sysconf(_SC_PAGESIZE) * NR_PAGES;
+ ptr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ if (ptr == MAP_FAILED) {
+ perror("map() failed");
+ return -1;
+ }
+ memset(ptr, PATTERN, alloc_size);
+
+ mirror_ptr = (char *) mremap(ptr, 0, alloc_size, MREMAP_MAYMOVE);
+ if (mirror_ptr == MAP_FAILED) {
+ perror("mremap() failed");
+ return -1;
+ }
+
+ if (test_mirror(ptr, mirror_ptr, alloc_size))
+ return 1;
+ return 0;
+}
--
1.8.5.2


2017-07-25 13:36:08

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add tests to validate mirror functionality with mremap

On Tue 25-07-17 12:06:57, Anshuman Khandual wrote:
[...]
> diff --git a/tools/testing/selftests/vm/mremap_mirror_private_anon.c b/tools/testing/selftests/vm/mremap_mirror_private_anon.c
[...]
> + ptr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (ptr == MAP_FAILED) {
> + perror("map() failed");
> + return -1;
> + }
> + memset(ptr, PATTERN, alloc_size);
> +
> + mirror_ptr = (char *) mremap(ptr, 0, alloc_size, MREMAP_MAYMOVE);
> + if (mirror_ptr == MAP_FAILED) {
> + perror("mremap() failed");
> + return -1;
> + }

What is the point of this test? It will break with Mike's patch very
soon. Btw. it never worked.
--
Michal Hocko
SUSE Labs

2017-07-26 04:26:16

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add tests to validate mirror functionality with mremap

On 07/25/2017 07:06 PM, Michal Hocko wrote:
> On Tue 25-07-17 12:06:57, Anshuman Khandual wrote:
> [...]
>> diff --git a/tools/testing/selftests/vm/mremap_mirror_private_anon.c b/tools/testing/selftests/vm/mremap_mirror_private_anon.c
> [...]
>> + ptr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
>> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> + if (ptr == MAP_FAILED) {
>> + perror("map() failed");
>> + return -1;
>> + }
>> + memset(ptr, PATTERN, alloc_size);
>> +
>> + mirror_ptr = (char *) mremap(ptr, 0, alloc_size, MREMAP_MAYMOVE);
>> + if (mirror_ptr == MAP_FAILED) {
>> + perror("mremap() failed");
>> + return -1;
>> + }
>
> What is the point of this test? It will break with Mike's patch very
> soon. Btw. it never worked.

It works now. The new 'mirrored' buffer does not have same elements
as that of the original one. Yes, once Mike's patch is merged, it
will fail during the mremap() call itself IIUC. I can change this
test to verify that mremap() call failure then but now the mismatch
of elements between the buffers is what is expected and is happening.

But if you would like I can change the test to check for mremap()
failure in this case (which will fail in the current mainline but
will pass eventually when Mike's patch is in). Please do suggest.

2017-07-26 05:49:42

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH V2] selftests/vm: Add tests to validate mirror functionality with mremap

On Wed 26-07-17 09:54:26, Anshuman Khandual wrote:
> On 07/25/2017 07:06 PM, Michal Hocko wrote:
> > On Tue 25-07-17 12:06:57, Anshuman Khandual wrote:
> > [...]
> >> diff --git a/tools/testing/selftests/vm/mremap_mirror_private_anon.c b/tools/testing/selftests/vm/mremap_mirror_private_anon.c
> > [...]
> >> + ptr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
> >> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> >> + if (ptr == MAP_FAILED) {
> >> + perror("map() failed");
> >> + return -1;
> >> + }
> >> + memset(ptr, PATTERN, alloc_size);
> >> +
> >> + mirror_ptr = (char *) mremap(ptr, 0, alloc_size, MREMAP_MAYMOVE);
> >> + if (mirror_ptr == MAP_FAILED) {
> >> + perror("mremap() failed");
> >> + return -1;
> >> + }
> >
> > What is the point of this test? It will break with Mike's patch very
> > soon. Btw. it never worked.
>
> It works now. The new 'mirrored' buffer does not have same elements
> as that of the original one.

So what exactly are you testing here? The current implementation or the
semantic of mremap. Because having a different content after mremap
doesn't make _any_ sense to me. I might be misreading the intention of
the syscall but to me it sounds like the content should be same as the
original one... If my reading is wrong then -EINVAL for anon mremaps is
simply wrong.
--
Michal Hocko
SUSE Labs