[PATCH 0/3] userfaultfd: selftests: cleanups and trivial fixes

[PATCH 0/3] userfaultfd: selftests: cleanups and trivial fixes

[peterx]

Recently I wrote some uffd write-protection test for the
not-yet-published uffd-wp tree, and I picked these common patches out
first for the selftest which even suite for master.

Any feedback is welcomed.  Please have a look, thanks.

Peter Xu (3):
  userfaultfd: selftest: cleanup help messages
  userfaultfd: selftest: generalize read and poll
  userfaultfd: selftest: recycle lock threads first

 tools/testing/selftests/vm/userfaultfd.c | 131 +++++++++++++----------
 1 file changed, 74 insertions(+), 57 deletions(-)

-- 
2.17.1

[PATCH 0/3] userfaultfd: selftests: cleanups and trivial fixes

[Peter Xu]

Recently I wrote some uffd write-protection test for the
not-yet-published uffd-wp tree, and I picked these common patches out
first for the selftest which even suite for master.

Any feedback is welcomed.  Please have a look, thanks.

Peter Xu (3):
  userfaultfd: selftest: cleanup help messages
  userfaultfd: selftest: generalize read and poll
  userfaultfd: selftest: recycle lock threads first

 tools/testing/selftests/vm/userfaultfd.c | 131 +++++++++++++----------
 1 file changed, 74 insertions(+), 57 deletions(-)

-- 
2.17.1

[PATCH 0/3] userfaultfd: selftests: cleanups and trivial fixes

[Peter Xu]

Recently I wrote some uffd write-protection test for the
not-yet-published uffd-wp tree, and I picked these common patches out
first for the selftest which even suite for master.

Any feedback is welcomed.  Please have a look, thanks.

Peter Xu (3):
  userfaultfd: selftest: cleanup help messages
  userfaultfd: selftest: generalize read and poll
  userfaultfd: selftest: recycle lock threads first

 tools/testing/selftests/vm/userfaultfd.c | 131 +++++++++++++----------
 1 file changed, 74 insertions(+), 57 deletions(-)

-- 
2.17.1

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[peterx]

Firstly, the help in the comment region is obsolete, now we support
three parameters.  Since at it, change it and move it into the help
message of the program.

Also, the help messages dumped here and there is obsolete too.  Use a
single usage() helper.

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 7b8171e3128a..2a84adaf8cf8 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -34,18 +34,6 @@
  * per-CPU threads 1 by triggering userfaults inside
  * pthread_mutex_lock will also verify the atomicity of the memory
  * transfer (UFFDIO_COPY).
- *
- * The program takes two parameters: the amounts of physical memory in
- * megabytes (MiB) of the area and the number of bounces to execute.
- *
- * # 100MiB 99999 bounces
- * ./userfaultfd 100 99999
- *
- * # 1GiB 99 bounces
- * ./userfaultfd 1000 99
- *
- * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
- * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
  */
 
 #define _GNU_SOURCE
@@ -115,6 +103,28 @@ pthread_attr_t attr;
 				 ~(unsigned long)(sizeof(unsigned long long) \
 						  -  1)))
 
