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%)
--
2.20.1
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 <[email protected]>
---
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.
*/
--
2.20.1
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 <[email protected]>
---
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
--
2.20.1
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 <[email protected]>
---
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;
+}
--
2.20.1
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
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 <[email protected]>
> ---
> 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 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 <[email protected]>
> ---
> 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.
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).