* [PATCH] mm/selftest: Add MAP_POPULATE test
@ 2018-08-01 23:36 Dmitry Safonov
2018-08-02 0:02 ` Shuah Khan
2018-08-02 0:12 ` Andrew Morton
0 siblings, 2 replies; 5+ messages in thread
From: Dmitry Safonov @ 2018-08-01 23:36 UTC (permalink / raw)
To: linux-kernel
Cc: Dmitry Safonov, Andrew Morton, Dmitry Safonov, Hua Zhong,
Shuah Khan, Stuart Ritchie, linux-kselftest
As many other projects, we use some shmalloc allocator.
At some point we need to make a part of allocated pages back private to
process. And it should be populated straight away.
Check that (MAP_PRIVATE | MAP_POPULATE) actually copies the private page.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Hua Zhong <hzhong@arista.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Stuart Ritchie <sritchie@arista.com>
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
tools/testing/selftests/vm/.gitignore | 1 +
tools/testing/selftests/vm/Makefile | 1 +
tools/testing/selftests/vm/map_populate.c | 113 ++++++++++++++++++++++++++++++
tools/testing/selftests/vm/run_vmtests | 11 +++
4 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vm/map_populate.c
diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
index 342c7bc9dc8c..af5ff83f6d7f 100644
--- a/tools/testing/selftests/vm/.gitignore
+++ b/tools/testing/selftests/vm/.gitignore
@@ -1,6 +1,7 @@
hugepage-mmap
hugepage-shm
map_hugetlb
+map_populate
thuge-gen
compaction_test
mlock2-tests
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index fdefa2295ddc..9881876d2aa0 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -12,6 +12,7 @@ TEST_GEN_FILES += gup_benchmark
TEST_GEN_FILES += hugepage-mmap
TEST_GEN_FILES += hugepage-shm
TEST_GEN_FILES += map_hugetlb
+TEST_GEN_FILES += map_populate
TEST_GEN_FILES += mlock-random-test
TEST_GEN_FILES += mlock2-tests
TEST_GEN_FILES += on-fault-limit
diff --git a/tools/testing/selftests/vm/map_populate.c b/tools/testing/selftests/vm/map_populate.c
new file mode 100644
index 000000000000..df3eacc8eecb
--- /dev/null
+++ b/tools/testing/selftests/vm/map_populate.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Dmitry Safonov, Arista Networks
+ *
+ * MAP_POPULATE | MAP_PRIVATE should COW VMA pages.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#ifndef MMAP_SZ
+#define MMAP_SZ 4096
+#endif
+
+#define BUG_ON(condition, description) \
+ do { \
+ if (condition) { \
+ fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n", __func__, \
+ __LINE__, (description), strerror(errno)); \
+ exit(1); \
+ } \
+ } while (0)
+
+static int parent_f(int sock, unsigned long *smap, int child)
+{
+ int status, ret;
+
+ ret = read(sock, &status, sizeof(int));
+ BUG_ON(ret <= 0, "read(sock)");
+
+ *smap = 0x22222BAD;
+ ret = msync(smap, MMAP_SZ, MS_SYNC);
+ BUG_ON(ret, "msync()");
+
+ ret = write(sock, &status, sizeof(int));
+ BUG_ON(ret <= 0, "write(sock)");
+
+ waitpid(child, &status, 0);
+ BUG_ON(!WIFEXITED(status), "child hasn't suicide by own will");
+
+ return WEXITSTATUS(status);
+}
+
+static int child_f(int sock, unsigned long *smap, int fd)
+{
+ int ret, buf = 0;
+
+ smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_POPULATE, fd, 0);
+ BUG_ON(smap == MAP_FAILED, "mmap()");
+
+ BUG_ON(*smap != 0xdeadbabe, "MAP_PRIVATE | MAP_POPULATE changed file");
+
+ ret = write(sock, &buf, sizeof(int));
+ BUG_ON(ret <= 0, "write(sock)");
+
+ ret = read(sock, &buf, sizeof(int));
+ BUG_ON(ret <= 0, "read(sock)");
+
+ BUG_ON(*smap == 0x22222BAD, "MAP_POPULATE didn't COW private page");
+ BUG_ON(*smap != 0xdeadbabe, "mapping was corrupted");
+
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ int sock[2], child, ret;
+ FILE *ftmp;
+ unsigned long *smap;
+
+ ftmp = tmpfile();
+ BUG_ON(ftmp == 0, "tmpfile()");
+
+ ret = ftruncate(fileno(ftmp), MMAP_SZ);
+ BUG_ON(ret, "ftruncate()");
+
+ smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
+ MAP_SHARED, fileno(ftmp), 0);
+ BUG_ON(smap == MAP_FAILED, "mmap()");
+
+ *smap = 0xdeadbabe;
+ /* Probably unnecessary, but let it be. */
+ ret = msync(smap, MMAP_SZ, MS_SYNC);
+ BUG_ON(ret, "msync()");
+
+ ret = socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sock);
+ BUG_ON(ret, "socketpair()");
+
+ child = fork();
+ BUG_ON(child == -1, "fork()");
+
+ if (child) {
+ ret = close(sock[0]);
+ BUG_ON(ret, "close()");
+
+ return parent_f(sock[1], smap, child);
+ }
+
+ ret = close(sock[1]);
+ BUG_ON(ret, "close()");
+
+ return child_f(sock[0], smap, fileno(ftmp));
+}
diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtests
index 88cbe5575f0c..584a91ae4a8f 100755
--- a/tools/testing/selftests/vm/run_vmtests
+++ b/tools/testing/selftests/vm/run_vmtests
@@ -168,6 +168,17 @@ else
fi
echo "--------------------"
+echo "running map_populate"
+echo "--------------------"
+./map_populate
+if [ $? -ne 0 ]; then
+ echo "[FAIL]"
+ exitcode=1
+else
+ echo "[PASS]"
+fi
+
+echo "--------------------"
echo "running mlock2-tests"
echo "--------------------"
./mlock2-tests
--
2.13.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/selftest: Add MAP_POPULATE test
2018-08-01 23:36 [PATCH] mm/selftest: Add MAP_POPULATE test Dmitry Safonov
@ 2018-08-02 0:02 ` Shuah Khan
2018-08-02 0:24 ` Dmitry Safonov
2018-08-02 0:12 ` Andrew Morton
1 sibling, 1 reply; 5+ messages in thread
From: Shuah Khan @ 2018-08-02 0:02 UTC (permalink / raw)
To: Dmitry Safonov, linux-kernel
Cc: Andrew Morton, Dmitry Safonov, Hua Zhong, Stuart Ritchie,
linux-kselftest, Shuah Khan
Hi Dmitry,
Thanks for the test. A few comments below.
On 08/01/2018 05:36 PM, Dmitry Safonov wrote:
> As many other projects, we use some shmalloc allocator.
> At some point we need to make a part of allocated pages back private to
> process. And it should be populated straight away.
> Check that (MAP_PRIVATE | MAP_POPULATE) actually copies the private page.
It is not clear from this commit log what this test is doing? It would
be helpful to rephrase.
"MAP_POPULATE | MAP_PRIVATE memory maps should COW VMA pages. This test is
for verifying that it does" something along the lines??
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dmitry Safonov <0x7f454c46@gmail.com>
> Cc: Hua Zhong <hzhong@arista.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Stuart Ritchie <sritchie@arista.com>
> Cc: linux-kselftest@vger.kernel.org
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
> tools/testing/selftests/vm/.gitignore | 1 +
> tools/testing/selftests/vm/Makefile | 1 +
> tools/testing/selftests/vm/map_populate.c | 113 ++++++++++++++++++++++++++++++
> tools/testing/selftests/vm/run_vmtests | 11 +++
> 4 files changed, 126 insertions(+)
> create mode 100644 tools/testing/selftests/vm/map_populate.c
>
> diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
> index 342c7bc9dc8c..af5ff83f6d7f 100644
> --- a/tools/testing/selftests/vm/.gitignore
> +++ b/tools/testing/selftests/vm/.gitignore
> @@ -1,6 +1,7 @@
> hugepage-mmap
> hugepage-shm
> map_hugetlb
> +map_populate
> thuge-gen
> compaction_test
> mlock2-tests
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index fdefa2295ddc..9881876d2aa0 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -12,6 +12,7 @@ TEST_GEN_FILES += gup_benchmark
> TEST_GEN_FILES += hugepage-mmap
> TEST_GEN_FILES += hugepage-shm
> TEST_GEN_FILES += map_hugetlb
> +TEST_GEN_FILES += map_populate
> TEST_GEN_FILES += mlock-random-test
> TEST_GEN_FILES += mlock2-tests
> TEST_GEN_FILES += on-fault-limit
> diff --git a/tools/testing/selftests/vm/map_populate.c b/tools/testing/selftests/vm/map_populate.c
> new file mode 100644
> index 000000000000..df3eacc8eecb
> --- /dev/null
> +++ b/tools/testing/selftests/vm/map_populate.c
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Dmitry Safonov, Arista Networks
> + *
> + * MAP_POPULATE | MAP_PRIVATE should COW VMA pages.
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#ifndef MMAP_SZ
> +#define MMAP_SZ 4096
> +#endif
> +
> +#define BUG_ON(condition, description) \
> + do { \
> + if (condition) { \
> + fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n", __func__, \
> + __LINE__, (description), strerror(errno)); \
> + exit(1); \
> + } \
> + } while (0)
> +
> +static int parent_f(int sock, unsigned long *smap, int child)
> +{
> + int status, ret;
> +
> + ret = read(sock, &status, sizeof(int));
> + BUG_ON(ret <= 0, "read(sock)");
> +
> + *smap = 0x22222BAD;
> + ret = msync(smap, MMAP_SZ, MS_SYNC);
> + BUG_ON(ret, "msync()");
> +
> + ret = write(sock, &status, sizeof(int));
> + BUG_ON(ret <= 0, "write(sock)");
> +
> + waitpid(child, &status, 0);
> + BUG_ON(!WIFEXITED(status), "child hasn't suicide by own will");
What does this mean? Could you rephrase this please?
> +
> + return WEXITSTATUS(status);
> +}
> +
> +static int child_f(int sock, unsigned long *smap, int fd)
> +{
> + int ret, buf = 0;
> +
> + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_POPULATE, fd, 0);
> + BUG_ON(smap == MAP_FAILED, "mmap()");
> +
> + BUG_ON(*smap != 0xdeadbabe, "MAP_PRIVATE | MAP_POPULATE changed file");
What is expected here? What is expected?
> +
> + ret = write(sock, &buf, sizeof(int));
> + BUG_ON(ret <= 0, "write(sock)");
> +
> + ret = read(sock, &buf, sizeof(int));
> + BUG_ON(ret <= 0, "read(sock)");
> +
> + BUG_ON(*smap == 0x22222BAD, "MAP_POPULATE didn't COW private page");
> + BUG_ON(*smap != 0xdeadbabe, "mapping was corrupted");
> +
> + return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int sock[2], child, ret;
> + FILE *ftmp;
> + unsigned long *smap;
> +
> + ftmp = tmpfile();
> + BUG_ON(ftmp == 0, "tmpfile()");
> +
> + ret = ftruncate(fileno(ftmp), MMAP_SZ);
> + BUG_ON(ret, "ftruncate()");
> +
> + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> + MAP_SHARED, fileno(ftmp), 0);
> + BUG_ON(smap == MAP_FAILED, "mmap()");
> +
> + *smap = 0xdeadbabe;
> + /* Probably unnecessary, but let it be. */
> + ret = msync(smap, MMAP_SZ, MS_SYNC);
> + BUG_ON(ret, "msync()");
> +
> + ret = socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sock);
> + BUG_ON(ret, "socketpair()");
> +
> + child = fork();
> + BUG_ON(child == -1, "fork()");
> +
> + if (child) {
> + ret = close(sock[0]);
> + BUG_ON(ret, "close()");
> +
> + return parent_f(sock[1], smap, child);
> + }
> +
> + ret = close(sock[1]);
> + BUG_ON(ret, "close()");
> +
> + return child_f(sock[0], smap, fileno(ftmp));
> +}
> diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtests
> index 88cbe5575f0c..584a91ae4a8f 100755
> --- a/tools/testing/selftests/vm/run_vmtests
> +++ b/tools/testing/selftests/vm/run_vmtests
> @@ -168,6 +168,17 @@ else
> fi
>
> echo "--------------------"
> +echo "running map_populate"
> +echo "--------------------"
> +./map_populate
> +if [ $? -ne 0 ]; then
> + echo "[FAIL]"
> + exitcode=1
> +else
> + echo "[PASS]"
> +fi
> +
> +echo "--------------------"
> echo "running mlock2-tests"
> echo "--------------------"
> ./mlock2-tests
>
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/selftest: Add MAP_POPULATE test
2018-08-01 23:36 [PATCH] mm/selftest: Add MAP_POPULATE test Dmitry Safonov
2018-08-02 0:02 ` Shuah Khan
@ 2018-08-02 0:12 ` Andrew Morton
2018-08-02 0:29 ` Dmitry Safonov
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2018-08-02 0:12 UTC (permalink / raw)
To: Dmitry Safonov
Cc: linux-kernel, Dmitry Safonov, Hua Zhong, Shuah Khan,
Stuart Ritchie, linux-kselftest
On Thu, 2 Aug 2018 00:36:36 +0100 Dmitry Safonov <dima@arista.com> wrote:
> As many other projects, we use some shmalloc allocator.
> At some point we need to make a part of allocated pages back private to
> process. And it should be populated straight away.
> Check that (MAP_PRIVATE | MAP_POPULATE) actually copies the private page.
>
> ...
>
> --- /dev/null
> +++ b/tools/testing/selftests/vm/map_populate.c
>
> ...
>
> +#define BUG_ON(condition, description) \
> + do { \
> + if (condition) { \
> + fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n", __func__, \
> + __LINE__, (description), strerror(errno)); \
> + exit(1); \
> + } \
> + } while (0)
This is userspace. Why not use assert()?
>
> ...
>
> +int main(int argc, char **argv)
> +{
> + int sock[2], child, ret;
> + FILE *ftmp;
> + unsigned long *smap;
> +
> + ftmp = tmpfile();
Seems odd to putz around with stdio when you just want the fd.
mkstemp(), maybe?
> + BUG_ON(ftmp == 0, "tmpfile()");
> +
> + ret = ftruncate(fileno(ftmp), MMAP_SZ);
> + BUG_ON(ret, "ftruncate()");
> +
> + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> + MAP_SHARED, fileno(ftmp), 0);
> + BUG_ON(smap == MAP_FAILED, "mmap()");
> +
> + *smap = 0xdeadbabe;
> + /* Probably unnecessary, but let it be. */
> + ret = msync(smap, MMAP_SZ, MS_SYNC);
> + BUG_ON(ret, "msync()");
> +
> + ret = socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sock);
> + BUG_ON(ret, "socketpair()");
> +
> + child = fork();
> + BUG_ON(child == -1, "fork()");
> +
> + if (child) {
> + ret = close(sock[0]);
> + BUG_ON(ret, "close()");
> +
> + return parent_f(sock[1], smap, child);
> + }
> +
> + ret = close(sock[1]);
> + BUG_ON(ret, "close()");
> +
> + return child_f(sock[0], smap, fileno(ftmp));
> +}
>
> ...
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/selftest: Add MAP_POPULATE test
2018-08-02 0:02 ` Shuah Khan
@ 2018-08-02 0:24 ` Dmitry Safonov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Safonov @ 2018-08-02 0:24 UTC (permalink / raw)
To: Shuah Khan, linux-kernel
Cc: Andrew Morton, Dmitry Safonov, Hua Zhong, Stuart Ritchie,
linux-kselftest
On Wed, 2018-08-01 at 18:02 -0600, Shuah Khan wrote:
> Hi Dmitry,
Hi Shuah,
Thanks for the review,
>
> Thanks for the test. A few comments below.
>
> On 08/01/2018 05:36 PM, Dmitry Safonov wrote:
> > As many other projects, we use some shmalloc allocator.
> > At some point we need to make a part of allocated pages back
> > private to
> > process. And it should be populated straight away.
> > Check that (MAP_PRIVATE | MAP_POPULATE) actually copies the private
> > page.
>
> It is not clear from this commit log what this test is doing? It
> would
> be helpful to rephrase.
>
> "MAP_POPULATE | MAP_PRIVATE memory maps should COW VMA pages. This
> test is
> for verifying that it does" something along the lines??
Uhm, no. COW VMA is just MAP_PRIVATE.
IOW, if you do mmap(MAP_PRIVATE), you need to actually "touch" (call
write() on) the pages to get a copy. Which might be not handy if one
has hundreds of pages in a VMA.
But mmap(MAP_PRIVATE | MAP_POPULATE) should return mapping with already
pre-copied pages, not the COW.
>
> >
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Dmitry Safonov <0x7f454c46@gmail.com>
> > Cc: Hua Zhong <hzhong@arista.com>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Cc: Stuart Ritchie <sritchie@arista.com>
> > Cc: linux-kselftest@vger.kernel.org
> > Signed-off-by: Dmitry Safonov <dima@arista.com>
> > ---
> > tools/testing/selftests/vm/.gitignore | 1 +
> > tools/testing/selftests/vm/Makefile | 1 +
> > tools/testing/selftests/vm/map_populate.c | 113
> > ++++++++++++++++++++++++++++++
> > tools/testing/selftests/vm/run_vmtests | 11 +++
> > 4 files changed, 126 insertions(+)
> > create mode 100644 tools/testing/selftests/vm/map_populate.c
> >
> > diff --git a/tools/testing/selftests/vm/.gitignore
> > b/tools/testing/selftests/vm/.gitignore
> > index 342c7bc9dc8c..af5ff83f6d7f 100644
> > --- a/tools/testing/selftests/vm/.gitignore
> > +++ b/tools/testing/selftests/vm/.gitignore
> > @@ -1,6 +1,7 @@
> > hugepage-mmap
> > hugepage-shm
> > map_hugetlb
> > +map_populate
> > thuge-gen
> > compaction_test
> > mlock2-tests
> > diff --git a/tools/testing/selftests/vm/Makefile
> > b/tools/testing/selftests/vm/Makefile
> > index fdefa2295ddc..9881876d2aa0 100644
> > --- a/tools/testing/selftests/vm/Makefile
> > +++ b/tools/testing/selftests/vm/Makefile
> > @@ -12,6 +12,7 @@ TEST_GEN_FILES += gup_benchmark
> > TEST_GEN_FILES += hugepage-mmap
> > TEST_GEN_FILES += hugepage-shm
> > TEST_GEN_FILES += map_hugetlb
> > +TEST_GEN_FILES += map_populate
> > TEST_GEN_FILES += mlock-random-test
> > TEST_GEN_FILES += mlock2-tests
> > TEST_GEN_FILES += on-fault-limit
> > diff --git a/tools/testing/selftests/vm/map_populate.c
> > b/tools/testing/selftests/vm/map_populate.c
> > new file mode 100644
> > index 000000000000..df3eacc8eecb
> > --- /dev/null
> > +++ b/tools/testing/selftests/vm/map_populate.c
> > @@ -0,0 +1,113 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Dmitry Safonov, Arista Networks
> > + *
> > + * MAP_POPULATE | MAP_PRIVATE should COW VMA pages.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <sys/mman.h>
> > +#include <sys/socket.h>
> > +#include <sys/types.h>
> > +#include <sys/wait.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +
> > +#ifndef MMAP_SZ
> > +#define MMAP_SZ 4096
> > +#endif
> > +
> > +#define BUG_ON(condition, description)
> > \
> > + do {
> > \
> > + if (condition) {
> > \
> > + fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n",
> > __func__, \
> > + __LINE__, (description),
> > strerror(errno)); \
> > + exit(1);
> > \
> > + }
> > \
> > + } while (0)
> > +
> > +static int parent_f(int sock, unsigned long *smap, int child)
> > +{
> > + int status, ret;
> > +
> > + ret = read(sock, &status, sizeof(int));
> > + BUG_ON(ret <= 0, "read(sock)");
> > +
> > + *smap = 0x22222BAD;
> > + ret = msync(smap, MMAP_SZ, MS_SYNC);
> > + BUG_ON(ret, "msync()");
> > +
> > + ret = write(sock, &status, sizeof(int));
> > + BUG_ON(ret <= 0, "write(sock)");
> > +
> > + waitpid(child, &status, 0);
> > + BUG_ON(!WIFEXITED(status), "child hasn't suicide by own
> > will");
>
> What does this mean? Could you rephrase this please?
Sure, I didn't want to write "child hasn't exited" because it sounds
like it still alive.. I can change it to "child in unexpected state"
if that sounds any better?
Or I can check WIFSIGNALED(), WIFSIGNALED(), WIFSTOPPED() too and write
what happened to the child, but it will needlessly blow the code.
>
> > +
> > + return WEXITSTATUS(status);
> > +}
> > +
> > +static int child_f(int sock, unsigned long *smap, int fd)
> > +{
> > + int ret, buf = 0;
> > +
> > + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> > + MAP_PRIVATE | MAP_POPULATE, fd, 0);
> > + BUG_ON(smap == MAP_FAILED, "mmap()");
> > +
> > + BUG_ON(*smap != 0xdeadbabe, "MAP_PRIVATE | MAP_POPULATE
> > changed file");
>
> What is expected here? What is expected?
It's expected that the value in file is the same as before remapping it
MAP_PRIVATE (same as parent set initially).
>
> > +
> > + ret = write(sock, &buf, sizeof(int));
> > + BUG_ON(ret <= 0, "write(sock)");
> > +
> > + ret = read(sock, &buf, sizeof(int));
> > + BUG_ON(ret <= 0, "read(sock)");
> > +
> > + BUG_ON(*smap == 0x22222BAD, "MAP_POPULATE didn't COW
> > private page");
> > + BUG_ON(*smap != 0xdeadbabe, "mapping was corrupted");
> > +
> > + return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + int sock[2], child, ret;
> > + FILE *ftmp;
> > + unsigned long *smap;
> > +
> > + ftmp = tmpfile();
> > + BUG_ON(ftmp == 0, "tmpfile()");
> > +
> > + ret = ftruncate(fileno(ftmp), MMAP_SZ);
> > + BUG_ON(ret, "ftruncate()");
> > +
> > + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> > + MAP_SHARED, fileno(ftmp), 0);
> > + BUG_ON(smap == MAP_FAILED, "mmap()");
> > +
> > + *smap = 0xdeadbabe;
> > + /* Probably unnecessary, but let it be. */
> > + ret = msync(smap, MMAP_SZ, MS_SYNC);
> > + BUG_ON(ret, "msync()");
> > +
> > + ret = socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sock);
> > + BUG_ON(ret, "socketpair()");
> > +
> > + child = fork();
> > + BUG_ON(child == -1, "fork()");
> > +
> > + if (child) {
> > + ret = close(sock[0]);
> > + BUG_ON(ret, "close()");
> > +
> > + return parent_f(sock[1], smap, child);
> > + }
> > +
> > + ret = close(sock[1]);
> > + BUG_ON(ret, "close()");
> > +
> > + return child_f(sock[0], smap, fileno(ftmp));
> > +}
> > diff --git a/tools/testing/selftests/vm/run_vmtests
> > b/tools/testing/selftests/vm/run_vmtests
> > index 88cbe5575f0c..584a91ae4a8f 100755
> > --- a/tools/testing/selftests/vm/run_vmtests
> > +++ b/tools/testing/selftests/vm/run_vmtests
> > @@ -168,6 +168,17 @@ else
> > fi
> >
> > echo "--------------------"
> > +echo "running map_populate"
> > +echo "--------------------"
> > +./map_populate
> > +if [ $? -ne 0 ]; then
> > + echo "[FAIL]"
> > + exitcode=1
> > +else
> > + echo "[PASS]"
> > +fi
> > +
> > +echo "--------------------"
> > echo "running mlock2-tests"
> > echo "--------------------"
> > ./mlock2-tests
> >
>
> thanks,
> -- Shuah
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/selftest: Add MAP_POPULATE test
2018-08-02 0:12 ` Andrew Morton
@ 2018-08-02 0:29 ` Dmitry Safonov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Safonov @ 2018-08-02 0:29 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Dmitry Safonov, Hua Zhong, Shuah Khan,
Stuart Ritchie, linux-kselftest
On Wed, 2018-08-01 at 17:12 -0700, Andrew Morton wrote:
> On Thu, 2 Aug 2018 00:36:36 +0100 Dmitry Safonov <dima@arista.com>
> wrote:
>
> > As many other projects, we use some shmalloc allocator.
> > At some point we need to make a part of allocated pages back
> > private to
> > process. And it should be populated straight away.
> > Check that (MAP_PRIVATE | MAP_POPULATE) actually copies the private
> > page.
> >
> > ...
> >
> > --- /dev/null
> > +++ b/tools/testing/selftests/vm/map_populate.c
> >
> > ...
> >
> > +#define BUG_ON(condition, description)
> > \
> > + do {
> > \
> > + if (condition) {
> > \
> > + fprintf(stderr, "[FAIL]\t%s:%d\t%s:%s\n",
> > __func__, \
> > + __LINE__, (description),
> > strerror(errno)); \
> > + exit(1);
> > \
> > + }
> > \
> > + } while (0)
>
> This is userspace. Why not use assert()?
I wanted to print errno too. Of cause it's possible to get it from
coredump - but I think it's easier if you get buildbot reply to see
what the error is from the log.
>
> >
> > ...
> >
> > +int main(int argc, char **argv)
> > +{
> > + int sock[2], child, ret;
> > + FILE *ftmp;
> > + unsigned long *smap;
> > +
> > + ftmp = tmpfile();
>
> Seems odd to putz around with stdio when you just want the fd.
> mkstemp(), maybe?
It needs a template and unlink() call.. I can do it.
I initially thought about memfd_create(), can change it that way.
The only downside is that it will be syscall(NR_memfd_create,...)
>
> > + BUG_ON(ftmp == 0, "tmpfile()");
> > +
> > + ret = ftruncate(fileno(ftmp), MMAP_SZ);
> > + BUG_ON(ret, "ftruncate()");
> > +
> > + smap = mmap(0, MMAP_SZ, PROT_READ | PROT_WRITE,
> > + MAP_SHARED, fileno(ftmp), 0);
> > + BUG_ON(smap == MAP_FAILED, "mmap()");
> > +
> > + *smap = 0xdeadbabe;
> > + /* Probably unnecessary, but let it be. */
> > + ret = msync(smap, MMAP_SZ, MS_SYNC);
> > + BUG_ON(ret, "msync()");
> > +
> > + ret = socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sock);
> > + BUG_ON(ret, "socketpair()");
> > +
> > + child = fork();
> > + BUG_ON(child == -1, "fork()");
> > +
> > + if (child) {
> > + ret = close(sock[0]);
> > + BUG_ON(ret, "close()");
> > +
> > + return parent_f(sock[1], smap, child);
> > + }
> > +
> > + ret = close(sock[1]);
> > + BUG_ON(ret, "close()");
> > +
> > + return child_f(sock[0], smap, fileno(ftmp));
> > +}
> >
> > ...
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-02 0:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-01 23:36 [PATCH] mm/selftest: Add MAP_POPULATE test Dmitry Safonov
2018-08-02 0:02 ` Shuah Khan
2018-08-02 0:24 ` Dmitry Safonov
2018-08-02 0:12 ` Andrew Morton
2018-08-02 0:29 ` Dmitry Safonov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox