diff mbox series

[19/29] selftests/mm: Let uffd_handle_page_fault() takes wp parameter

Message ID 20230330160812.3107522-1-peterx@redhat.com (mailing list archive)
State New
Headers show
Series selftests/mm: Split / Refactor userfault test | expand

Commit Message

Peter Xu March 30, 2023, 4:08 p.m. UTC 
Make the handler optionally apply WP bit when resolving page faults for
either missing or minor page faults.  This move towards removing global
test_uffdio_wp outside of the common code.

For this, slightly abuse uffd_stats to keep one more parameter on whether
we'd like to resolve page faults with WP bit set.  Note that only the name
is abused, it'll be better to be called uffd_args or similar but let's not
bother for now.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/mm/uffd-common.c | 17 +++++++++--------
 tools/testing/selftests/mm/uffd-common.h |  6 ++++--
 tools/testing/selftests/mm/uffd-stress.c | 16 ++++++++++------
 3 files changed, 23 insertions(+), 16 deletions(-)

Comments

Mike Rapoport April 11, 2023, 10:52 a.m. UTC | #1 
On Thu, Mar 30, 2023 at 12:08:12PM -0400, Peter Xu wrote:
> Subject: selftests/mm: Let uffd_handle_page_fault() takes wp parameter

Nit:                                                 ^ take

> Make the handler optionally apply WP bit when resolving page faults for
> either missing or minor page faults.  This move towards removing global

Nit:                                        ^ moves

> test_uffdio_wp outside of the common code.
> 
> For this, slightly abuse uffd_stats to keep one more parameter on whether
> we'd like to resolve page faults with WP bit set.  Note that only the name
> is abused, it'll be better to be called uffd_args or similar but let's not
> bother for now.

Maybe one of the first commits in the series should have been
s/uffd_stats/uffd_args/g, but I realize that it's PITA, so I won't insist.
 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tools/testing/selftests/mm/uffd-common.c | 17 +++++++++--------
