[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a1a6f5dd-80ab-4d53-90de-eb003c996cc1@linuxfoundation.org>
Date: Tue, 26 Mar 2024 14:00:29 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>,
Shuah Khan <shuah@...nel.org>
Cc: kernel@...labora.com, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 1/2] selftests: x86: test_vsyscall: conform test to TAP
format output
On 3/14/24 04:32, Muhammad Usama Anjum wrote:
> Conform the layout, informational and status messages to TAP. No
> functional change is intended other than the layout of output messages.
> Without using TAP messages, the passed/failed/skip test names cannot be
> found.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> ---
I am seeing lot more changes than conform and formatting changes.
> tools/testing/selftests/x86/test_vsyscall.c | 506 +++++++++-----------
> 1 file changed, 238 insertions(+), 268 deletions(-)
>
> diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
> index 47cab972807c4..d4c8e8d79d389 100644
> --- a/tools/testing/selftests/x86/test_vsyscall.c
> +++ b/tools/testing/selftests/x86/test_vsyscall.c
> @@ -21,6 +21,13 @@
> #include <sys/uio.h>
>
> #include "helpers.h"
> +#include "../kselftest.h"
> +
> +#ifdef __x86_64__
> +#define TOTAL_TESTS 13
> +#else
> +#define TOTAL_TESTS 8
> +#endif
>
> #ifdef __x86_64__
> # define VSYS(x) (x)
> @@ -39,18 +46,6 @@
> /* max length of lines in /proc/self/maps - anything longer is skipped here */
> #define MAPS_LINE_LEN 128
>
> -static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
> - int flags)
> -{
> - struct sigaction sa;
> - memset(&sa, 0, sizeof(sa));
> - sa.sa_sigaction = handler;
> - sa.sa_flags = SA_SIGINFO | flags;
> - sigemptyset(&sa.sa_mask);
> - if (sigaction(sig, &sa, 0))
> - err(1, "sigaction");
> -}
> -
Why is this deleted?
> /* vsyscalls and vDSO */
> bool vsyscall_map_r = false, vsyscall_map_x = false;
>
> @@ -75,83 +70,25 @@ static void init_vdso(void)
> if (!vdso)
> vdso = dlopen("linux-gate.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
> if (!vdso) {
> - printf("[WARN]\tfailed to find vDSO\n");
> + ksft_print_msg("[WARN] failed to find vDSO\n");
> return;
> }
>
> vdso_gtod = (gtod_t)dlsym(vdso, "__vdso_gettimeofday");
> if (!vdso_gtod)
> - printf("[WARN]\tfailed to find gettimeofday in vDSO\n");
> + ksft_print_msg("[WARN] failed to find gettimeofday in vDSO\n");
>
> vdso_gettime = (vgettime_t)dlsym(vdso, "__vdso_clock_gettime");
> if (!vdso_gettime)
> - printf("[WARN]\tfailed to find clock_gettime in vDSO\n");
> + ksft_print_msg("[WARN] failed to find clock_gettime in vDSO\n");
>
> vdso_time = (time_func_t)dlsym(vdso, "__vdso_time");
> if (!vdso_time)
> - printf("[WARN]\tfailed to find time in vDSO\n");
> + ksft_print_msg("[WARN] failed to find time in vDSO\n");
>
> vdso_getcpu = (getcpu_t)dlsym(vdso, "__vdso_getcpu");
> if (!vdso_getcpu)
> - printf("[WARN]\tfailed to find getcpu in vDSO\n");
> -}
> -
> -static int init_vsys(void)
> -{
> -#ifdef __x86_64__
> - int nerrs = 0;
> - FILE *maps;
> - char line[MAPS_LINE_LEN];
> - bool found = false;
> -
> - maps = fopen("/proc/self/maps", "r");
> - if (!maps) {
> - printf("[WARN]\tCould not open /proc/self/maps -- assuming vsyscall is r-x\n");
> - vsyscall_map_r = true;
> - return 0;
> - }
> -
> - while (fgets(line, MAPS_LINE_LEN, maps)) {
> - char r, x;
> - void *start, *end;
> - char name[MAPS_LINE_LEN];
> -
> - /* sscanf() is safe here as strlen(name) >= strlen(line) */
> - if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",
> - &start, &end, &r, &x, name) != 5)
> - continue;
> -
> - if (strcmp(name, "[vsyscall]"))
> - continue;
> -
> - printf("\tvsyscall map: %s", line);
> -
> - if (start != (void *)0xffffffffff600000 ||
> - end != (void *)0xffffffffff601000) {
> - printf("[FAIL]\taddress range is nonsense\n");
> - nerrs++;
> - }
> -
> - printf("\tvsyscall permissions are %c-%c\n", r, x);
> - vsyscall_map_r = (r == 'r');
> - vsyscall_map_x = (x == 'x');
> -
> - found = true;
> - break;
> - }
> -
> - fclose(maps);
> -
> - if (!found) {
> - printf("\tno vsyscall map in /proc/self/maps\n");
> - vsyscall_map_r = false;
> - vsyscall_map_x = false;
> - }
> -
> - return nerrs;
> -#else
> - return 0;
> -#endif
> + ksft_print_msg("[WARN] failed to find getcpu in vDSO\n");
> }
>
What's going on here?
These changes are more extensive than the changelog indicates.
Additional explanation is needed before I can accept this.
thanks,
-- Shuah
Powered by blists - more mailing lists