[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6d1a298f-6af9-4568-b171-cb4a83121a95@arm.com>
Date: Thu, 20 Feb 2025 21:02:26 +0530
From: Dev Jain <dev.jain@....com>
To: Brendan Jackman <jackmanb@...gle.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>
Cc: Mateusz Guzik <mjguzik@...il.com>, linux-mm@...ck.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] selftests/mm: Report errno when things fail
On 20/02/25 8:33 pm, Brendan Jackman wrote:
> Just reporting failure doesn't tell you what went wrong. This can fail
> in different ways so report errno to help the reader get started
> debugging.
IMHO it would be even better if we reported strerror(errno).
>
> Signed-off-by: Brendan Jackman <jackmanb@...gle.com>
> ---
> tools/testing/selftests/mm/gup_longterm.c | 32 +++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c
> index 9423ad439a6140163bdef2974615bb86406a8c14..46a2139b3a646f6c050eb031a770f615be76c433 100644
> --- a/tools/testing/selftests/mm/gup_longterm.c
> +++ b/tools/testing/selftests/mm/gup_longterm.c
> @@ -96,13 +96,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> int ret;
>
> if (ftruncate(fd, size)) {
> - ksft_test_result_fail("ftruncate() failed\n");
> + ksft_test_result_fail("ftruncate() failed (%d)\n", errno);
> return;
> }
>
> if (fallocate(fd, 0, 0, size)) {
> if (size == pagesize)
> - ksft_test_result_fail("fallocate() failed\n");
> + ksft_test_result_fail("fallocate() failed (%d)\n", errno);
> else
> ksft_test_result_skip("need more free huge pages\n");
> return;
> @@ -112,7 +112,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> shared ? MAP_SHARED : MAP_PRIVATE, fd, 0);
> if (mem == MAP_FAILED) {
> if (size == pagesize || shared)
> - ksft_test_result_fail("mmap() failed\n");
> + ksft_test_result_fail("mmap() failed (%d)\n", errno);
> else
> ksft_test_result_skip("need more free huge pages\n");
> return;
> @@ -130,7 +130,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> */
> ret = mprotect(mem, size, PROT_READ);
> if (ret) {
> - ksft_test_result_fail("mprotect() failed\n");
> + ksft_test_result_fail("mprotect() failed (%d)\n", errno);
> goto munmap;
> }
> /* FALLTHROUGH */
> @@ -165,18 +165,18 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> args.flags |= rw ? PIN_LONGTERM_TEST_FLAG_USE_WRITE : 0;
> ret = ioctl(gup_fd, PIN_LONGTERM_TEST_START, &args);
> if (ret && errno == EINVAL) {
> - ksft_test_result_skip("PIN_LONGTERM_TEST_START failed\n");
> + ksft_test_result_skip("PIN_LONGTERM_TEST_START failed (EINVAL)n");
> break;
> } else if (ret && errno == EFAULT) {
> ksft_test_result(!should_work, "Should have failed\n");
> break;
> } else if (ret) {
> - ksft_test_result_fail("PIN_LONGTERM_TEST_START failed\n");
> + ksft_test_result_fail("PIN_LONGTERM_TEST_START failed (%d)\n", errno);
> break;
> }
>
> if (ioctl(gup_fd, PIN_LONGTERM_TEST_STOP))
> - ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed\n");
> + ksft_print_msg("[INFO] PIN_LONGTERM_TEST_STOP failed (%d)\n", errno);
>
> /*
> * TODO: if the kernel ever supports long-term R/W pinning on
> @@ -202,7 +202,7 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> /* Skip on errors, as we might just lack kernel support. */
> ret = io_uring_queue_init(1, &ring, 0);
> if (ret < 0) {
> - ksft_test_result_skip("io_uring_queue_init() failed\n");
> + ksft_test_result_skip("io_uring_queue_init() failed (%d)\n", errno);
> break;
> }
> /*
> @@ -215,13 +215,13 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared)
> /* Only new kernels return EFAULT. */
> if (ret && (errno == ENOSPC || errno == EOPNOTSUPP ||
> errno == EFAULT)) {
> - ksft_test_result(!should_work, "Should have failed\n");
> + ksft_test_result(!should_work, "Should have failed (%d)\n", errno);
> } else if (ret) {
> /*
> * We might just lack support or have insufficient
> * MEMLOCK limits.
> */
> - ksft_test_result_skip("io_uring_register_buffers() failed\n");
> + ksft_test_result_skip("io_uring_register_buffers() failed (%d)\n", errno);
> } else {
> ksft_test_result(should_work, "Should have worked\n");
> io_uring_unregister_buffers(&ring);
> @@ -249,7 +249,7 @@ static void run_with_memfd(test_fn fn, const char *desc)
>
> fd = memfd_create("test", 0);
> if (fd < 0) {
> - ksft_test_result_fail("memfd_create() failed\n");
> + ksft_test_result_fail("memfd_create() failed (%d)\n", errno);
> return;
> }
>
> @@ -266,13 +266,13 @@ static void run_with_tmpfile(test_fn fn, const char *desc)
>
> file = tmpfile();
> if (!file) {
> - ksft_test_result_fail("tmpfile() failed\n");
> + ksft_test_result_fail("tmpfile() failed (%d)\n", errno);
> return;
> }
>
> fd = fileno(file);
> if (fd < 0) {
> - ksft_test_result_fail("fileno() failed\n");
> + ksft_test_result_fail("fileno() failed (%d)\n", errno);
> goto close;
> }
>
> @@ -290,12 +290,12 @@ static void run_with_local_tmpfile(test_fn fn, const char *desc)
>
> fd = mkstemp(filename);
> if (fd < 0) {
> - ksft_test_result_fail("mkstemp() failed\n");
> + ksft_test_result_fail("mkstemp() failed (%d)\n", errno);
> return;
> }
>
> if (unlink(filename)) {
> - ksft_test_result_fail("unlink() failed\n");
> + ksft_test_result_fail("unlink() failed (%d)\n", errno);
> goto close;
> }
>
> @@ -317,7 +317,7 @@ static void run_with_memfd_hugetlb(test_fn fn, const char *desc,
>
> fd = memfd_create("test", flags);
> if (fd < 0) {
> - ksft_test_result_skip("memfd_create() failed\n");
> + ksft_test_result_skip("memfd_create() failed (%d)\n", errno);
> return;
> }
>
>
Powered by blists - more mailing lists