>  tools/testing/selftests/mm/uffd-common.h |  6 ++++--
>  tools/testing/selftests/mm/uffd-stress.c | 16 ++++++++++------
>  3 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
> index 025e40ffc7bf..92b7e00efa8a 100644
> --- a/tools/testing/selftests/mm/uffd-common.c
> +++ b/tools/testing/selftests/mm/uffd-common.c
> @@ -353,7 +353,7 @@ void wp_range(int ufd, __u64 start, __u64 len, bool wp)
>  		err("clear WP failed: address=0x%"PRIx64, (uint64_t)start);
>  }
> 
> -static void continue_range(int ufd, __u64 start, __u64 len)
> +static void continue_range(int ufd, __u64 start, __u64 len, bool wp)
>  {
>  	struct uffdio_continue req;
>  	int ret;
> @@ -361,7 +361,7 @@ static void continue_range(int ufd, __u64 start, __u64 len)
>  	req.range.start = start;
>  	req.range.len = len;
>  	req.mode = 0;
> -	if (test_uffdio_wp)
> +	if (wp)
>  		req.mode |= UFFDIO_CONTINUE_MODE_WP;
> 
>  	if (ioctl(ufd, UFFDIO_CONTINUE, &req))
> @@ -429,7 +429,8 @@ void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats)
>  				    area_dst_alias));
>  		for (b = 0; b < page_size; ++b)
>  			area[b] = ~area[b];
> -		continue_range(uffd, msg->arg.pagefault.address, page_size);
> +		continue_range(uffd, msg->arg.pagefault.address, page_size,
> +			       stats->apply_wp);
>  		stats->minor_faults++;
>  	} else {
>  		/*
> @@ -459,7 +460,7 @@ void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats)
>  		offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
>  		offset &= ~(page_size-1);
> 
> -		if (copy_page(uffd, offset))
> +		if (copy_page(uffd, offset, stats->apply_wp))
>  			stats->missing_faults++;
>  	}
>  }
> @@ -555,7 +556,7 @@ static void wake_range(int ufd, unsigned long addr, unsigned long len)
>  			addr), exit(1);
>  }
> 
> -int __copy_page(int ufd, unsigned long offset, bool retry)
> +int __copy_page(int ufd, unsigned long offset, bool retry, bool wp)
>  {
>  	struct uffdio_copy uffdio_copy;
> 
> @@ -564,7 +565,7 @@ int __copy_page(int ufd, unsigned long offset, bool retry)
>  	uffdio_copy.dst = (unsigned long) area_dst + offset;
>  	uffdio_copy.src = (unsigned long) area_src + offset;
>  	uffdio_copy.len = page_size;
> -	if (test_uffdio_wp)
> +	if (wp)
>  		uffdio_copy.mode = UFFDIO_COPY_MODE_WP;
>  	else
>  		uffdio_copy.mode = 0;
> @@ -587,8 +588,8 @@ int __copy_page(int ufd, unsigned long offset, bool retry)
>  	return 0;
>  }
> 
> -int copy_page(int ufd, unsigned long offset)
> +int copy_page(int ufd, unsigned long offset, bool wp)
>  {
> -	return __copy_page(ufd, offset, false);
> +	return __copy_page(ufd, offset, false, wp);
>  }
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
> index 47565b2f2dee..f4bc73ce3b48 100644
> --- a/tools/testing/selftests/mm/uffd-common.h
> +++ b/tools/testing/selftests/mm/uffd-common.h
> @@ -72,6 +72,8 @@
>  /* Userfaultfd test statistics */
>  struct uffd_stats {
>  	int cpu;
> +	/* Whether apply wr-protects when installing pages */
> +	bool apply_wp;
>  	unsigned long missing_faults;
>  	unsigned long wp_faults;
>  	unsigned long minor_faults;
> @@ -104,8 +106,8 @@ void userfaultfd_open(uint64_t *features);
>  int uffd_read_msg(int ufd, struct uffd_msg *msg);
>  void wp_range(int ufd, __u64 start, __u64 len, bool wp);
>  void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats);
> -int __copy_page(int ufd, unsigned long offset, bool retry);
> -int copy_page(int ufd, unsigned long offset);
> +int __copy_page(int ufd, unsigned long offset, bool retry, bool wp);
> +int copy_page(int ufd, unsigned long offset, bool wp);
>  void *uffd_poll_thread(void *arg);
> 
>  #define TEST_ANON	1
> diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> index 54fc9b4ffa3c..70cb0619354e 100644
> --- a/tools/testing/selftests/mm/uffd-stress.c
> +++ b/tools/testing/selftests/mm/uffd-stress.c
> @@ -97,6 +97,7 @@ static void uffd_stats_reset(struct uffd_stats *uffd_stats,
> 
>  	for (i = 0; i < n_cpus; i++) {
>  		uffd_stats[i].cpu = i;
> +		uffd_stats[i].apply_wp = test_uffdio_wp;
>  		uffd_stats[i].missing_faults = 0;
>  		uffd_stats[i].wp_faults = 0;
>  		uffd_stats[i].minor_faults = 0;
> @@ -156,7 +157,7 @@ static void *locking_thread(void *arg)
> 
>  static int copy_page_retry(int ufd, unsigned long offset)
>  {
> -	return __copy_page(ufd, offset, true);
> +	return __copy_page(ufd, offset, true, test_uffdio_wp);
>  }
> 
>  pthread_mutex_t uffd_read_mutex = PTHREAD_MUTEX_INITIALIZER;
> @@ -309,7 +310,7 @@ static void sighndl(int sig, siginfo_t *siginfo, void *ptr)
>   * This also tests UFFD_FEATURE_EVENT_FORK event along with the signal
>   * feature. Using monitor thread, verify no userfault events are generated.
>   */
> -static int faulting_process(int signal_test)
> +static int faulting_process(int signal_test, bool wp)
>  {
>  	unsigned long nr;
>  	unsigned long long count;
> @@ -344,7 +345,7 @@ static int faulting_process(int signal_test)
>  					if (steps == 1) {
>  						/* This is a MISSING request */
>  						steps++;
> -						if (copy_page(uffd, offset))
> +						if (copy_page(uffd, offset, wp))
>  							signalled++;
>  					} else {
>  						/* This is a WP request */
> @@ -508,6 +509,7 @@ static int userfaultfd_events_test(void)
>  			  true, test_uffdio_wp, false))
>  		err("register failure");
> 
> +	stats.apply_wp = test_uffdio_wp;
>  	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
>  		err("uffd_poll_thread create");
> 
> @@ -516,7 +518,7 @@ static int userfaultfd_events_test(void)
>  		err("fork");
> 
>  	if (!pid)
> -		exit(faulting_process(0));
> +		exit(faulting_process(0, test_uffdio_wp));
> 
>  	waitpid(pid, &err, 0);
>  	if (err)
> @@ -552,11 +554,12 @@ static int userfaultfd_sig_test(void)
>  			  true, test_uffdio_wp, false))
>  		err("register failure");
> 
> -	if (faulting_process(1))
> +	if (faulting_process(1, test_uffdio_wp))
>  		err("faulting process failed");
> 
>  	uffd_test_ops->release_pages(area_dst);
> 
> +	stats.apply_wp = test_uffdio_wp;
>  	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
>  		err("uffd_poll_thread create");
> 
> @@ -565,7 +568,7 @@ static int userfaultfd_sig_test(void)
>  		err("fork");
> 
>  	if (!pid)
> -		exit(faulting_process(2));
> +		exit(faulting_process(2, test_uffdio_wp));
> 
>  	waitpid(pid, &err, 0);
>  	if (err)
> @@ -629,6 +632,7 @@ static int userfaultfd_minor_test(void)
>  		       page_size);
>  	}
> 
> +	stats.apply_wp = test_uffdio_wp;
>  	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
>  		err("uffd_poll_thread create");
> 
> -- 
> 2.39.1
>
Peter Xu April 11, 2023, 7:36 p.m. UTC | #2 
On Tue, Apr 11, 2023 at 01:52:58PM +0300, Mike Rapoport wrote:
> On Thu, Mar 30, 2023 at 12:08:12PM -0400, Peter Xu wrote:
> > Subject: selftests/mm: Let uffd_handle_page_fault() takes wp parameter
> 
> Nit:                                                 ^ take
> 
> > Make the handler optionally apply WP bit when resolving page faults for
> > either missing or minor page faults.  This move towards removing global
> 
> Nit:                                        ^ moves

