| Message ID | 20240704023324.83564-3-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | selftests/vDSO: fix clang build errors, and Makefile cleanup | expand |
On 7/4/24 7:33 AM, John Hubbard wrote: > There were a couple of errors here: > > 1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program > to be built. However, lib.mk already does that because it assumes "bare" > program names are passed in, so this ended up creating > $(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended. > > 2. lib.mk was included before TEST_GEN_PROGS was set, which led to > lib.mk's "all:" target not seeing anything to rebuild. > > So nothing worked, which caused the author to force things by creating > an "all:" target locally--while still including ../lib.mk. > > Fix all of this by including ../lib.mk at the right place, and removing > the $(OUTPUT) prefix to the programs to be built, and removing the > duplicate "all:" target. > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/vDSO/Makefile | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile > index d53a4d8008f9..209ede5de208 100644 > --- a/tools/testing/selftests/vDSO/Makefile > +++ b/tools/testing/selftests/vDSO/Makefile > @@ -1,16 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0 > -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 $(OUTPUT)/vdso_test_getcpu > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres > +TEST_GEN_PROGS := vdso_test_gettimeofday > +TEST_GEN_PROGS += vdso_test_getcpu > +TEST_GEN_PROGS += vdso_test_abi > +TEST_GEN_PROGS += vdso_test_clock_getres > ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64)) > -TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 > +TEST_GEN_PROGS += vdso_standalone_test_x86 > endif > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness > +TEST_GEN_PROGS += vdso_test_correctness > > CFLAGS := -std=gnu99 > CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector > @@ -19,7 +18,7 @@ ifeq ($(CONFIG_X86_32),y) > LDLIBS += -lgcc_s > endif > > -all: $(TEST_GEN_PROGS) > +include ../lib.mk > $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c > $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c > $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c
On 7/3/24 20:33, John Hubbard wrote: > There were a couple of errors here: > > 1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program > to be built. However, lib.mk already does that because it assumes "bare" > program names are passed in, so this ended up creating > $(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended. > > 2. lib.mk was included before TEST_GEN_PROGS was set, which led to > lib.mk's "all:" target not seeing anything to rebuild. > > So nothing worked, which caused the author to force things by creating > an "all:" target locally--while still including ../lib.mk. > > Fix all of this by including ../lib.mk at the right place, and removing > the $(OUTPUT) prefix to the programs to be built, and removing the > duplicate "all:" target. > > Signed-off-by: John Hubbard <jhubbard@nvidia.com> Why does the summary say mm ? selftests/mm: remove partially duplicated "all:" target in Makefile > --- > tools/testing/selftests/vDSO/Makefile | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile > index d53a4d8008f9..209ede5de208 100644 > --- a/tools/testing/selftests/vDSO/Makefile > +++ b/tools/testing/selftests/vDSO/Makefile > @@ -1,16 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0 > -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 $(OUTPUT)/vdso_test_getcpu > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres > +TEST_GEN_PROGS := vdso_test_gettimeofday > +TEST_GEN_PROGS += vdso_test_getcpu > +TEST_GEN_PROGS += vdso_test_abi > +TEST_GEN_PROGS += vdso_test_clock_getres > ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64)) > -TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 > +TEST_GEN_PROGS += vdso_standalone_test_x86 > endif > -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness > +TEST_GEN_PROGS += vdso_test_correctness > > CFLAGS := -std=gnu99 > CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector > @@ -19,7 +18,7 @@ ifeq ($(CONFIG_X86_32),y) > LDLIBS += -lgcc_s > endif > > -all: $(TEST_GEN_PROGS) > +include ../lib.mk > $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c > $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c > $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c thanks, -- Shuah
On 7/5/24 10:53, Shuah Khan wrote: > On 7/3/24 20:33, John Hubbard wrote: >> There were a couple of errors here: >> >> 1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program >> to be built. However, lib.mk already does that because it assumes "bare" >> program names are passed in, so this ended up creating >> $(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended. >> >> 2. lib.mk was included before TEST_GEN_PROGS was set, which led to >> lib.mk's "all:" target not seeing anything to rebuild. >> >> So nothing worked, which caused the author to force things by creating >> an "all:" target locally--while still including ../lib.mk. >> >> Fix all of this by including ../lib.mk at the right place, and removing >> the $(OUTPUT) prefix to the programs to be built, and removing the >> duplicate "all:" target. >> >> Signed-off-by: John Hubbard <jhubbard@nvidia.com> > > Why does the summary say mm ? This is a mistake. I was also preparing some mm work at the time, and typed the wrong thing. Could you fix it up to s/mm/vDSO/ for me? Or if it's easier, I can send an updated patch, let me know. thanks, John Hubbard NVIDIA > > selftests/mm: remove partially duplicated "all:" target in Makefile > >> --- >> tools/testing/selftests/vDSO/Makefile | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/tools/testing/selftests/vDSO/Makefile >> b/tools/testing/selftests/vDSO/Makefile >> index d53a4d8008f9..209ede5de208 100644 >> --- a/tools/testing/selftests/vDSO/Makefile >> +++ b/tools/testing/selftests/vDSO/Makefile >> @@ -1,16 +1,15 @@ >> # SPDX-License-Identifier: GPL-2.0 >> -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 >> $(OUTPUT)/vdso_test_getcpu >> -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi >> -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres >> +TEST_GEN_PROGS := vdso_test_gettimeofday >> +TEST_GEN_PROGS += vdso_test_getcpu >> +TEST_GEN_PROGS += vdso_test_abi >> +TEST_GEN_PROGS += vdso_test_clock_getres >> ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64)) >> -TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 >> +TEST_GEN_PROGS += vdso_standalone_test_x86 >> endif >> -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness >> +TEST_GEN_PROGS += vdso_test_correctness >> CFLAGS := -std=gnu99 >> CFLAGS_vdso_standalone_test_x86 := -nostdlib >> -fno-asynchronous-unwind-tables -fno-stack-protector >> @@ -19,7 +18,7 @@ ifeq ($(CONFIG_X86_32),y) >> LDLIBS += -lgcc_s >> endif >> -all: $(TEST_GEN_PROGS) >> +include ../lib.mk >> $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c >> $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c >> $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c > > > thanks, > -- Shuah
On 7/5/24 11:58, John Hubbard wrote: > On 7/5/24 10:53, Shuah Khan wrote: >> On 7/3/24 20:33, John Hubbard wrote: >>> There were a couple of errors here: >>> >>> 1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program >>> to be built. However, lib.mk already does that because it assumes "bare" >>> program names are passed in, so this ended up creating >>> $(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended. >>> >>> 2. lib.mk was included before TEST_GEN_PROGS was set, which led to >>> lib.mk's "all:" target not seeing anything to rebuild. >>> >>> So nothing worked, which caused the author to force things by creating >>> an "all:" target locally--while still including ../lib.mk. >>> >>> Fix all of this by including ../lib.mk at the right place, and removing >>> the $(OUTPUT) prefix to the programs to be built, and removing the >>> duplicate "all:" target. >>> >>> Signed-off-by: John Hubbard <jhubbard@nvidia.com> >> >> Why does the summary say mm ? > > This is a mistake. I was also preparing some mm work at the time, > and typed the wrong thing. > > Could you fix it up to s/mm/vDSO/ for me? Or if it's easier, I can send > an updated patch, let me know. > > Send me updated patch - it will be easier that way. I saw the note about getting these into 6.10 - As soon as you send the patch I will apply them and send them for 6.10 thanks, -- Shuah
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile index d53a4d8008f9..209ede5de208 100644 --- a/tools/testing/selftests/vDSO/Makefile +++ b/tools/testing/selftests/vDSO/Makefile @@ -1,16 +1,15 @@ # SPDX-License-Identifier: GPL-2.0 -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 $(OUTPUT)/vdso_test_getcpu -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_clock_getres +TEST_GEN_PROGS := vdso_test_gettimeofday +TEST_GEN_PROGS += vdso_test_getcpu +TEST_GEN_PROGS += vdso_test_abi +TEST_GEN_PROGS += vdso_test_clock_getres ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64)) -TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86 +TEST_GEN_PROGS += vdso_standalone_test_x86 endif -TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness +TEST_GEN_PROGS += vdso_test_correctness CFLAGS := -std=gnu99 CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector @@ -19,7 +18,7 @@ ifeq ($(CONFIG_X86_32),y) LDLIBS += -lgcc_s endif -all: $(TEST_GEN_PROGS) +include ../lib.mk $(OUTPUT)/vdso_test_gettimeofday: parse_vdso.c vdso_test_gettimeofday.c $(OUTPUT)/vdso_test_getcpu: parse_vdso.c vdso_test_getcpu.c $(OUTPUT)/vdso_test_abi: parse_vdso.c vdso_test_abi.c
There were a couple of errors here: 1. TEST_GEN_PROGS was incorrectly prepending $(OUTPUT) to each program to be built. However, lib.mk already does that because it assumes "bare" program names are passed in, so this ended up creating $(OUTPUT)/$(OUTPUT)/file.c, which of course won't work as intended. 2. lib.mk was included before TEST_GEN_PROGS was set, which led to lib.mk's "all:" target not seeing anything to rebuild. So nothing worked, which caused the author to force things by creating an "all:" target locally--while still including ../lib.mk. Fix all of this by including ../lib.mk at the right place, and removing the $(OUTPUT) prefix to the programs to be built, and removing the duplicate "all:" target. Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- tools/testing/selftests/vDSO/Makefile | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)