Re: [PATCH v2 02/10] KVM: selftests: simplify setup_demand_paging error handling

On Wed, May 19, 2021 at 1:03 PM Axel Rasmussen axelrasmussen@google.com wrote:

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

.../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?

#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