This series does a bit of a cleanup of the existing tests for the vDSO in kselftest and then adds a new test for getcpu().
Mark Brown (3): selftests: vdso: Rename vdso_test to vdso_test_gettimeofday selftests: vdso: Use a header file to prototype parse_vdso API selftests: vdso: Add a selftest for vDSO getcpu()
tools/testing/selftests/vDSO/.gitignore | 2 + tools/testing/selftests/vDSO/Makefile | 5 +- tools/testing/selftests/vDSO/parse_vdso.c | 24 +-------- tools/testing/selftests/vDSO/parse_vdso.h | 31 ++++++++++++ .../selftests/vDSO/vdso_standalone_test_x86.c | 4 +- .../testing/selftests/vDSO/vdso_test_getcpu.c | 50 +++++++++++++++++++ .../{vdso_test.c => vdso_test_gettimeofday.c} | 10 ++-- 7 files changed, 92 insertions(+), 34 deletions(-) create mode 100644 tools/testing/selftests/vDSO/parse_vdso.h create mode 100644 tools/testing/selftests/vDSO/vdso_test_getcpu.c rename tools/testing/selftests/vDSO/{vdso_test.c => vdso_test_gettimeofday.c} (84%)
Currently the vDSO kselftests have a test called vdso_test which tests the vDSO implementation of gettimeofday(). In preparation for adding tests for other vDSO functionality rename this test to reflect what's going on.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/vDSO/.gitignore | 1 + tools/testing/selftests/vDSO/Makefile | 4 ++-- .../selftests/vDSO/{vdso_test.c => vdso_test_gettimeofday.c} | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) rename tools/testing/selftests/vDSO/{vdso_test.c => vdso_test_gettimeofday.c} (89%)
diff --git a/tools/testing/selftests/vDSO/.gitignore b/tools/testing/selftests/vDSO/.gitignore index 382cfb39a1a3..74f49bd5f6dd 100644 --- a/tools/testing/selftests/vDSO/.gitignore +++ b/tools/testing/selftests/vDSO/.gitignore @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only vdso_test +vdso_test_gettimeofday vdso_standalone_test_x86 diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index 9e03d61f52fd..ae15d700b62e 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -4,7 +4,7 @@ include ../lib.mk uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
-TEST_GEN_PROGS := $(OUTPUT)/vdso_test +TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -17,7 +17,7 @@ LDLIBS += -lgcc_s endif
all: $(TEST_GEN_PROGS) -$(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c +$(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_test.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c similarity index 89% rename from tools/testing/selftests/vDSO/vdso_test.c rename to tools/testing/selftests/vDSO/vdso_test_gettimeofday.c index 719d5a6bd664..511c0dc5e47e 100644 --- a/tools/testing/selftests/vDSO/vdso_test.c +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c @@ -1,10 +1,11 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * vdso_test.c: Sample code to test parse_vdso.c + * vdso_test_gettimeofday.c: Sample code to test parse_vdso.c and + * vDSO gettimeofday() * Copyright (c) 2014 Andy Lutomirski * * Compile with: - * gcc -std=gnu99 vdso_test.c parse_vdso.c + * gcc -std=gnu99 vdso_test_gettimeofday.c parse_vdso_gettimeofday.c * * Tested on x86, 32-bit and 64-bit. It may work on other architectures, too. */
Both vdso_test_gettimeofday and vdso_standalone_test_x86 use the library in parse_vdso.c but each separately declares the API it offers which is not ideal. Create a header file with prototypes of the functions and use it in both the library and the tests to ensure that the same prototypes are used throughout.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/vDSO/parse_vdso.c | 24 +------------- tools/testing/selftests/vDSO/parse_vdso.h | 31 +++++++++++++++++++ .../selftests/vDSO/vdso_standalone_test_x86.c | 4 +-- .../selftests/vDSO/vdso_test_gettimeofday.c | 5 +-- 4 files changed, 34 insertions(+), 30 deletions(-) create mode 100644 tools/testing/selftests/vDSO/parse_vdso.h
diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 1dbb4b87268f..413f75620a35 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -21,29 +21,7 @@ #include <limits.h> #include <elf.h>
-/* - * To use this vDSO parser, first call one of the vdso_init_* functions. - * If you've already parsed auxv, then pass the value of AT_SYSINFO_EHDR - * to vdso_init_from_sysinfo_ehdr. Otherwise pass auxv to vdso_init_from_auxv. - * Then call vdso_sym for each symbol you want. For example, to look up - * gettimeofday on x86_64, use: - * - * <some pointer> = vdso_sym("LINUX_2.6", "gettimeofday"); - * or - * <some pointer> = vdso_sym("LINUX_2.6", "__vdso_gettimeofday"); - * - * vdso_sym will return 0 if the symbol doesn't exist or if the init function - * failed or was not called. vdso_sym is a little slow, so its return value - * should be cached. - * - * vdso_sym is threadsafe; the init functions are not. - * - * These are the prototypes: - */ -extern void vdso_init_from_auxv(void *auxv); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void *vdso_sym(const char *version, const char *name); - +#include "parse_vdso.h"
/* And here's the code. */ #ifndef ELF_BITS diff --git a/tools/testing/selftests/vDSO/parse_vdso.h b/tools/testing/selftests/vDSO/parse_vdso.h new file mode 100644 index 000000000000..ea4b8635bb0b --- /dev/null +++ b/tools/testing/selftests/vDSO/parse_vdso.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef PARSE_VDSO_H +#define PARSE_VDSO_H + +#include <stdint.h> + +/* + * To use this vDSO parser, first call one of the vdso_init_* functions. + * If you've already parsed auxv, then pass the value of AT_SYSINFO_EHDR + * to vdso_init_from_sysinfo_ehdr. Otherwise pass auxv to vdso_init_from_auxv. + * Then call vdso_sym for each symbol you want. For example, to look up + * gettimeofday on x86_64, use: + * + * <some pointer> = vdso_sym("LINUX_2.6", "gettimeofday"); + * or + * <some pointer> = vdso_sym("LINUX_2.6", "__vdso_gettimeofday"); + * + * vdso_sym will return 0 if the symbol doesn't exist or if the init function + * failed or was not called. vdso_sym is a little slow, so its return value + * should be cached. + * + * vdso_sym is threadsafe; the init functions are not. + * + * These are the prototypes: + */ +extern void *vdso_sym(const char *version, const char *name); +extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); +extern void vdso_init_from_auxv(void *auxv); + +#endif diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c index 5ac4b00acfbc..8a44ff973ee1 100644 --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c @@ -16,9 +16,7 @@ #include <unistd.h> #include <stdint.h>
-extern void *vdso_sym(const char *version, const char *name); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void vdso_init_from_auxv(void *auxv); +#include "parse_vdso.h"
/* We need a libc functions... */ int strcmp(const char *a, const char *b) diff --git a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c index 511c0dc5e47e..8ccc73ed8240 100644 --- a/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c +++ b/tools/testing/selftests/vDSO/vdso_test_gettimeofday.c @@ -17,10 +17,7 @@ #include <sys/time.h>
#include "../kselftest.h" - -extern void *vdso_sym(const char *version, const char *name); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void vdso_init_from_auxv(void *auxv); +#include "parse_vdso.h"
/* * ARM64's vDSO exports its gettimeofday() implementation with a different
On 5/5/20 11:47 AM, Mark Brown wrote:
Both vdso_test_gettimeofday and vdso_standalone_test_x86 use the library in parse_vdso.c but each separately declares the API it offers which is not ideal. Create a header file with prototypes of the functions and use it in both the library and the tests to ensure that the same prototypes are used throughout.
Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/vDSO/parse_vdso.c | 24 +------------- tools/testing/selftests/vDSO/parse_vdso.h | 31 +++++++++++++++++++ .../selftests/vDSO/vdso_standalone_test_x86.c | 4 +-- .../selftests/vDSO/vdso_test_gettimeofday.c | 5 +-- 4 files changed, 34 insertions(+), 30 deletions(-) create mode 100644 tools/testing/selftests/vDSO/parse_vdso.h
diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c index 1dbb4b87268f..413f75620a35 100644 --- a/tools/testing/selftests/vDSO/parse_vdso.c +++ b/tools/testing/selftests/vDSO/parse_vdso.c @@ -21,29 +21,7 @@ #include <limits.h> #include <elf.h> -/*
- To use this vDSO parser, first call one of the vdso_init_* functions.
- If you've already parsed auxv, then pass the value of AT_SYSINFO_EHDR
- to vdso_init_from_sysinfo_ehdr. Otherwise pass auxv to vdso_init_from_auxv.
- Then call vdso_sym for each symbol you want. For example, to look up
- gettimeofday on x86_64, use:
<some pointer> = vdso_sym("LINUX_2.6", "gettimeofday");
- or
<some pointer> = vdso_sym("LINUX_2.6", "__vdso_gettimeofday");
- vdso_sym will return 0 if the symbol doesn't exist or if the init function
- failed or was not called. vdso_sym is a little slow, so its return value
- should be cached.
- vdso_sym is threadsafe; the init functions are not.
- These are the prototypes:
- */
-extern void vdso_init_from_auxv(void *auxv); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void *vdso_sym(const char *version, const char *name);
+#include "parse_vdso.h" /* And here's the code. */ #ifndef ELF_BITS diff --git a/tools/testing/selftests/vDSO/parse_vdso.h b/tools/testing/selftests/vDSO/parse_vdso.h new file mode 100644 index 000000000000..ea4b8635bb0b --- /dev/null +++ b/tools/testing/selftests/vDSO/parse_vdso.h @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef PARSE_VDSO_H +#define PARSE_VDSO_H
+#include <stdint.h>
+/*
- To use this vDSO parser, first call one of the vdso_init_* functions.
- If you've already parsed auxv, then pass the value of AT_SYSINFO_EHDR
- to vdso_init_from_sysinfo_ehdr. Otherwise pass auxv to vdso_init_from_auxv.
- Then call vdso_sym for each symbol you want. For example, to look up
- gettimeofday on x86_64, use:
<some pointer> = vdso_sym("LINUX_2.6", "gettimeofday");
- or
<some pointer> = vdso_sym("LINUX_2.6", "__vdso_gettimeofday");
- vdso_sym will return 0 if the symbol doesn't exist or if the init function
- failed or was not called. vdso_sym is a little slow, so its return value
- should be cached.
- vdso_sym is threadsafe; the init functions are not.
- These are the prototypes:
- */
+extern void *vdso_sym(const char *version, const char *name); +extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); +extern void vdso_init_from_auxv(void *auxv);
You don't need extern here - this should be in scope?
+#endif diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c index 5ac4b00acfbc..8a44ff973ee1 100644 --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c @@ -16,9 +16,7 @@ #include <unistd.h> #include <stdint.h> -extern void *vdso_sym(const char *version, const char *name); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void vdso_init_from_auxv(void *auxv);
thanks, -- Shuah
On Tue, May 19, 2020 at 11:29:10AM -0600, shuah wrote:
On 5/5/20 11:47 AM, Mark Brown wrote:
- These are the prototypes:
- */
-extern void vdso_init_from_auxv(void *auxv); -extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); -extern void *vdso_sym(const char *version, const char *name);
- These are the prototypes:
- */
+extern void *vdso_sym(const char *version, const char *name); +extern void vdso_init_from_sysinfo_ehdr(uintptr_t base); +extern void vdso_init_from_auxv(void *auxv);
You don't need extern here - this should be in scope?
This is just code motion of existing code which has the externs. There's no cases where you *need* the extern, it's just syntactic sugar.
Provide a very basic selftest for getcpu() which similarly to our existing test for gettimeofday() looks up the function in the vDSO and prints the results it gets if the function exists and succeeds.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/vDSO/.gitignore | 1 + tools/testing/selftests/vDSO/Makefile | 3 +- .../testing/selftests/vDSO/vdso_test_getcpu.c | 50 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/vDSO/vdso_test_getcpu.c
diff --git a/tools/testing/selftests/vDSO/.gitignore b/tools/testing/selftests/vDSO/.gitignore index 74f49bd5f6dd..5eb64d41e541 100644 --- a/tools/testing/selftests/vDSO/.gitignore +++ b/tools/testing/selftests/vDSO/.gitignore @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only vdso_test vdso_test_gettimeofday +vdso_test_getcpu vdso_standalone_test_x86 diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index ae15d700b62e..0069f2f83f86 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -4,7 +4,7 @@ include ../lib.mk uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
-TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday +TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +18,7 @@ endif
all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c +$(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_test_getcpu.c b/tools/testing/selftests/vDSO/vdso_test_getcpu.c new file mode 100644 index 000000000000..a9dd3db145f3 --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_test_getcpu.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * vdso_test_getcpu.c: Sample code to test parse_vdso.c and vDSO getcpu() + * + * Copyright (c) 2020 Arm Ltd + */ + +#include <stdint.h> +#include <elf.h> +#include <stdio.h> +#include <sys/auxv.h> +#include <sys/time.h> + +#include "../kselftest.h" +#include "parse_vdso.h" + +const char *version = "LINUX_2.6"; +const char *name = "__vdso_getcpu"; + +struct getcpu_cache; +typedef long (*getcpu_t)(unsigned int *, unsigned int *, + struct getcpu_cache *); + +int main(int argc, char **argv) +{ + unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + if (!sysinfo_ehdr) { + printf("AT_SYSINFO_EHDR is not present!\n"); + return KSFT_SKIP; + } + + vdso_init_from_sysinfo_ehdr(getauxval(AT_SYSINFO_EHDR)); + + getcpu_t get_cpu = (getcpu_t)vdso_sym(version, name); + if (!get_cpu) { + printf("Could not find %s\n", name); + return KSFT_SKIP; + } + + unsigned int cpu, node; + long ret = get_cpu(&cpu, &node, 0); + if (ret == 0) { + printf("Running on CPU %u node %u\n", cpu, node); + } else { + printf("%s failed\n", name); + return KSFT_FAIL; + } + + return 0; +}
Hi Mark,
On 5/5/20 11:47 AM, Mark Brown wrote:
Provide a very basic selftest for getcpu() which similarly to our existing test for gettimeofday() looks up the function in the vDSO and prints the results it gets if the function exists and succeeds.
Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/vDSO/.gitignore | 1 + tools/testing/selftests/vDSO/Makefile | 3 +- .../testing/selftests/vDSO/vdso_test_getcpu.c | 50 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/vDSO/vdso_test_getcpu.c
diff --git a/tools/testing/selftests/vDSO/.gitignore b/tools/testing/selftests/vDSO/.gitignore index 74f49bd5f6dd..5eb64d41e541 100644 --- a/tools/testing/selftests/vDSO/.gitignore +++ b/tools/testing/selftests/vDSO/.gitignore @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only vdso_test vdso_test_gettimeofday +vdso_test_getcpu vdso_standalone_test_x86 diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index ae15d700b62e..0069f2f83f86 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -4,7 +4,7 @@ include ../lib.mk uname_M := $(shell uname -m 2>/dev/null || echo not) ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) -TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday +TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu ifeq ($(ARCH),x86) TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 endif @@ -18,6 +18,7 @@ endif all: $(TEST_GEN_PROGS) $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c +$(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c $(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \ vdso_standalone_test_x86.c parse_vdso.c \ diff --git a/tools/testing/selftests/vDSO/vdso_test_getcpu.c b/tools/testing/selftests/vDSO/vdso_test_getcpu.c new file mode 100644 index 000000000000..a9dd3db145f3 --- /dev/null +++ b/tools/testing/selftests/vDSO/vdso_test_getcpu.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- vdso_test_getcpu.c: Sample code to test parse_vdso.c and vDSO getcpu()
- Copyright (c) 2020 Arm Ltd
- */
+#include <stdint.h> +#include <elf.h> +#include <stdio.h> +#include <sys/auxv.h> +#include <sys/time.h>
+#include "../kselftest.h" +#include "parse_vdso.h"
+const char *version = "LINUX_2.6"; +const char *name = "__vdso_getcpu";
+struct getcpu_cache; +typedef long (*getcpu_t)(unsigned int *, unsigned int *,
struct getcpu_cache *);+int main(int argc, char **argv) +{
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations #135: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:27: + unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + if (!sysinfo_ehdr) {
WARNING: Missing a blank line after declarations #143: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:35: + getcpu_t get_cpu = (getcpu_t)vdso_sym(version, name); + if (!get_cpu) {
WARNING: Missing a blank line after declarations #150: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:42: + long ret = get_cpu(&cpu, &node, 0); + if (ret == 0) {
Please send v2 for this one.
thanks, -- Shuah
On Tue, May 19, 2020 at 11:11:28AM -0600, shuah wrote:
On 5/5/20 11:47 AM, Mark Brown wrote:
+int main(int argc, char **argv) +{
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations #135: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:27:
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
- if (!sysinfo_ehdr) {
This is the idiom in use by the existing gettimeofday test:
WARNING: Missing a blank line after declarations #38: FILE: tools/testing/selftests/vDSO/vdso_test_gettimeofday.c:38: + unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR); + if (!sysinfo_ehdr) {
so I don't know how you want the code to look here?
On 5/19/20 11:44 AM, Mark Brown wrote:
On Tue, May 19, 2020 at 11:11:28AM -0600, shuah wrote:
On 5/5/20 11:47 AM, Mark Brown wrote:
+int main(int argc, char **argv) +{
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations #135: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:27:
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
A blank line after declarations here just like what checkpatch suggests. It makes it readable.
- if (!sysinfo_ehdr) {
This is the idiom in use by the existing gettimeofday test:
WARNING: Missing a blank line after declarations #38: FILE: tools/testing/selftests/vDSO/vdso_test_gettimeofday.c:38:
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
- if (!sysinfo_ehdr) {
so I don't know how you want the code to look here?
See above.
thanks, -- Shuah
On Fri, May 22, 2020 at 08:55:50AM -0600, shuah wrote:
On 5/19/20 11:44 AM, Mark Brown wrote:
WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations #135: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:27:
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
A blank line after declarations here just like what checkpatch suggests. It makes it readable.
That doesn't match the idiom used by any of the surrounding code :(
On 5/22/20 9:12 AM, Mark Brown wrote:
On Fri, May 22, 2020 at 08:55:50AM -0600, shuah wrote:
On 5/19/20 11:44 AM, Mark Brown wrote:
WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations #135: FILE: tools/testing/selftests/vDSO/vdso_test_getcpu.c:27:
- unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
A blank line after declarations here just like what checkpatch suggests. It makes it readable.
That doesn't match the idiom used by any of the surrounding code :(
I can't parse the idiom statement? Can you clarify it please.
thanks, -- Shuah
On Fri, May 22, 2020 at 09:15:07AM -0600, shuah wrote:
On 5/22/20 9:12 AM, Mark Brown wrote:
That doesn't match the idiom used by any of the surrounding code :(
I can't parse the idiom statement? Can you clarify it please.
The other code in the vDSO selftests does this (the quoted line was a cut'n'paste from the gettimeofday() test and most of the functions in parse_vdso.c are similar).
On 5/5/20 11:47 AM, Mark Brown wrote:
This series does a bit of a cleanup of the existing tests for the vDSO in kselftest and then adds a new test for getcpu().
Mark Brown (3): selftests: vdso: Rename vdso_test to vdso_test_gettimeofday selftests: vdso: Use a header file to prototype parse_vdso API selftests: vdso: Add a selftest for vDSO getcpu()
tools/testing/selftests/vDSO/.gitignore | 2 + tools/testing/selftests/vDSO/Makefile | 5 +- tools/testing/selftests/vDSO/parse_vdso.c | 24 +-------- tools/testing/selftests/vDSO/parse_vdso.h | 31 ++++++++++++ .../selftests/vDSO/vdso_standalone_test_x86.c | 4 +- .../testing/selftests/vDSO/vdso_test_getcpu.c | 50 +++++++++++++++++++ .../{vdso_test.c => vdso_test_gettimeofday.c} | 10 ++-- 7 files changed, 92 insertions(+), 34 deletions(-) create mode 100644 tools/testing/selftests/vDSO/parse_vdso.h create mode 100644 tools/testing/selftests/vDSO/vdso_test_getcpu.c rename tools/testing/selftests/vDSO/{vdso_test.c => vdso_test_gettimeofday.c} (84%)
Thanks Mark. These look good to me. I will apply them for Linux 5.8-rc1.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Mark Brown -
shuah