[PATCH 0/4] selftests/vm: Drop hugetlb mntpoint in run_vmtests.sh

[PATCH 0/4] selftests/vm: Drop hugetlb mntpoint in run_vmtests.sh

[Peter Xu]

Since I started working on patch 1 when debugging the race anyway, I
cleaned it up so we can use the same memfd for both hugetlb and shmem which
is cleaner.

Then I figured it's not hard if with patch 1 to replace all the file-based
test to use memfd for hugetlbb so I did.  Then patch 4 dropped the hugetlb
mntpoint for run_vmtests.sh.

Please have a look, thanks.

Peter Xu (4):
  selftests/vm: Use memfd for hugetlb tests
  selftests/vm: Use memfd for hugetlb-madvise test
  selftests/vm: Use memfd for hugepage-mremap test
  selftests/vm: Drop mnt point for hugetlb in run_vmtests.sh

 tools/testing/selftests/vm/hugepage-mremap.c | 21 +++----
 tools/testing/selftests/vm/hugetlb-madvise.c | 12 +---
 tools/testing/selftests/vm/run_vmtests.sh    | 18 +-----
 tools/testing/selftests/vm/userfaultfd.c     | 60 +++++++-------------
 4 files changed, 35 insertions(+), 76 deletions(-)

-- 
2.37.3

[PATCH 1/4] selftests/vm: Use memfd for hugetlb tests

[Peter Xu]

We already used memfd for shmem test, move it forward with hugetlb too so
that we don't need user to specify the hugetlb file path explicitly when
running hugetlb shared tests.

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

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 74babdbc02e5..c0c6853cdce5 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -93,10 +93,8 @@ static volatile bool test_uffdio_zeropage_eexist = true;
 static bool test_uffdio_wp = true;
 /* Whether to test uffd minor faults */
 static bool test_uffdio_minor = false;
-
 static bool map_shared;
-static int shm_fd;
-static int huge_fd;
+static int mem_fd;
 static unsigned long long *count_verify;
 static int uffd = -1;
 static int uffd_flags, finished, *pipefd;
@@ -260,35 +258,21 @@ static void hugetlb_release_pages(char *rel_area)
 
 static void hugetlb_allocate_area(void **alloc_area, bool is_src)
 {
+	off_t size = nr_pages * page_size;
+	off_t offset = is_src ? 0 : size;
 	void *area_alias = NULL;
 	char **alloc_area_alias;
 
-	if (!map_shared)
-		*alloc_area = mmap(NULL,
-			nr_pages * page_size,
-			PROT_READ | PROT_WRITE,
-			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB |
-				(is_src ? 0 : MAP_NORESERVE),
-			-1,
-			0);
-	else
-		*alloc_area = mmap(NULL,
-			nr_pages * page_size,
-			PROT_READ | PROT_WRITE,
-			MAP_SHARED |
-				(is_src ? 0 : MAP_NORESERVE),
-			huge_fd,
-			is_src ? 0 : nr_pages * page_size);
+	*alloc_area = mmap(NULL, size, PROT_READ | PROT_WRITE,
+			   (map_shared ? MAP_SHARED : MAP_PRIVATE) |
+			   (is_src ? 0 : MAP_NORESERVE),
+			   mem_fd, offset);
 	if (*alloc_area == MAP_FAILED)
 		err("mmap of hugetlbfs file failed");
 
 	if (map_shared) {
-		area_alias = mmap(NULL,
-			nr_pages * page_size,
-			PROT_READ | PROT_WRITE,
-			MAP_SHARED,
-			huge_fd,
-			is_src ? 0 : nr_pages * page_size);
+		area_alias = mmap(NULL, size, PROT_READ | PROT_WRITE,
+				  MAP_SHARED, mem_fd, offset);
 		if (area_alias == MAP_FAILED)
 			err("mmap of hugetlb file alias failed");
 	}
@@ -334,14 +318,14 @@ static void shmem_allocate_area(void **alloc_area, bool is_src)
 	}
 
 	*alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
-			   shm_fd, offset);
+			   mem_fd, offset);
 	if (*alloc_area == MAP_FAILED)
 		err("mmap of memfd failed");
 	if (test_collapse && *alloc_area != p)
 		err("mmap of memfd failed at %p", p);
 
 	area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
-			  shm_fd, offset);
+			  mem_fd, offset);
 	if (area_alias == MAP_FAILED)
 		err("mmap of memfd alias failed");
 	if (test_collapse && area_alias != p_alias)
@@ -1821,21 +1805,17 @@ int main(int argc, char **argv)
 	}
 	nr_pages = nr_pages_per_cpu * nr_cpus;
 
