| Message ID | 20200505174728.46594-4-broonie@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | selftests: vdso: Add a selftest for vDSO getcpu() | expand |
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).
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; +}
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