Will fix.

> 
> > test_uffdio_wp outside of the common code.
> > 
> > For this, slightly abuse uffd_stats to keep one more parameter on whether
> > we'd like to resolve page faults with WP bit set.  Note that only the name
> > is abused, it'll be better to be called uffd_args or similar but let's not
> > bother for now.
> 
> Maybe one of the first commits in the series should have been
> s/uffd_stats/uffd_args/g, but I realize that it's PITA, so I won't insist.

It's fine, I'll add one patch.  The rebase does take some more minutes, but
it's manageable.  I'll drop this paragraph then.

Thanks,
diff mbox series

Patch

diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
index 025e40ffc7bf..92b7e00efa8a 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -353,7 +353,7 @@  void wp_range(int ufd, __u64 start, __u64 len, bool wp)
 		err("clear WP failed: address=0x%"PRIx64, (uint64_t)start);
 }
 
-static void continue_range(int ufd, __u64 start, __u64 len)
+static void continue_range(int ufd, __u64 start, __u64 len, bool wp)
 {
 	struct uffdio_continue req;
 	int ret;
@@ -361,7 +361,7 @@  static void continue_range(int ufd, __u64 start, __u64 len)
 	req.range.start = start;
 	req.range.len = len;
 	req.mode = 0;
-	if (test_uffdio_wp)
+	if (wp)
 		req.mode |= UFFDIO_CONTINUE_MODE_WP;
 
 	if (ioctl(ufd, UFFDIO_CONTINUE, &req))
@@ -429,7 +429,8 @@  void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats)
 				    area_dst_alias));
 		for (b = 0; b < page_size; ++b)
 			area[b] = ~area[b];
