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

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