+const char *examples =
+    "# 100MiB 99999 bounces\n"
+    "./userfaultfd anon 100 99999\n"
+    "\n"
+    "# 1GiB 99 bounces\n"
+    "./userfaultfd anon 1000 99\n"
+    "\n"
+    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
+    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
+    "\n";
+
+static void usage(void)
+{
+	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
+		"[hugetlbfs_file]\n\n");
+	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
+		"hugetlb_shared, shmem\n\n");
+	fprintf(stderr, "Examples:\n\n");
+	fprintf(stderr, examples);
+	exit(1);
+}
+
 static int anon_release_pages(char *rel_area)
 {
 	int ret = 0;
@@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
 int main(int argc, char **argv)
 {
 	if (argc < 4)
-		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
-				exit(1);
+		usage();
 
 	if (signal(SIGALRM, sigalrm) == SIG_ERR)
 		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
@@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
 		nr_cpus;
 	if (!nr_pages_per_cpu) {
 		fprintf(stderr, "invalid MiB\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 
 	bounces = atoi(argv[3]);
 	if (bounces <= 0) {
 		fprintf(stderr, "invalid bounces\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 	nr_pages = nr_pages_per_cpu * nr_cpus;
 
 	if (test_type == TEST_HUGETLB) {
 		if (argc < 5)
-			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
-				exit(1);
+			usage();
 		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
 		if (huge_fd < 0) {
 			fprintf(stderr, "Open of %s failed", argv[3]);
-- 
2.17.1

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Peter Xu]

Firstly, the help in the comment region is obsolete, now we support
three parameters.  Since at it, change it and move it into the help
message of the program.

Also, the help messages dumped here and there is obsolete too.  Use a
single usage() helper.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 7b8171e3128a..2a84adaf8cf8 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -34,18 +34,6 @@
  * per-CPU threads 1 by triggering userfaults inside
  * pthread_mutex_lock will also verify the atomicity of the memory
  * transfer (UFFDIO_COPY).
- *
- * The program takes two parameters: the amounts of physical memory in
- * megabytes (MiB) of the area and the number of bounces to execute.
- *
- * # 100MiB 99999 bounces
- * ./userfaultfd 100 99999
- *
- * # 1GiB 99 bounces
- * ./userfaultfd 1000 99
- *
- * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
- * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
  */
 
 #define _GNU_SOURCE
@@ -115,6 +103,28 @@ pthread_attr_t attr;
 				 ~(unsigned long)(sizeof(unsigned long long) \
 						  -  1)))
 
+const char *examples =
+    "# 100MiB 99999 bounces\n"
+    "./userfaultfd anon 100 99999\n"
+    "\n"
+    "# 1GiB 99 bounces\n"
+    "./userfaultfd anon 1000 99\n"
+    "\n"
+    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
+    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
+    "\n";
+
+static void usage(void)
+{
+	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
+		"[hugetlbfs_file]\n\n");
+	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
+		"hugetlb_shared, shmem\n\n");
+	fprintf(stderr, "Examples:\n\n");
+	fprintf(stderr, examples);
+	exit(1);
+}
+
 static int anon_release_pages(char *rel_area)
 {
 	int ret = 0;
@@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
 int main(int argc, char **argv)
 {
 	if (argc < 4)
-		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
-				exit(1);
+		usage();
 
 	if (signal(SIGALRM, sigalrm) == SIG_ERR)
 		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
@@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
 		nr_cpus;
 	if (!nr_pages_per_cpu) {
 		fprintf(stderr, "invalid MiB\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 
 	bounces = atoi(argv[3]);
 	if (bounces <= 0) {
 		fprintf(stderr, "invalid bounces\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 	nr_pages = nr_pages_per_cpu * nr_cpus;
 
 	if (test_type == TEST_HUGETLB) {
 		if (argc < 5)
-			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
-				exit(1);
+			usage();
 		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
 		if (huge_fd < 0) {
 			fprintf(stderr, "Open of %s failed", argv[3]);
-- 
2.17.1

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Peter Xu]

Firstly, the help in the comment region is obsolete, now we support
three parameters.  Since at it, change it and move it into the help
message of the program.

Also, the help messages dumped here and there is obsolete too.  Use a
single usage() helper.

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 7b8171e3128a..2a84adaf8cf8 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -34,18 +34,6 @@
  * per-CPU threads 1 by triggering userfaults inside
  * pthread_mutex_lock will also verify the atomicity of the memory
  * transfer (UFFDIO_COPY).
- *
- * The program takes two parameters: the amounts of physical memory in
- * megabytes (MiB) of the area and the number of bounces to execute.
- *
- * # 100MiB 99999 bounces
- * ./userfaultfd 100 99999
- *
- * # 1GiB 99 bounces
- * ./userfaultfd 1000 99
- *
- * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
- * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
  */
 
 #define _GNU_SOURCE
@@ -115,6 +103,28 @@ pthread_attr_t attr;
 				 ~(unsigned long)(sizeof(unsigned long long) \
 						  -  1)))
 
+const char *examples =
+    "# 100MiB 99999 bounces\n"
+    "./userfaultfd anon 100 99999\n"
+    "\n"
+    "# 1GiB 99 bounces\n"
+    "./userfaultfd anon 1000 99\n"
+    "\n"
+    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
+    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
+    "\n";
+
+static void usage(void)
+{
+	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
+		"[hugetlbfs_file]\n\n");
+	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
+		"hugetlb_shared, shmem\n\n");
+	fprintf(stderr, "Examples:\n\n");
+	fprintf(stderr, examples);
+	exit(1);
+}
+
 static int anon_release_pages(char *rel_area)
 {
 	int ret = 0;
@@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
 int main(int argc, char **argv)
 {
 	if (argc < 4)
-		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
-				exit(1);
+		usage();
 
 	if (signal(SIGALRM, sigalrm) == SIG_ERR)
 		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
@@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
 		nr_cpus;
 	if (!nr_pages_per_cpu) {
 		fprintf(stderr, "invalid MiB\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 
 	bounces = atoi(argv[3]);
 	if (bounces <= 0) {
 		fprintf(stderr, "invalid bounces\n");
-		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
+		usage();
 	}
 	nr_pages = nr_pages_per_cpu * nr_cpus;
 
 	if (test_type == TEST_HUGETLB) {
 		if (argc < 5)
-			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
-				exit(1);
+			usage();
 		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
 		if (huge_fd < 0) {
 			fprintf(stderr, "Open of %s failed", argv[3]);
-- 
2.17.1

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[rppt]

On Sat, Sep 29, 2018 at 04:43:09PM +0800, Peter Xu wrote:
> Firstly, the help in the comment region is obsolete, now we support
> three parameters.  Since at it, change it and move it into the help
> message of the program.
> 
> Also, the help messages dumped here and there is obsolete too.  Use a
> single usage() helper.
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>

Small comment below, otherwise

Acked-by: Mike Rapoport <rppt at linux.vnet.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 7b8171e3128a..2a84adaf8cf8 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -34,18 +34,6 @@
>   * per-CPU threads 1 by triggering userfaults inside
>   * pthread_mutex_lock will also verify the atomicity of the memory
>   * transfer (UFFDIO_COPY).
> - *
> - * The program takes two parameters: the amounts of physical memory in
> - * megabytes (MiB) of the area and the number of bounces to execute.
> - *
> - * # 100MiB 99999 bounces
> - * ./userfaultfd 100 99999
> - *
> - * # 1GiB 99 bounces
> - * ./userfaultfd 1000 99
> - *
> - * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
> - * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
>   */
> 
>  #define _GNU_SOURCE
> @@ -115,6 +103,28 @@ pthread_attr_t attr;
>  				 ~(unsigned long)(sizeof(unsigned long long) \
>  						  -  1)))
> 
> +const char *examples =
> +    "# 100MiB 99999 bounces\n"
> +    "./userfaultfd anon 100 99999\n"
> +    "\n"
> +    "# 1GiB 99 bounces\n"
> +    "./userfaultfd anon 1000 99\n"
> +    "\n"
> +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> +    "\n";

While at it, can you please update the examples to include other test
types?

> +
> +static void usage(void)
> +{
> +	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
> +		"[hugetlbfs_file]\n\n");
> +	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
> +		"hugetlb_shared, shmem\n\n");
> +	fprintf(stderr, "Examples:\n\n");
> +	fprintf(stderr, examples);
> +	exit(1);
> +}
> +
>  static int anon_release_pages(char *rel_area)
>  {
>  	int ret = 0;
> @@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
>  int main(int argc, char **argv)
>  {
>  	if (argc < 4)
> -		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
> -				exit(1);
> +		usage();
> 
>  	if (signal(SIGALRM, sigalrm) == SIG_ERR)
>  		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
> @@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
>  		nr_cpus;
>  	if (!nr_pages_per_cpu) {
>  		fprintf(stderr, "invalid MiB\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
> 
>  	bounces = atoi(argv[3]);
>  	if (bounces <= 0) {
>  		fprintf(stderr, "invalid bounces\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
>  	nr_pages = nr_pages_per_cpu * nr_cpus;
> 
>  	if (test_type == TEST_HUGETLB) {
>  		if (argc < 5)
> -			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
> -				exit(1);
> +			usage();
>  		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
>  		if (huge_fd < 0) {
>  			fprintf(stderr, "Open of %s failed", argv[3]);
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Mike Rapoport]

On Sat, Sep 29, 2018 at 04:43:09PM +0800, Peter Xu wrote:
> Firstly, the help in the comment region is obsolete, now we support
> three parameters.  Since at it, change it and move it into the help
> message of the program.
> 
> Also, the help messages dumped here and there is obsolete too.  Use a
> single usage() helper.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Small comment below, otherwise

Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 7b8171e3128a..2a84adaf8cf8 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -34,18 +34,6 @@
>   * per-CPU threads 1 by triggering userfaults inside
>   * pthread_mutex_lock will also verify the atomicity of the memory
>   * transfer (UFFDIO_COPY).
> - *
> - * The program takes two parameters: the amounts of physical memory in
> - * megabytes (MiB) of the area and the number of bounces to execute.
> - *
> - * # 100MiB 99999 bounces
> - * ./userfaultfd 100 99999
> - *
> - * # 1GiB 99 bounces
> - * ./userfaultfd 1000 99
> - *
> - * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
> - * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
>   */
> 
>  #define _GNU_SOURCE
> @@ -115,6 +103,28 @@ pthread_attr_t attr;
>  				 ~(unsigned long)(sizeof(unsigned long long) \
>  						  -  1)))
> 
> +const char *examples =
> +    "# 100MiB 99999 bounces\n"
> +    "./userfaultfd anon 100 99999\n"
> +    "\n"
> +    "# 1GiB 99 bounces\n"
> +    "./userfaultfd anon 1000 99\n"
> +    "\n"
> +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> +    "\n";

While at it, can you please update the examples to include other test
types?

> +
> +static void usage(void)
> +{
> +	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
> +		"[hugetlbfs_file]\n\n");
> +	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
> +		"hugetlb_shared, shmem\n\n");
> +	fprintf(stderr, "Examples:\n\n");
> +	fprintf(stderr, examples);
> +	exit(1);
> +}
> +
>  static int anon_release_pages(char *rel_area)
>  {
>  	int ret = 0;
> @@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
>  int main(int argc, char **argv)
>  {
>  	if (argc < 4)
> -		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
> -				exit(1);
> +		usage();
> 
>  	if (signal(SIGALRM, sigalrm) == SIG_ERR)
>  		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
> @@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
>  		nr_cpus;
>  	if (!nr_pages_per_cpu) {
>  		fprintf(stderr, "invalid MiB\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
> 
>  	bounces = atoi(argv[3]);
>  	if (bounces <= 0) {
>  		fprintf(stderr, "invalid bounces\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
>  	nr_pages = nr_pages_per_cpu * nr_cpus;
> 
>  	if (test_type == TEST_HUGETLB) {
>  		if (argc < 5)
> -			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
> -				exit(1);
> +			usage();
>  		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
>  		if (huge_fd < 0) {
>  			fprintf(stderr, "Open of %s failed", argv[3]);
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Mike Rapoport]

On Sat, Sep 29, 2018@04:43:09PM +0800, Peter Xu wrote:
> Firstly, the help in the comment region is obsolete, now we support
> three parameters.  Since at it, change it and move it into the help
> message of the program.
> 
> Also, the help messages dumped here and there is obsolete too.  Use a
> single usage() helper.
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>

Small comment below, otherwise

Acked-by: Mike Rapoport <rppt at linux.vnet.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 44 ++++++++++++++----------
>  1 file changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 7b8171e3128a..2a84adaf8cf8 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -34,18 +34,6 @@
>   * per-CPU threads 1 by triggering userfaults inside
>   * pthread_mutex_lock will also verify the atomicity of the memory
>   * transfer (UFFDIO_COPY).
> - *
> - * The program takes two parameters: the amounts of physical memory in
> - * megabytes (MiB) of the area and the number of bounces to execute.
> - *
> - * # 100MiB 99999 bounces
> - * ./userfaultfd 100 99999
> - *
> - * # 1GiB 99 bounces
> - * ./userfaultfd 1000 99
> - *
> - * # 10MiB-~6GiB 999 bounces, continue forever unless an error triggers
> - * while ./userfaultfd $[RANDOM % 6000 + 10] 999; do true; done
>   */
> 
>  #define _GNU_SOURCE
> @@ -115,6 +103,28 @@ pthread_attr_t attr;
>  				 ~(unsigned long)(sizeof(unsigned long long) \
>  						  -  1)))
> 
> +const char *examples =
> +    "# 100MiB 99999 bounces\n"
> +    "./userfaultfd anon 100 99999\n"
> +    "\n"
> +    "# 1GiB 99 bounces\n"
> +    "./userfaultfd anon 1000 99\n"
> +    "\n"
> +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> +    "\n";

While at it, can you please update the examples to include other test
types?

> +
> +static void usage(void)
> +{
> +	fprintf(stderr, "\nUsage: ./userfaultfd <test type> <MiB> <bounces> "
> +		"[hugetlbfs_file]\n\n");
> +	fprintf(stderr, "Supported <test type>: anon, hugetlb, "
> +		"hugetlb_shared, shmem\n\n");
> +	fprintf(stderr, "Examples:\n\n");
> +	fprintf(stderr, examples);
> +	exit(1);
> +}
> +
>  static int anon_release_pages(char *rel_area)
>  {
>  	int ret = 0;
> @@ -1272,8 +1282,7 @@ static void sigalrm(int sig)
>  int main(int argc, char **argv)
>  {
>  	if (argc < 4)
> -		fprintf(stderr, "Usage: <test type> <MiB> <bounces> [hugetlbfs_file]\n"),
> -				exit(1);
> +		usage();
> 
>  	if (signal(SIGALRM, sigalrm) == SIG_ERR)
>  		fprintf(stderr, "failed to arm SIGALRM"), exit(1);
> @@ -1286,20 +1295,19 @@ int main(int argc, char **argv)
>  		nr_cpus;
>  	if (!nr_pages_per_cpu) {
>  		fprintf(stderr, "invalid MiB\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
> 
>  	bounces = atoi(argv[3]);
>  	if (bounces <= 0) {
>  		fprintf(stderr, "invalid bounces\n");
> -		fprintf(stderr, "Usage: <MiB> <bounces>\n"), exit(1);
> +		usage();
>  	}
>  	nr_pages = nr_pages_per_cpu * nr_cpus;
> 
>  	if (test_type == TEST_HUGETLB) {
>  		if (argc < 5)
> -			fprintf(stderr, "Usage: hugetlb <MiB> <bounces> <hugetlbfs_file>\n"),
> -				exit(1);
> +			usage();
>  		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
>  		if (huge_fd < 0) {
>  			fprintf(stderr, "Open of %s failed", argv[3]);
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[peterx]

On Sat, Sep 29, 2018 at 01:28:12PM +0300, Mike Rapoport wrote:

[...]

> > +const char *examples =
> > +    "# 100MiB 99999 bounces\n"
> > +    "./userfaultfd anon 100 99999\n"
> > +    "\n"
> > +    "# 1GiB 99 bounces\n"
> > +    "./userfaultfd anon 1000 99\n"
> > +    "\n"
> > +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> > +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> > +    "\n";
> 
> While at it, can you please update the examples to include other test
> types?

Sure thing.

Thanks for the quick review!

Regards,

-- 
Peter Xu

Re: [PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Peter Xu]

On Sat, Sep 29, 2018 at 01:28:12PM +0300, Mike Rapoport wrote:

[...]

> > +const char *examples =
> > +    "# 100MiB 99999 bounces\n"
> > +    "./userfaultfd anon 100 99999\n"
> > +    "\n"
> > +    "# 1GiB 99 bounces\n"
> > +    "./userfaultfd anon 1000 99\n"
> > +    "\n"
> > +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> > +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> > +    "\n";
> 
> While at it, can you please update the examples to include other test
> types?

Sure thing.

Thanks for the quick review!

Regards,

-- 
Peter Xu

[PATCH 1/3] userfaultfd: selftest: cleanup help messages

[Peter Xu]

On Sat, Sep 29, 2018@01:28:12PM +0300, Mike Rapoport wrote:

[...]

> > +const char *examples =
> > +    "# 100MiB 99999 bounces\n"
> > +    "./userfaultfd anon 100 99999\n"
> > +    "\n"
> > +    "# 1GiB 99 bounces\n"
> > +    "./userfaultfd anon 1000 99\n"
> > +    "\n"
> > +    "# 10MiB-~6GiB 999 bounces, continue forever unless an error triggers\n"
> > +    "while ./userfaultfd anon $[RANDOM % 6000 + 10] 999; do true; done\n"
> > +    "\n";
> 
> While at it, can you please update the examples to include other test
> types?

Sure thing.

Thanks for the quick review!

Regards,

-- 
Peter Xu

[PATCH 2/3] userfaultfd: selftest: generalize read and poll

[peterx]

We do very similar things in read and poll modes, but we're copying the
codes around.  Share the codes properly on reading the message and
handling the page fault to make the code cleaner.  Meanwhile this solves
previous mismatch of behaviors between the two modes on that the old
code:

- did not check EAGAIN case in read() mode
- ignored BOUNCE_VERIFY check in read() mode

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 2a84adaf8cf8..f79706f13ce7 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
 	return __copy_page(ufd, offset, false);
 }
 
+static int uffd_read_msg(int ufd, struct uffd_msg *msg)
+{
+	int ret = read(uffd, msg, sizeof(*msg));
+
+	if (ret != sizeof(*msg)) {
+		if (ret < 0)
+			if (errno == EAGAIN)
+				return 1;
+			else
+				perror("blocking read error"), exit(1);
+		else
+				fprintf(stderr, "short read\n"), exit(1);
+	}
+
+	return 0;
+}
+
+/* Return 1 if page fault handled by us; otherwise 0 */
+static int uffd_handle_page_fault(struct uffd_msg *msg)
+{
+	unsigned long offset;
+
+	if (msg->event != UFFD_EVENT_PAGEFAULT)
+		fprintf(stderr, "unexpected msg event %u\n",
+			msg->event), exit(1);
+
+	if (bounces & BOUNCE_VERIFY &&
+	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
+		fprintf(stderr, "unexpected write fault\n"), exit(1);
+
+	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+	offset &= ~(page_size-1);
+
+	return copy_page(uffd, offset);
+}
+
 static void *uffd_poll_thread(void *arg)
 {
 	unsigned long cpu = (unsigned long) arg;
@@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
 	struct uffd_msg msg;
 	struct uffdio_register uffd_reg;
 	int ret;
-	unsigned long offset;
 	char tmp_chr;
 	unsigned long userfaults = 0;
 
@@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
 		if (!(pollfd[0].revents & POLLIN))
 			fprintf(stderr, "pollfd[0].revents %d\n",
 				pollfd[0].revents), exit(1);
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret < 0) {
-			if (errno == EAGAIN)
-				continue;
-			perror("nonblocking read error"), exit(1);
-		}
+		if (uffd_read_msg(uffd, &msg))
+			continue;
 		switch (msg.event) {
 		default:
 			fprintf(stderr, "unexpected msg event %u\n",
 				msg.event), exit(1);
 			break;
 		case UFFD_EVENT_PAGEFAULT:
-			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-				fprintf(stderr, "unexpected write fault\n"), exit(1);
-			offset = (char *)(unsigned long)msg.arg.pagefault.address -
-				area_dst;
-			offset &= ~(page_size-1);
-			if (copy_page(uffd, offset))
-				userfaults++;
+			userfaults += uffd_handle_page_fault(&msg);
 			break;
 		case UFFD_EVENT_FORK:
 			close(uffd);
@@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
 {
 	unsigned long *this_cpu_userfaults;
 	struct uffd_msg msg;
-	unsigned long offset;
-	int ret;
 
 	this_cpu_userfaults = (unsigned long *) arg;
 	*this_cpu_userfaults = 0;
@@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
 	/* from here cancellation is ok */
 
 	for (;;) {
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret != sizeof(msg)) {
-			if (ret < 0)
-				perror("blocking read error"), exit(1);
-			else
-				fprintf(stderr, "short read\n"), exit(1);
-		}
-		if (msg.event != UFFD_EVENT_PAGEFAULT)
-			fprintf(stderr, "unexpected msg event %u\n",
-				msg.event), exit(1);
-		if (bounces & BOUNCE_VERIFY &&
-		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-			fprintf(stderr, "unexpected write fault\n"), exit(1);
-		offset = (char *)(unsigned long)msg.arg.pagefault.address -
-			 area_dst;
-		offset &= ~(page_size-1);
-		if (copy_page(uffd, offset))
-			(*this_cpu_userfaults)++;
+		if (uffd_read_msg(uffd, &msg))
+			continue;
+		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
 	}
 	return (void *)NULL;
 }
-- 
2.17.1

[PATCH 2/3] userfaultfd: selftest: generalize read and poll

[Peter Xu]

We do very similar things in read and poll modes, but we're copying the
codes around.  Share the codes properly on reading the message and
handling the page fault to make the code cleaner.  Meanwhile this solves
previous mismatch of behaviors between the two modes on that the old
code:

- did not check EAGAIN case in read() mode
- ignored BOUNCE_VERIFY check in read() mode

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 2a84adaf8cf8..f79706f13ce7 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
 	return __copy_page(ufd, offset, false);
 }
 
+static int uffd_read_msg(int ufd, struct uffd_msg *msg)
+{
+	int ret = read(uffd, msg, sizeof(*msg));
+
+	if (ret != sizeof(*msg)) {
+		if (ret < 0)
+			if (errno == EAGAIN)
+				return 1;
+			else
+				perror("blocking read error"), exit(1);
+		else
+				fprintf(stderr, "short read\n"), exit(1);
+	}
+
+	return 0;
+}
+
+/* Return 1 if page fault handled by us; otherwise 0 */
+static int uffd_handle_page_fault(struct uffd_msg *msg)
+{
+	unsigned long offset;
+
+	if (msg->event != UFFD_EVENT_PAGEFAULT)
+		fprintf(stderr, "unexpected msg event %u\n",
+			msg->event), exit(1);
+
+	if (bounces & BOUNCE_VERIFY &&
+	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
+		fprintf(stderr, "unexpected write fault\n"), exit(1);
+
+	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+	offset &= ~(page_size-1);
+
+	return copy_page(uffd, offset);
+}
+
 static void *uffd_poll_thread(void *arg)
 {
 	unsigned long cpu = (unsigned long) arg;
@@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
 	struct uffd_msg msg;
 	struct uffdio_register uffd_reg;
 	int ret;
-	unsigned long offset;
 	char tmp_chr;
 	unsigned long userfaults = 0;
 
@@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
 		if (!(pollfd[0].revents & POLLIN))
 			fprintf(stderr, "pollfd[0].revents %d\n",
 				pollfd[0].revents), exit(1);
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret < 0) {
-			if (errno == EAGAIN)
-				continue;
-			perror("nonblocking read error"), exit(1);
-		}
+		if (uffd_read_msg(uffd, &msg))
+			continue;
 		switch (msg.event) {
 		default:
 			fprintf(stderr, "unexpected msg event %u\n",
 				msg.event), exit(1);
 			break;
 		case UFFD_EVENT_PAGEFAULT:
-			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-				fprintf(stderr, "unexpected write fault\n"), exit(1);
-			offset = (char *)(unsigned long)msg.arg.pagefault.address -
-				area_dst;
-			offset &= ~(page_size-1);
-			if (copy_page(uffd, offset))
-				userfaults++;
+			userfaults += uffd_handle_page_fault(&msg);
 			break;
 		case UFFD_EVENT_FORK:
 			close(uffd);
@@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
 {
 	unsigned long *this_cpu_userfaults;
 	struct uffd_msg msg;
-	unsigned long offset;
-	int ret;
 
 	this_cpu_userfaults = (unsigned long *) arg;
 	*this_cpu_userfaults = 0;
@@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
 	/* from here cancellation is ok */
 
 	for (;;) {
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret != sizeof(msg)) {
-			if (ret < 0)
-				perror("blocking read error"), exit(1);
-			else
-				fprintf(stderr, "short read\n"), exit(1);
-		}
-		if (msg.event != UFFD_EVENT_PAGEFAULT)
-			fprintf(stderr, "unexpected msg event %u\n",
-				msg.event), exit(1);
-		if (bounces & BOUNCE_VERIFY &&
-		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-			fprintf(stderr, "unexpected write fault\n"), exit(1);
-		offset = (char *)(unsigned long)msg.arg.pagefault.address -
-			 area_dst;
-		offset &= ~(page_size-1);
-		if (copy_page(uffd, offset))
-			(*this_cpu_userfaults)++;
+		if (uffd_read_msg(uffd, &msg))
+			continue;
+		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
 	}
 	return (void *)NULL;
 }
-- 
2.17.1

[PATCH 2/3] userfaultfd: selftest: generalize read and poll

[Peter Xu]

We do very similar things in read and poll modes, but we're copying the
codes around.  Share the codes properly on reading the message and
handling the page fault to make the code cleaner.  Meanwhile this solves
previous mismatch of behaviors between the two modes on that the old
code:

- did not check EAGAIN case in read() mode
- ignored BOUNCE_VERIFY check in read() mode

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 2a84adaf8cf8..f79706f13ce7 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
 	return __copy_page(ufd, offset, false);
 }
 
+static int uffd_read_msg(int ufd, struct uffd_msg *msg)
+{
+	int ret = read(uffd, msg, sizeof(*msg));
+
+	if (ret != sizeof(*msg)) {
+		if (ret < 0)
+			if (errno == EAGAIN)
+				return 1;
+			else
+				perror("blocking read error"), exit(1);
+		else
+				fprintf(stderr, "short read\n"), exit(1);
+	}
+
+	return 0;
+}
+
+/* Return 1 if page fault handled by us; otherwise 0 */
+static int uffd_handle_page_fault(struct uffd_msg *msg)
+{
+	unsigned long offset;
+
+	if (msg->event != UFFD_EVENT_PAGEFAULT)
+		fprintf(stderr, "unexpected msg event %u\n",
+			msg->event), exit(1);
+
+	if (bounces & BOUNCE_VERIFY &&
+	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
+		fprintf(stderr, "unexpected write fault\n"), exit(1);
+
+	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
+	offset &= ~(page_size-1);
+
+	return copy_page(uffd, offset);
+}
+
 static void *uffd_poll_thread(void *arg)
 {
 	unsigned long cpu = (unsigned long) arg;
@@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
 	struct uffd_msg msg;
 	struct uffdio_register uffd_reg;
 	int ret;
-	unsigned long offset;
 	char tmp_chr;
 	unsigned long userfaults = 0;
 
@@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
 		if (!(pollfd[0].revents & POLLIN))
 			fprintf(stderr, "pollfd[0].revents %d\n",
 				pollfd[0].revents), exit(1);
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret < 0) {
-			if (errno == EAGAIN)
-				continue;
-			perror("nonblocking read error"), exit(1);
-		}
+		if (uffd_read_msg(uffd, &msg))
+			continue;
 		switch (msg.event) {
 		default:
 			fprintf(stderr, "unexpected msg event %u\n",
 				msg.event), exit(1);
 			break;
 		case UFFD_EVENT_PAGEFAULT:
-			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-				fprintf(stderr, "unexpected write fault\n"), exit(1);
-			offset = (char *)(unsigned long)msg.arg.pagefault.address -
-				area_dst;
-			offset &= ~(page_size-1);
-			if (copy_page(uffd, offset))
-				userfaults++;
+			userfaults += uffd_handle_page_fault(&msg);
 			break;
 		case UFFD_EVENT_FORK:
 			close(uffd);
@@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
 {
 	unsigned long *this_cpu_userfaults;
 	struct uffd_msg msg;
-	unsigned long offset;
-	int ret;
 
 	this_cpu_userfaults = (unsigned long *) arg;
 	*this_cpu_userfaults = 0;
@@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
 	/* from here cancellation is ok */
 
 	for (;;) {
-		ret = read(uffd, &msg, sizeof(msg));
-		if (ret != sizeof(msg)) {
-			if (ret < 0)
-				perror("blocking read error"), exit(1);
-			else
-				fprintf(stderr, "short read\n"), exit(1);
-		}
-		if (msg.event != UFFD_EVENT_PAGEFAULT)
-			fprintf(stderr, "unexpected msg event %u\n",
-				msg.event), exit(1);
-		if (bounces & BOUNCE_VERIFY &&
-		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
-			fprintf(stderr, "unexpected write fault\n"), exit(1);
-		offset = (char *)(unsigned long)msg.arg.pagefault.address -
-			 area_dst;
-		offset &= ~(page_size-1);
-		if (copy_page(uffd, offset))
-			(*this_cpu_userfaults)++;
+		if (uffd_read_msg(uffd, &msg))
+			continue;
+		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
 	}
 	return (void *)NULL;
 }
-- 
2.17.1

[PATCH 2/3] userfaultfd: selftest: generalize read and poll

[rppt]

On Sat, Sep 29, 2018 at 04:43:10PM +0800, Peter Xu wrote:
> We do very similar things in read and poll modes, but we're copying the
> codes around.  Share the codes properly on reading the message and
> handling the page fault to make the code cleaner.  Meanwhile this solves
> previous mismatch of behaviors between the two modes on that the old
> code:
> 
> - did not check EAGAIN case in read() mode
> - ignored BOUNCE_VERIFY check in read() mode
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>
> ---
>  tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
>  1 file changed, 42 insertions(+), 34 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 2a84adaf8cf8..f79706f13ce7 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
>  	return __copy_page(ufd, offset, false);
>  }
> 
> +static int uffd_read_msg(int ufd, struct uffd_msg *msg)
> +{
> +	int ret = read(uffd, msg, sizeof(*msg));
> +
> +	if (ret != sizeof(*msg)) {
> +		if (ret < 0)

I'd appreciate curly brace here

> +			if (errno == EAGAIN)
> +				return 1;
> +			else
> +				perror("blocking read error"), exit(1);
> +		else

and here

> +				fprintf(stderr, "short read\n"), exit(1);
> +	}
> +
> +	return 0;
> +}
> +
> +/* Return 1 if page fault handled by us; otherwise 0 */
> +static int uffd_handle_page_fault(struct uffd_msg *msg)
> +{
> +	unsigned long offset;
> +
> +	if (msg->event != UFFD_EVENT_PAGEFAULT)
> +		fprintf(stderr, "unexpected msg event %u\n",
> +			msg->event), exit(1);
> +
> +	if (bounces & BOUNCE_VERIFY &&
> +	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> +		fprintf(stderr, "unexpected write fault\n"), exit(1);
> +
> +	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
> +	offset &= ~(page_size-1);
> +
> +	return copy_page(uffd, offset);
> +}
> +
>  static void *uffd_poll_thread(void *arg)
>  {
>  	unsigned long cpu = (unsigned long) arg;
> @@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
>  	struct uffd_msg msg;
>  	struct uffdio_register uffd_reg;
>  	int ret;
> -	unsigned long offset;
>  	char tmp_chr;
>  	unsigned long userfaults = 0;
> 
> @@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
>  		if (!(pollfd[0].revents & POLLIN))
>  			fprintf(stderr, "pollfd[0].revents %d\n",
>  				pollfd[0].revents), exit(1);
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret < 0) {
> -			if (errno == EAGAIN)
> -				continue;
> -			perror("nonblocking read error"), exit(1);
> -		}
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
>  		switch (msg.event) {
>  		default:
>  			fprintf(stderr, "unexpected msg event %u\n",
>  				msg.event), exit(1);
>  			break;
>  		case UFFD_EVENT_PAGEFAULT:
> -			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -				fprintf(stderr, "unexpected write fault\n"), exit(1);
> -			offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -				area_dst;
> -			offset &= ~(page_size-1);
> -			if (copy_page(uffd, offset))
> -				userfaults++;
> +			userfaults += uffd_handle_page_fault(&msg);
>  			break;
>  		case UFFD_EVENT_FORK:
>  			close(uffd);
> @@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
>  {
>  	unsigned long *this_cpu_userfaults;
>  	struct uffd_msg msg;
> -	unsigned long offset;
> -	int ret;
> 
>  	this_cpu_userfaults = (unsigned long *) arg;
>  	*this_cpu_userfaults = 0;
> @@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
>  	/* from here cancellation is ok */
> 
>  	for (;;) {
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret != sizeof(msg)) {
> -			if (ret < 0)
> -				perror("blocking read error"), exit(1);
> -			else
> -				fprintf(stderr, "short read\n"), exit(1);
> -		}
> -		if (msg.event != UFFD_EVENT_PAGEFAULT)
> -			fprintf(stderr, "unexpected msg event %u\n",
> -				msg.event), exit(1);
> -		if (bounces & BOUNCE_VERIFY &&
> -		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -			fprintf(stderr, "unexpected write fault\n"), exit(1);
> -		offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -			 area_dst;
> -		offset &= ~(page_size-1);
> -		if (copy_page(uffd, offset))
> -			(*this_cpu_userfaults)++;
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
> +		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
>  	}
>  	return (void *)NULL;
>  }
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 2/3] userfaultfd: selftest: generalize read and poll

[Mike Rapoport]

On Sat, Sep 29, 2018 at 04:43:10PM +0800, Peter Xu wrote:
> We do very similar things in read and poll modes, but we're copying the
> codes around.  Share the codes properly on reading the message and
> handling the page fault to make the code cleaner.  Meanwhile this solves
> previous mismatch of behaviors between the two modes on that the old
> code:
> 
> - did not check EAGAIN case in read() mode
> - ignored BOUNCE_VERIFY check in read() mode
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
>  1 file changed, 42 insertions(+), 34 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 2a84adaf8cf8..f79706f13ce7 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
>  	return __copy_page(ufd, offset, false);
>  }
> 
> +static int uffd_read_msg(int ufd, struct uffd_msg *msg)
> +{
> +	int ret = read(uffd, msg, sizeof(*msg));
> +
> +	if (ret != sizeof(*msg)) {
> +		if (ret < 0)

I'd appreciate curly brace here

> +			if (errno == EAGAIN)
> +				return 1;
> +			else
> +				perror("blocking read error"), exit(1);
> +		else

and here

> +				fprintf(stderr, "short read\n"), exit(1);
> +	}
> +
> +	return 0;
> +}
> +
> +/* Return 1 if page fault handled by us; otherwise 0 */
> +static int uffd_handle_page_fault(struct uffd_msg *msg)
> +{
> +	unsigned long offset;
> +
> +	if (msg->event != UFFD_EVENT_PAGEFAULT)
> +		fprintf(stderr, "unexpected msg event %u\n",
> +			msg->event), exit(1);
> +
> +	if (bounces & BOUNCE_VERIFY &&
> +	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> +		fprintf(stderr, "unexpected write fault\n"), exit(1);
> +
> +	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
> +	offset &= ~(page_size-1);
> +
> +	return copy_page(uffd, offset);
> +}
> +
>  static void *uffd_poll_thread(void *arg)
>  {
>  	unsigned long cpu = (unsigned long) arg;
> @@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
>  	struct uffd_msg msg;
>  	struct uffdio_register uffd_reg;
>  	int ret;
> -	unsigned long offset;
>  	char tmp_chr;
>  	unsigned long userfaults = 0;
> 
> @@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
>  		if (!(pollfd[0].revents & POLLIN))
>  			fprintf(stderr, "pollfd[0].revents %d\n",
>  				pollfd[0].revents), exit(1);
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret < 0) {
> -			if (errno == EAGAIN)
> -				continue;
> -			perror("nonblocking read error"), exit(1);
> -		}
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
>  		switch (msg.event) {
>  		default:
>  			fprintf(stderr, "unexpected msg event %u\n",
>  				msg.event), exit(1);
>  			break;
>  		case UFFD_EVENT_PAGEFAULT:
> -			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -				fprintf(stderr, "unexpected write fault\n"), exit(1);
> -			offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -				area_dst;
> -			offset &= ~(page_size-1);
> -			if (copy_page(uffd, offset))
> -				userfaults++;
> +			userfaults += uffd_handle_page_fault(&msg);
>  			break;
>  		case UFFD_EVENT_FORK:
>  			close(uffd);
> @@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
>  {
>  	unsigned long *this_cpu_userfaults;
>  	struct uffd_msg msg;
> -	unsigned long offset;
> -	int ret;
> 
>  	this_cpu_userfaults = (unsigned long *) arg;
>  	*this_cpu_userfaults = 0;
> @@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
>  	/* from here cancellation is ok */
> 
>  	for (;;) {
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret != sizeof(msg)) {
> -			if (ret < 0)
> -				perror("blocking read error"), exit(1);
> -			else
> -				fprintf(stderr, "short read\n"), exit(1);
> -		}
> -		if (msg.event != UFFD_EVENT_PAGEFAULT)
> -			fprintf(stderr, "unexpected msg event %u\n",
> -				msg.event), exit(1);
> -		if (bounces & BOUNCE_VERIFY &&
> -		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -			fprintf(stderr, "unexpected write fault\n"), exit(1);
> -		offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -			 area_dst;
> -		offset &= ~(page_size-1);
> -		if (copy_page(uffd, offset))
> -			(*this_cpu_userfaults)++;
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
> +		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
>  	}
>  	return (void *)NULL;
>  }
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

[PATCH 2/3] userfaultfd: selftest: generalize read and poll

[Mike Rapoport]

On Sat, Sep 29, 2018@04:43:10PM +0800, Peter Xu wrote:
> We do very similar things in read and poll modes, but we're copying the
> codes around.  Share the codes properly on reading the message and
> handling the page fault to make the code cleaner.  Meanwhile this solves
> previous mismatch of behaviors between the two modes on that the old
> code:
> 
> - did not check EAGAIN case in read() mode
> - ignored BOUNCE_VERIFY check in read() mode
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>
> ---
>  tools/testing/selftests/vm/userfaultfd.c | 76 +++++++++++++-----------
>  1 file changed, 42 insertions(+), 34 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 2a84adaf8cf8..f79706f13ce7 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -449,6 +449,42 @@ static int copy_page(int ufd, unsigned long offset)
>  	return __copy_page(ufd, offset, false);
>  }
> 
> +static int uffd_read_msg(int ufd, struct uffd_msg *msg)
> +{
> +	int ret = read(uffd, msg, sizeof(*msg));
> +
> +	if (ret != sizeof(*msg)) {
> +		if (ret < 0)

I'd appreciate curly brace here

> +			if (errno == EAGAIN)
> +				return 1;
> +			else
> +				perror("blocking read error"), exit(1);
> +		else

and here

> +				fprintf(stderr, "short read\n"), exit(1);
> +	}
> +
> +	return 0;
> +}
> +
> +/* Return 1 if page fault handled by us; otherwise 0 */
> +static int uffd_handle_page_fault(struct uffd_msg *msg)
> +{
> +	unsigned long offset;
> +
> +	if (msg->event != UFFD_EVENT_PAGEFAULT)
> +		fprintf(stderr, "unexpected msg event %u\n",
> +			msg->event), exit(1);
> +
> +	if (bounces & BOUNCE_VERIFY &&
> +	    msg->arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> +		fprintf(stderr, "unexpected write fault\n"), exit(1);
> +
> +	offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
> +	offset &= ~(page_size-1);
> +
> +	return copy_page(uffd, offset);
> +}
> +
>  static void *uffd_poll_thread(void *arg)
>  {
>  	unsigned long cpu = (unsigned long) arg;
> @@ -456,7 +492,6 @@ static void *uffd_poll_thread(void *arg)
>  	struct uffd_msg msg;
>  	struct uffdio_register uffd_reg;
>  	int ret;
> -	unsigned long offset;
>  	char tmp_chr;
>  	unsigned long userfaults = 0;
> 
> @@ -480,25 +515,15 @@ static void *uffd_poll_thread(void *arg)
>  		if (!(pollfd[0].revents & POLLIN))
>  			fprintf(stderr, "pollfd[0].revents %d\n",
>  				pollfd[0].revents), exit(1);
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret < 0) {
> -			if (errno == EAGAIN)
> -				continue;
> -			perror("nonblocking read error"), exit(1);
> -		}
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
>  		switch (msg.event) {
>  		default:
>  			fprintf(stderr, "unexpected msg event %u\n",
>  				msg.event), exit(1);
>  			break;
>  		case UFFD_EVENT_PAGEFAULT:
> -			if (msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -				fprintf(stderr, "unexpected write fault\n"), exit(1);
> -			offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -				area_dst;
> -			offset &= ~(page_size-1);
> -			if (copy_page(uffd, offset))
> -				userfaults++;
> +			userfaults += uffd_handle_page_fault(&msg);
>  			break;
>  		case UFFD_EVENT_FORK:
>  			close(uffd);
> @@ -526,8 +551,6 @@ static void *uffd_read_thread(void *arg)
>  {
>  	unsigned long *this_cpu_userfaults;
>  	struct uffd_msg msg;
> -	unsigned long offset;
> -	int ret;
> 
>  	this_cpu_userfaults = (unsigned long *) arg;
>  	*this_cpu_userfaults = 0;
> @@ -536,24 +559,9 @@ static void *uffd_read_thread(void *arg)
>  	/* from here cancellation is ok */
> 
>  	for (;;) {
> -		ret = read(uffd, &msg, sizeof(msg));
> -		if (ret != sizeof(msg)) {
> -			if (ret < 0)
> -				perror("blocking read error"), exit(1);
> -			else
> -				fprintf(stderr, "short read\n"), exit(1);
> -		}
> -		if (msg.event != UFFD_EVENT_PAGEFAULT)
> -			fprintf(stderr, "unexpected msg event %u\n",
> -				msg.event), exit(1);
> -		if (bounces & BOUNCE_VERIFY &&
> -		    msg.arg.pagefault.flags & UFFD_PAGEFAULT_FLAG_WRITE)
> -			fprintf(stderr, "unexpected write fault\n"), exit(1);
> -		offset = (char *)(unsigned long)msg.arg.pagefault.address -
> -			 area_dst;
> -		offset &= ~(page_size-1);
> -		if (copy_page(uffd, offset))
> -			(*this_cpu_userfaults)++;
> +		if (uffd_read_msg(uffd, &msg))
> +			continue;
> +		(*this_cpu_userfaults) += uffd_handle_page_fault(&msg);
>  	}
>  	return (void *)NULL;
>  }
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

[PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[peterx]

Now we recycle the uffd servicing threads earlier than the lock
threads.  It might happen that when the lock thread is still blocked at
a pthread mutex lock while the servicing thread has already quitted for
the cpu so the lock thread will be blocked forever and hang the test
program.  To fix the possible race, recycle the lock threads first.

This never happens with current missing-only tests, but when I start to
run the write-protection tests (the feature is not yet posted upstream)
it happens every time of the run possibly because in that new test we'll
need to service two page faults for each lock operation.

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f79706f13ce7..a388675b15af 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
 	if (uffd_test_ops->release_pages(area_src))
 		return 1;
 
+
+	finished = 1;
+	for (cpu = 0; cpu < nr_cpus; cpu++)
+		if (pthread_join(locking_threads[cpu], NULL))
+			return 1;
+
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 		char c;
 		if (bounces & BOUNCE_POLL) {
@@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
 		}
 	}
 
-	finished = 1;
-	for (cpu = 0; cpu < nr_cpus; cpu++)
-		if (pthread_join(locking_threads[cpu], NULL))
-			return 1;
-
 	return 0;
 }
 
-- 
2.17.1

[PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[Peter Xu]

Now we recycle the uffd servicing threads earlier than the lock
threads.  It might happen that when the lock thread is still blocked at
a pthread mutex lock while the servicing thread has already quitted for
the cpu so the lock thread will be blocked forever and hang the test
program.  To fix the possible race, recycle the lock threads first.

This never happens with current missing-only tests, but when I start to
run the write-protection tests (the feature is not yet posted upstream)
it happens every time of the run possibly because in that new test we'll
need to service two page faults for each lock operation.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f79706f13ce7..a388675b15af 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
 	if (uffd_test_ops->release_pages(area_src))
 		return 1;
 
+
+	finished = 1;
+	for (cpu = 0; cpu < nr_cpus; cpu++)
+		if (pthread_join(locking_threads[cpu], NULL))
+			return 1;
+
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 		char c;
 		if (bounces & BOUNCE_POLL) {
@@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
 		}
 	}
 
-	finished = 1;
-	for (cpu = 0; cpu < nr_cpus; cpu++)
-		if (pthread_join(locking_threads[cpu], NULL))
-			return 1;
-
 	return 0;
 }
 
-- 
2.17.1

[PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[Peter Xu]

Now we recycle the uffd servicing threads earlier than the lock
threads.  It might happen that when the lock thread is still blocked at
a pthread mutex lock while the servicing thread has already quitted for
the cpu so the lock thread will be blocked forever and hang the test
program.  To fix the possible race, recycle the lock threads first.

This never happens with current missing-only tests, but when I start to
run the write-protection tests (the feature is not yet posted upstream)
it happens every time of the run possibly because in that new test we'll
need to service two page faults for each lock operation.

Signed-off-by: Peter Xu <peterx at redhat.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index f79706f13ce7..a388675b15af 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
 	if (uffd_test_ops->release_pages(area_src))
 		return 1;
 
+
+	finished = 1;
+	for (cpu = 0; cpu < nr_cpus; cpu++)
+		if (pthread_join(locking_threads[cpu], NULL))
+			return 1;
+
 	for (cpu = 0; cpu < nr_cpus; cpu++) {
 		char c;
 		if (bounces & BOUNCE_POLL) {
@@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
 		}
 	}
 
-	finished = 1;
-	for (cpu = 0; cpu < nr_cpus; cpu++)
-		if (pthread_join(locking_threads[cpu], NULL))
-			return 1;
-
 	return 0;
 }
 
-- 
2.17.1

[PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[rppt]

On Sat, Sep 29, 2018 at 04:43:11PM +0800, Peter Xu wrote:
> Now we recycle the uffd servicing threads earlier than the lock
> threads.  It might happen that when the lock thread is still blocked at
> a pthread mutex lock while the servicing thread has already quitted for
> the cpu so the lock thread will be blocked forever and hang the test
> program.  To fix the possible race, recycle the lock threads first.
> 
> This never happens with current missing-only tests, but when I start to
> run the write-protection tests (the feature is not yet posted upstream)
> it happens every time of the run possibly because in that new test we'll
> need to service two page faults for each lock operation.
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>

Acked-by: Mike Rapoport <rppt at linux.vnt.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index f79706f13ce7..a388675b15af 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
>  	if (uffd_test_ops->release_pages(area_src))
>  		return 1;
> 
> +
> +	finished = 1;
> +	for (cpu = 0; cpu < nr_cpus; cpu++)
> +		if (pthread_join(locking_threads[cpu], NULL))
> +			return 1;
> +
>  	for (cpu = 0; cpu < nr_cpus; cpu++) {
>  		char c;
>  		if (bounces & BOUNCE_POLL) {
> @@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
>  		}
>  	}
> 
> -	finished = 1;
> -	for (cpu = 0; cpu < nr_cpus; cpu++)
> -		if (pthread_join(locking_threads[cpu], NULL))
> -			return 1;
> -
>  	return 0;
>  }
> 
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[Mike Rapoport]

On Sat, Sep 29, 2018 at 04:43:11PM +0800, Peter Xu wrote:
> Now we recycle the uffd servicing threads earlier than the lock
> threads.  It might happen that when the lock thread is still blocked at
> a pthread mutex lock while the servicing thread has already quitted for
> the cpu so the lock thread will be blocked forever and hang the test
> program.  To fix the possible race, recycle the lock threads first.
> 
> This never happens with current missing-only tests, but when I start to
> run the write-protection tests (the feature is not yet posted upstream)
> it happens every time of the run possibly because in that new test we'll
> need to service two page faults for each lock operation.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Acked-by: Mike Rapoport <rppt@linux.vnt.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index f79706f13ce7..a388675b15af 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
>  	if (uffd_test_ops->release_pages(area_src))
>  		return 1;
> 
> +
> +	finished = 1;
> +	for (cpu = 0; cpu < nr_cpus; cpu++)
> +		if (pthread_join(locking_threads[cpu], NULL))
> +			return 1;
> +
>  	for (cpu = 0; cpu < nr_cpus; cpu++) {
>  		char c;
>  		if (bounces & BOUNCE_POLL) {
> @@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
>  		}
>  	}
> 
> -	finished = 1;
> -	for (cpu = 0; cpu < nr_cpus; cpu++)
> -		if (pthread_join(locking_threads[cpu], NULL))
> -			return 1;
> -
>  	return 0;
>  }
> 
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.

[PATCH 3/3] userfaultfd: selftest: recycle lock threads first

[Mike Rapoport]

On Sat, Sep 29, 2018@04:43:11PM +0800, Peter Xu wrote:
> Now we recycle the uffd servicing threads earlier than the lock
> threads.  It might happen that when the lock thread is still blocked at
> a pthread mutex lock while the servicing thread has already quitted for
> the cpu so the lock thread will be blocked forever and hang the test
> program.  To fix the possible race, recycle the lock threads first.
> 
> This never happens with current missing-only tests, but when I start to
> run the write-protection tests (the feature is not yet posted upstream)
> it happens every time of the run possibly because in that new test we'll
> need to service two page faults for each lock operation.
> 
> Signed-off-by: Peter Xu <peterx at redhat.com>

Acked-by: Mike Rapoport <rppt at linux.vnt.ibm.com>

> ---
>  tools/testing/selftests/vm/userfaultfd.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index f79706f13ce7..a388675b15af 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -623,6 +623,12 @@ static int stress(unsigned long *userfaults)
>  	if (uffd_test_ops->release_pages(area_src))
>  		return 1;
> 
> +
> +	finished = 1;
> +	for (cpu = 0; cpu < nr_cpus; cpu++)
> +		if (pthread_join(locking_threads[cpu], NULL))
> +			return 1;
> +
>  	for (cpu = 0; cpu < nr_cpus; cpu++) {
>  		char c;
>  		if (bounces & BOUNCE_POLL) {
> @@ -640,11 +646,6 @@ static int stress(unsigned long *userfaults)
>  		}
>  	}
> 
> -	finished = 1;
> -	for (cpu = 0; cpu < nr_cpus; cpu++)
> -		if (pthread_join(locking_threads[cpu], NULL))
> -			return 1;
> -
>  	return 0;
>  }
> 
> -- 
> 2.17.1
> 

-- 
Sincerely yours,
Mike.