-	if (test_type == TEST_HUGETLB && map_shared) {
-		if (argc < 5)
-			usage();
-		huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
-		if (huge_fd < 0)
-			err("Open of %s failed", argv[4]);
-		if (ftruncate(huge_fd, 0))
-			err("ftruncate %s to size 0 failed", argv[4]);
-	} else if (test_type == TEST_SHMEM) {
-		shm_fd = memfd_create(argv[0], 0);
-		if (shm_fd < 0)
+	if (test_type == TEST_SHMEM || test_type == TEST_HUGETLB) {
+		unsigned int memfd_flags = 0;
+
+		if (test_type == TEST_HUGETLB)
+			memfd_flags = MFD_HUGETLB;
+		mem_fd = memfd_create(argv[0], memfd_flags);
+		if (mem_fd < 0)
 			err("memfd_create");
-		if (ftruncate(shm_fd, nr_pages * page_size * 2))
+		if (ftruncate(mem_fd, nr_pages * page_size * 2))
 			err("ftruncate");
-		if (fallocate(shm_fd,
+		if (fallocate(mem_fd,
 			      FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0,
 			      nr_pages * page_size * 2))
 			err("fallocate");
-- 
2.37.3

[PATCH 2/4] selftests/vm: Use memfd for hugetlb-madvise test

[Peter Xu]

For dropping the hugetlb mountpoint in run_vmtests.sh.  Since no parameter
is needed, drop USAGE too.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/hugetlb-madvise.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/vm/hugetlb-madvise.c b/tools/testing/selftests/vm/hugetlb-madvise.c
index 3c9943131881..f96435b70986 100644
--- a/tools/testing/selftests/vm/hugetlb-madvise.c
+++ b/tools/testing/selftests/vm/hugetlb-madvise.c
@@ -12,6 +12,7 @@
  * directory.
  */
 
+#define _GNU_SOURCE
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
@@ -19,7 +20,6 @@
 #define __USE_GNU
 #include <fcntl.h>
 
-#define USAGE	"USAGE: %s <hugepagefile_name>\n"
 #define MIN_FREE_PAGES	20
 #define NR_HUGE_PAGES	10	/* common number of pages to map/allocate */
 
@@ -103,11 +103,6 @@ int main(int argc, char **argv)
 	int fd;
 	int ret;
 
-	if (argc != 2) {
-		printf(USAGE, argv[0]);
-		exit(1);
-	}
-
 	huge_page_size = default_huge_page_size();
 	if (!huge_page_size) {
 		printf("Unable to determine huge page size, exiting!\n");
@@ -125,9 +120,9 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	fd = open(argv[1], O_CREAT | O_RDWR, 0755);
+	fd = memfd_create(argv[0], MFD_HUGETLB);
 	if (fd < 0) {
-		perror("Open failed");
+		perror("memfd_create() failed");
 		exit(1);
 	}
 
@@ -406,6 +401,5 @@ int main(int argc, char **argv)
 	(void)munmap(addr2, NR_HUGE_PAGES * huge_page_size);
 
 	close(fd);
-	unlink(argv[1]);
 	return 0;
 }
-- 
2.37.3

[PATCH 3/4] selftests/vm: Use memfd for hugepage-mremap test

[Peter Xu]

For dropping the hugetlb mountpoint in run_vmtests.sh.  Cleaned it up a
little bit around the changed codes.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/hugepage-mremap.c | 21 +++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/vm/hugepage-mremap.c b/tools/testing/selftests/vm/hugepage-mremap.c
index e63a0214f639..e53b5eaa8fce 100644
--- a/tools/testing/selftests/vm/hugepage-mremap.c
+++ b/tools/testing/selftests/vm/hugepage-mremap.c
@@ -22,6 +22,7 @@
 #include <sys/syscall.h> /* Definition of SYS_* constants */
 #include <linux/userfaultfd.h>
 #include <sys/ioctl.h>
+#include <string.h>
 
 #define DEFAULT_LENGTH_MB 10UL
 #define MB_TO_BYTES(x) (x * 1024 * 1024)
@@ -108,26 +109,23 @@ static void register_region_with_uffd(char *addr, size_t len)
 int main(int argc, char *argv[])
 {
 	size_t length = 0;
+	int ret = 0, fd;
 
-	if (argc != 2 && argc != 3) {
-		printf("Usage: %s [length_in_MB] <hugetlb_file>\n", argv[0]);
+	if (argc >= 2 && !strcmp(argv[1], "-h")) {
+		printf("Usage: %s [length_in_MB]\n", argv[0]);
 		exit(1);
 	}
 
 	/* Read memory length as the first arg if valid, otherwise fallback to
 	 * the default length.
 	 */
-	if (argc == 3)
-		length = argc > 2 ? (size_t)atoi(argv[1]) : 0UL;
+	if (argc >= 2)
+		length = (size_t)atoi(argv[1]);
+	else
+		length = DEFAULT_LENGTH_MB;
 
-	length = length > 0 ? length : DEFAULT_LENGTH_MB;
 	length = MB_TO_BYTES(length);
-
-	int ret = 0;
-
-	/* last arg is the hugetlb file name */
-	int fd = open(argv[argc-1], O_CREAT | O_RDWR, 0755);
-
+	fd = memfd_create(argv[0], MFD_HUGETLB);
 	if (fd < 0) {
 		perror("Open failed");
 		exit(1);
@@ -185,7 +183,6 @@ int main(int argc, char *argv[])
 	}
 
 	close(fd);
-	unlink(argv[argc-1]);
 
 	return ret;
 }
-- 
2.37.3

[PATCH 4/4] selftests/vm: Drop mnt point for hugetlb in run_vmtests.sh

[Peter Xu]

After converting all the three relevant testcases (uffd, madvise, mremap)
to use memfd, no test will need the hugetlb mount point anymore.  Drop the
code.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/vm/run_vmtests.sh | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh
index e780e76c26b8..0dc9f545a32d 100755
--- a/tools/testing/selftests/vm/run_vmtests.sh
+++ b/tools/testing/selftests/vm/run_vmtests.sh
@@ -5,7 +5,6 @@
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
-mnt=./huge
 exitcode=0
 
 #get huge pagesize and freepages from /proc/meminfo
@@ -84,9 +83,6 @@ run_test() {
 	fi
 }
 
-mkdir "$mnt"
-mount -t hugetlbfs none "$mnt"
-
 run_test ./hugepage-mmap
 
 shmmax=$(cat /proc/sys/kernel/shmmax)
@@ -98,14 +94,9 @@ echo "$shmmax" > /proc/sys/kernel/shmmax
 echo "$shmall" > /proc/sys/kernel/shmall
 
 run_test ./map_hugetlb
-
-run_test ./hugepage-mremap "$mnt"/huge_mremap
-rm -f "$mnt"/huge_mremap
-
+run_test ./hugepage-mremap
 run_test ./hugepage-vmemmap
-
-run_test ./hugetlb-madvise "$mnt"/madvise-test
-rm -f "$mnt"/madvise-test
+run_test ./hugetlb-madvise
 
 echo "NOTE: The above hugetlb tests provide minimal coverage.  Use"
 echo "      https://github.com/libhugetlbfs/libhugetlbfs.git for"
@@ -126,14 +117,11 @@ for mod in "${uffd_mods[@]}"; do
 	# Hugetlb tests require source and destination huge pages. Pass in half
 	# the size ($half_ufd_size_MB), which is used for *each*.
 	run_test ./userfaultfd hugetlb${mod} "$half_ufd_size_MB" 32
-	run_test ./userfaultfd hugetlb_shared${mod} "$half_ufd_size_MB" 32 "$mnt"/uffd-test
-	rm -f "$mnt"/uffd-test
+	run_test ./userfaultfd hugetlb_shared${mod} "$half_ufd_size_MB" 32
 	run_test ./userfaultfd shmem${mod} 20 16
 done
 
 #cleanup
-umount "$mnt"
-rm -rf "$mnt"
 echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
 
 run_test ./compaction_test
-- 
2.37.3

Re: [PATCH 1/4] selftests/vm: Use memfd for hugetlb tests

[Axel Rasmussen]

On Tue, Oct 4, 2022 at 2:41 PM Peter Xu <peterx@redhat.com> wrote:
>
> We already used memfd for shmem test, move it forward with hugetlb too so
> that we don't need user to specify the hugetlb file path explicitly when
> running hugetlb shared tests.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tools/testing/selftests/vm/userfaultfd.c | 60 ++++++++----------------
>  1 file changed, 20 insertions(+), 40 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 74babdbc02e5..c0c6853cdce5 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -93,10 +93,8 @@ static volatile bool test_uffdio_zeropage_eexist = true;
>  static bool test_uffdio_wp = true;
>  /* Whether to test uffd minor faults */
>  static bool test_uffdio_minor = false;
> -
>  static bool map_shared;
> -static int shm_fd;
> -static int huge_fd;
> +static int mem_fd;
>  static unsigned long long *count_verify;
>  static int uffd = -1;
>  static int uffd_flags, finished, *pipefd;
> @@ -260,35 +258,21 @@ static void hugetlb_release_pages(char *rel_area)
>
>  static void hugetlb_allocate_area(void **alloc_area, bool is_src)
>  {
> +       off_t size = nr_pages * page_size;
> +       off_t offset = is_src ? 0 : size;
>         void *area_alias = NULL;
>         char **alloc_area_alias;
>
> -       if (!map_shared)
> -               *alloc_area = mmap(NULL,
> -                       nr_pages * page_size,
> -                       PROT_READ | PROT_WRITE,
> -                       MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB |
> -                               (is_src ? 0 : MAP_NORESERVE),
> -                       -1,
> -                       0);
> -       else
> -               *alloc_area = mmap(NULL,
> -                       nr_pages * page_size,
> -                       PROT_READ | PROT_WRITE,
> -                       MAP_SHARED |
> -                               (is_src ? 0 : MAP_NORESERVE),
> -                       huge_fd,
> -                       is_src ? 0 : nr_pages * page_size);
> +       *alloc_area = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +                          (map_shared ? MAP_SHARED : MAP_PRIVATE) |
> +                          (is_src ? 0 : MAP_NORESERVE),
> +                          mem_fd, offset);
>         if (*alloc_area == MAP_FAILED)
>                 err("mmap of hugetlbfs file failed");
>
>         if (map_shared) {
> -               area_alias = mmap(NULL,
> -                       nr_pages * page_size,
> -                       PROT_READ | PROT_WRITE,
> -                       MAP_SHARED,
> -                       huge_fd,
> -                       is_src ? 0 : nr_pages * page_size);
> +               area_alias = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +                                 MAP_SHARED, mem_fd, offset);
>                 if (area_alias == MAP_FAILED)
>                         err("mmap of hugetlb file alias failed");
>         }
> @@ -334,14 +318,14 @@ static void shmem_allocate_area(void **alloc_area, bool is_src)
>         }
>
>         *alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
> -                          shm_fd, offset);
> +                          mem_fd, offset);
>         if (*alloc_area == MAP_FAILED)
>                 err("mmap of memfd failed");
>         if (test_collapse && *alloc_area != p)
>                 err("mmap of memfd failed at %p", p);
>
>         area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
> -                         shm_fd, offset);
> +                         mem_fd, offset);
>         if (area_alias == MAP_FAILED)
>                 err("mmap of memfd alias failed");
>         if (test_collapse && area_alias != p_alias)
> @@ -1821,21 +1805,17 @@ int main(int argc, char **argv)
>         }
>         nr_pages = nr_pages_per_cpu * nr_cpus;
>
> -       if (test_type == TEST_HUGETLB && map_shared) {
> -               if (argc < 5)
> -                       usage();
> -               huge_fd = open(argv[4], O_CREAT | O_RDWR, 0755);
> -               if (huge_fd < 0)
> -                       err("Open of %s failed", argv[4]);
> -               if (ftruncate(huge_fd, 0))
> -                       err("ftruncate %s to size 0 failed", argv[4]);

We should also update the examples / help text near the top of the
file, since we're changing what arguments this accepts.

It might also be better to squash the changes to test arguments in
run_vmtests.sh into each patch, so if we're bisecting we'll have a
matching / working test + run_vmtests.sh combo at each commit.

> -       } else if (test_type == TEST_SHMEM) {
> -               shm_fd = memfd_create(argv[0], 0);
> -               if (shm_fd < 0)
> +       if (test_type == TEST_SHMEM || test_type == TEST_HUGETLB) {
> +               unsigned int memfd_flags = 0;
> +
> +               if (test_type == TEST_HUGETLB)
> +                       memfd_flags = MFD_HUGETLB;
> +               mem_fd = memfd_create(argv[0], memfd_flags);
> +               if (mem_fd < 0)
>                         err("memfd_create");
> -               if (ftruncate(shm_fd, nr_pages * page_size * 2))
> +               if (ftruncate(mem_fd, nr_pages * page_size * 2))
>                         err("ftruncate");
> -               if (fallocate(shm_fd,
> +               if (fallocate(mem_fd,
>                               FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0,
>                               nr_pages * page_size * 2))
>                         err("fallocate");
> --
> 2.37.3
>

This is a nice simplification! Thanks for doing it. Besides the
comments above, the rest of the code here looks correct to me. Feel
free to take:

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

Re: [PATCH 4/4] selftests/vm: Drop mnt point for hugetlb in run_vmtests.sh

[Axel Rasmussen]

On Tue, Oct 4, 2022 at 2:43 PM Peter Xu <peterx@redhat.com> wrote:
>
> After converting all the three relevant testcases (uffd, madvise, mremap)
> to use memfd, no test will need the hugetlb mount point anymore.  Drop the
> code.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tools/testing/selftests/vm/run_vmtests.sh | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/run_vmtests.sh b/tools/testing/selftests/vm/run_vmtests.sh
> index e780e76c26b8..0dc9f545a32d 100755
> --- a/tools/testing/selftests/vm/run_vmtests.sh
> +++ b/tools/testing/selftests/vm/run_vmtests.sh
> @@ -5,7 +5,6 @@
>  # Kselftest framework requirement - SKIP code is 4.
>  ksft_skip=4
>
> -mnt=./huge
>  exitcode=0
>
>  #get huge pagesize and freepages from /proc/meminfo
> @@ -84,9 +83,6 @@ run_test() {
>         fi
>  }
>
> -mkdir "$mnt"
> -mount -t hugetlbfs none "$mnt"
> -
>  run_test ./hugepage-mmap
>
>  shmmax=$(cat /proc/sys/kernel/shmmax)
> @@ -98,14 +94,9 @@ echo "$shmmax" > /proc/sys/kernel/shmmax
>  echo "$shmall" > /proc/sys/kernel/shmall
>
>  run_test ./map_hugetlb
> -
> -run_test ./hugepage-mremap "$mnt"/huge_mremap
> -rm -f "$mnt"/huge_mremap
> -
> +run_test ./hugepage-mremap
>  run_test ./hugepage-vmemmap
> -
> -run_test ./hugetlb-madvise "$mnt"/madvise-test
> -rm -f "$mnt"/madvise-test
> +run_test ./hugetlb-madvise
>
>  echo "NOTE: The above hugetlb tests provide minimal coverage.  Use"
>  echo "      https://github.com/libhugetlbfs/libhugetlbfs.git for"
> @@ -126,14 +117,11 @@ for mod in "${uffd_mods[@]}"; do
>         # Hugetlb tests require source and destination huge pages. Pass in half
>         # the size ($half_ufd_size_MB), which is used for *each*.
>         run_test ./userfaultfd hugetlb${mod} "$half_ufd_size_MB" 32
> -       run_test ./userfaultfd hugetlb_shared${mod} "$half_ufd_size_MB" 32 "$mnt"/uffd-test
> -       rm -f "$mnt"/uffd-test
> +       run_test ./userfaultfd hugetlb_shared${mod} "$half_ufd_size_MB" 32
>         run_test ./userfaultfd shmem${mod} 20 16
>  done
>
>  #cleanup
> -umount "$mnt"
> -rm -rf "$mnt"
>  echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>
>  run_test ./compaction_test
> --
> 2.37.3
>

Feel free to take:

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

Re: [PATCH 1/4] selftests/vm: Use memfd for hugetlb tests

[Peter Xu]

On Tue, Oct 04, 2022 at 03:04:23PM -0700, Axel Rasmussen wrote:
> We should also update the examples / help text near the top of the
> file, since we're changing what arguments this accepts.

Definitely, I missed that.

> 
> It might also be better to squash the changes to test arguments in
> run_vmtests.sh into each patch, so if we're bisecting we'll have a
> matching / working test + run_vmtests.sh combo at each commit.

I can do that, but may not be required - note that I did the first three
patches just to make sure they're still compatible with the old cmdlines,
by ignoring the extra params.

[...]

> This is a nice simplification! Thanks for doing it. Besides the
> comments above, the rest of the code here looks correct to me. Feel
> free to take:
> 
> Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>

Thanks!

-- 
Peter Xu

Re: [PATCH 1/4] selftests/vm: Use memfd for hugetlb tests

[Axel Rasmussen]

On Wed, Oct 5, 2022 at 7:44 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Tue, Oct 04, 2022 at 03:04:23PM -0700, Axel Rasmussen wrote:
> > We should also update the examples / help text near the top of the
> > file, since we're changing what arguments this accepts.
>
> Definitely, I missed that.
>
> >
> > It might also be better to squash the changes to test arguments in
> > run_vmtests.sh into each patch, so if we're bisecting we'll have a
> > matching / working test + run_vmtests.sh combo at each commit.
>
> I can do that, but may not be required - note that I did the first three
> patches just to make sure they're still compatible with the old cmdlines,
> by ignoring the extra params.

Ah! In that case, forget it. :)

>
> [...]
>
> > This is a nice simplification! Thanks for doing it. Besides the
> > comments above, the rest of the code here looks correct to me. Feel
> > free to take:
> >
> > Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
>
> Thanks!
>
> --
> Peter Xu
>