-		continue_range(uffd, msg->arg.pagefault.address, page_size);
+		continue_range(uffd, msg->arg.pagefault.address, page_size,
+			       stats->apply_wp);
 		stats->minor_faults++;
 	} else {
 		/*
@@ -459,7 +460,7 @@  void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats)
 		offset = (char *)(unsigned long)msg->arg.pagefault.address - area_dst;
 		offset &= ~(page_size-1);
 
-		if (copy_page(uffd, offset))
+		if (copy_page(uffd, offset, stats->apply_wp))
 			stats->missing_faults++;
 	}
 }
@@ -555,7 +556,7 @@  static void wake_range(int ufd, unsigned long addr, unsigned long len)
 			addr), exit(1);
 }
 
-int __copy_page(int ufd, unsigned long offset, bool retry)
+int __copy_page(int ufd, unsigned long offset, bool retry, bool wp)
 {
 	struct uffdio_copy uffdio_copy;
 
@@ -564,7 +565,7 @@  int __copy_page(int ufd, unsigned long offset, bool retry)
 	uffdio_copy.dst = (unsigned long) area_dst + offset;
 	uffdio_copy.src = (unsigned long) area_src + offset;
 	uffdio_copy.len = page_size;
-	if (test_uffdio_wp)
+	if (wp)
 		uffdio_copy.mode = UFFDIO_COPY_MODE_WP;
 	else
 		uffdio_copy.mode = 0;
@@ -587,8 +588,8 @@  int __copy_page(int ufd, unsigned long offset, bool retry)
 	return 0;
 }
 
-int copy_page(int ufd, unsigned long offset)
+int copy_page(int ufd, unsigned long offset, bool wp)
 {
-	return __copy_page(ufd, offset, false);
+	return __copy_page(ufd, offset, false, wp);
 }
 
diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
index 47565b2f2dee..f4bc73ce3b48 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -72,6 +72,8 @@ 
 /* Userfaultfd test statistics */
 struct uffd_stats {
 	int cpu;
+	/* Whether apply wr-protects when installing pages */
+	bool apply_wp;
 	unsigned long missing_faults;
 	unsigned long wp_faults;
 	unsigned long minor_faults;
@@ -104,8 +106,8 @@  void userfaultfd_open(uint64_t *features);
 int uffd_read_msg(int ufd, struct uffd_msg *msg);
 void wp_range(int ufd, __u64 start, __u64 len, bool wp);
 void uffd_handle_page_fault(struct uffd_msg *msg, struct uffd_stats *stats);
-int __copy_page(int ufd, unsigned long offset, bool retry);
-int copy_page(int ufd, unsigned long offset);
+int __copy_page(int ufd, unsigned long offset, bool retry, bool wp);
+int copy_page(int ufd, unsigned long offset, bool wp);
 void *uffd_poll_thread(void *arg);
 
 #define TEST_ANON	1
diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
index 54fc9b4ffa3c..70cb0619354e 100644
--- a/tools/testing/selftests/mm/uffd-stress.c
+++ b/tools/testing/selftests/mm/uffd-stress.c
@@ -97,6 +97,7 @@  static void uffd_stats_reset(struct uffd_stats *uffd_stats,
 
 	for (i = 0; i < n_cpus; i++) {
 		uffd_stats[i].cpu = i;
+		uffd_stats[i].apply_wp = test_uffdio_wp;
 		uffd_stats[i].missing_faults = 0;
 		uffd_stats[i].wp_faults = 0;
 		uffd_stats[i].minor_faults = 0;
@@ -156,7 +157,7 @@  static void *locking_thread(void *arg)
 
 static int copy_page_retry(int ufd, unsigned long offset)
 {
-	return __copy_page(ufd, offset, true);
+	return __copy_page(ufd, offset, true, test_uffdio_wp);
 }
 
 pthread_mutex_t uffd_read_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -309,7 +310,7 @@  static void sighndl(int sig, siginfo_t *siginfo, void *ptr)
  * This also tests UFFD_FEATURE_EVENT_FORK event along with the signal
  * feature. Using monitor thread, verify no userfault events are generated.
  */
-static int faulting_process(int signal_test)
+static int faulting_process(int signal_test, bool wp)
 {
 	unsigned long nr;
 	unsigned long long count;
@@ -344,7 +345,7 @@  static int faulting_process(int signal_test)
 					if (steps == 1) {
 						/* This is a MISSING request */
 						steps++;
-						if (copy_page(uffd, offset))
+						if (copy_page(uffd, offset, wp))
 							signalled++;
 					} else {
 						/* This is a WP request */
@@ -508,6 +509,7 @@  static int userfaultfd_events_test(void)
 			  true, test_uffdio_wp, false))
 		err("register failure");
 
+	stats.apply_wp = test_uffdio_wp;
 	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
 		err("uffd_poll_thread create");
 
@@ -516,7 +518,7 @@  static int userfaultfd_events_test(void)
 		err("fork");
 
 	if (!pid)
-		exit(faulting_process(0));
+		exit(faulting_process(0, test_uffdio_wp));
 
 	waitpid(pid, &err, 0);
 	if (err)
@@ -552,11 +554,12 @@  static int userfaultfd_sig_test(void)
 			  true, test_uffdio_wp, false))
 		err("register failure");
 
-	if (faulting_process(1))
+	if (faulting_process(1, test_uffdio_wp))
 		err("faulting process failed");
 
 	uffd_test_ops->release_pages(area_dst);
 
+	stats.apply_wp = test_uffdio_wp;
 	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
 		err("uffd_poll_thread create");
 
@@ -565,7 +568,7 @@  static int userfaultfd_sig_test(void)
 		err("fork");
 
 	if (!pid)
-		exit(faulting_process(2));
+		exit(faulting_process(2, test_uffdio_wp));
 
 	waitpid(pid, &err, 0);
 	if (err)
@@ -629,6 +632,7 @@  static int userfaultfd_minor_test(void)
 		       page_size);
 	}
 
+	stats.apply_wp = test_uffdio_wp;
 	if (pthread_create(&uffd_mon, &attr, uffd_poll_thread, &stats))
 		err("uffd_poll_thread create");