On Wed, May 19, 2021 at 3:14 PM Axel Rasmussen [off-list ref] wrote:

On Wed, May 19, 2021 at 2:45 PM Ben Gardon [off-list ref] wrote:

quoted

On Wed, May 19, 2021 at 1:03 PM Axel Rasmussen [off-list ref] wrote:

quoted

A small cleanup. Our caller writes:

  r = setup_demand_paging(...);
  if (r < 0) exit(-r);

Since we're just going to exit anyway, instead of returning an error we
can just re-use TEST_ASSERT. This makes the caller simpler, as well as
the function itself - no need to write our branches, etc.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>

Reviewed-by: Ben Gardon <redacted>

quoted

quoted

---
 .../selftests/kvm/demand_paging_test.c        | 51 +++++++------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 9398ba6ef023..601a1df24dd2 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c

@@ -9,6 +9,8 @@

 #define _GNU_SOURCE /* for pipe2 */

+#include <inttypes.h>
+#include <stdint.h>

Why do the includes need to change in this commit? Is it for the PRIu64 below?

Right, I didn't actually try compiling without these, but inttypes.h
defines PRIu64 and stdint.h defines uint64_t. In general I tend to
prefer including things like this because we're using their
definitions directly, even if we might be picking them up transiently
some other way.

Makes sense to me.

quoted

quoted

 #include <stdio.h>
 #include <stdlib.h>
 #include <time.h>

@@ -198,42 +200,32 @@ static void *uffd_handler_thread_fn(void *arg)
        return NULL;
 }

-static int setup_demand_paging(struct kvm_vm *vm,
-                              pthread_t *uffd_handler_thread, int pipefd,
-                              useconds_t uffd_delay,
-                              struct uffd_handler_args *uffd_args,
-                              void *hva, uint64_t len)
+static void setup_demand_paging(struct kvm_vm *vm,
+                               pthread_t *uffd_handler_thread, int pipefd,
+                               useconds_t uffd_delay,
+                               struct uffd_handler_args *uffd_args,
+                               void *hva, uint64_t len)
 {
        int uffd;
        struct uffdio_api uffdio_api;
        struct uffdio_register uffdio_register;

        uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
-       if (uffd == -1) {
-               pr_info("uffd creation failed\n");
-               return -1;
-       }
+       TEST_ASSERT(uffd >= 0, "uffd creation failed, errno: %d", errno);

        uffdio_api.api = UFFD_API;
        uffdio_api.features = 0;
-       if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
-               pr_info("ioctl uffdio_api failed\n");
-               return -1;
-       }
+       TEST_ASSERT(ioctl(uffd, UFFDIO_API, &uffdio_api) != -1,
+                   "ioctl UFFDIO_API failed: %" PRIu64,
+                   (uint64_t)uffdio_api.api);

        uffdio_register.range.start = (uint64_t)hva;
        uffdio_register.range.len = len;
        uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
-       if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
-               pr_info("ioctl uffdio_register failed\n");
-               return -1;
-       }
-
-       if ((uffdio_register.ioctls & UFFD_API_RANGE_IOCTLS) !=
-                       UFFD_API_RANGE_IOCTLS) {
-               pr_info("unexpected userfaultfd ioctl set\n");
-               return -1;
-       }
+       TEST_ASSERT(ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) != -1,
+                   "ioctl UFFDIO_REGISTER failed");
+       TEST_ASSERT((uffdio_register.ioctls & UFFD_API_RANGE_IOCTLS) ==
+                   UFFD_API_RANGE_IOCTLS, "unexpected userfaultfd ioctl set");

        uffd_args->uffd = uffd;
        uffd_args->pipefd = pipefd;

@@ -243,8 +235,6 @@ static int setup_demand_paging(struct kvm_vm *vm,

        PER_VCPU_DEBUG("Created uffd thread for HVA range [%p, %p)\n",
                       hva, hva + len);
-
-       return 0;
 }

 struct test_params {

@@ -321,13 +311,10 @@ static void run_test(enum vm_guest_mode mode, void *arg)
                                  O_CLOEXEC | O_NONBLOCK);
                        TEST_ASSERT(!r, "Failed to set up pipefd");

-                       r = setup_demand_paging(vm,
-                                               &uffd_handler_threads[vcpu_id],
-                                               pipefds[vcpu_id * 2],
-                                               p->uffd_delay, &uffd_args[vcpu_id],
-                                               vcpu_hva, vcpu_mem_size);
-                       if (r < 0)
-                               exit(-r);
+                       setup_demand_paging(vm, &uffd_handler_threads[vcpu_id],
+                                           pipefds[vcpu_id * 2], p->uffd_delay,
+                                           &uffd_args[vcpu_id], vcpu_hva,
+                                           vcpu_mem_size);
                }
        }

--

2.31.1.751.gd2f1c929bd-goog