| Message ID | 20180503192430.7582-2-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 03, 2018 at 01:24:26PM -0600, Ross Zwisler wrote: > The following commit > > commit c6ce3e2fe3da ("radix tree test suite: Add config option for map > shift") > > Introduced a phony makefile target called 'mapshift' that ends up > generating the file generated/map-shift.h. This phony target was then > added as a dependency of the top level 'targets' build target, which is > what is run when you go to tools/testing/radix-tree and just type 'make'. > > Unfortunately, this phony target doesn't actually work as a dependency, so > you end up getting: > > $ make > make: *** No rule to make target 'generated/map-shift.h', needed by 'main.o'. Stop. > make: *** Waiting for unfinished jobs.... > > Fix this by making the file generated/map-shift.h our real makefile target, > and add this a dependency of the top level build target. This commit breaks typing 'make SHIFT=6'. It doesn't rebuild the test suite any more. If I revert this patch, it works. Also, I can't reproduce the problem you're reporting here. So ... how do I reproduce it? Otherwise, I'm just going to revert this patch since it regresses a feature I find useful. > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > --- > tools/testing/radix-tree/Makefile | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile > index fa7ee369b3c9..db66f8a0d4be 100644 > --- a/tools/testing/radix-tree/Makefile > +++ b/tools/testing/radix-tree/Makefile > @@ -17,7 +17,7 @@ ifeq ($(BUILD), 32) > LDFLAGS += -m32 > endif > > -targets: mapshift $(TARGETS) > +targets: generated/map-shift.h $(TARGETS) > > main: $(OFILES) > > @@ -42,9 +42,7 @@ radix-tree.c: ../../../lib/radix-tree.c > idr.c: ../../../lib/idr.c > sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@ > > -.PHONY: mapshift > - > -mapshift: > +generated/map-shift.h: > @if ! grep -qws $(SHIFT) generated/map-shift.h; then \ > echo "#define RADIX_TREE_MAP_SHIFT $(SHIFT)" > \ > generated/map-shift.h; \ > -- > 2.14.3 >
On Sun, Jul 15, 2018 at 04:00:52PM -0700, Matthew Wilcox wrote: > On Thu, May 03, 2018 at 01:24:26PM -0600, Ross Zwisler wrote: > > The following commit > > > > commit c6ce3e2fe3da ("radix tree test suite: Add config option for map > > shift") > > > > Introduced a phony makefile target called 'mapshift' that ends up > > generating the file generated/map-shift.h. This phony target was then > > added as a dependency of the top level 'targets' build target, which is > > what is run when you go to tools/testing/radix-tree and just type 'make'. > > > > Unfortunately, this phony target doesn't actually work as a dependency, so > > you end up getting: > > > > $ make > > make: *** No rule to make target 'generated/map-shift.h', needed by 'main.o'. Stop. > > make: *** Waiting for unfinished jobs.... > > > > Fix this by making the file generated/map-shift.h our real makefile target, > > and add this a dependency of the top level build target. > > This commit breaks typing 'make SHIFT=6'. It doesn't rebuild the > test suite any more. If I revert this patch, it works. Also, I can't > reproduce the problem you're reporting here. So ... how do I reproduce > it? Otherwise, I'm just going to revert this patch since it regresses > a feature I find useful. The test suite builds fine for me in v4.17. From a completely clean tree, in tools/testing/radix-tree: $ make sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/radix-tree.c > radix-tree.c sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/idr.c > idr.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o main.o main.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o linux.o linux.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o test.o test.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o find_bit.o ../../lib/find_bit.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression1.o regression1.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression2.o regression2.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression3.o regression3.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o tag_check.o tag_check.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o multiorder.o multiorder.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr-test.o idr-test.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o iteration_check.o iteration_check.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o benchmark.o benchmark.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr.o idr.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o radix-tree.o radix-tree.c cc -fsanitize=address multiorder.o radix-tree.o idr.o linux.o test.o find_bit.o -lpthread -lurcu -o multiorder cc -fsanitize=address main.o radix-tree.o idr.o linux.o test.o find_bit.o regression1.o regression2.o regression3.o tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o -lpthread -lurcu -o main cc -fsanitize=address idr-test.o radix-tree.o idr.o linux.o test.o find_bit.o -lpthread -lurcu -o idr-test and you can successfully run the radix tree test suite by running 'main'. With the above mentioned commit reverted, this build fails: $ make make: *** No rule to make target 'generated/map-shift.h', needed by 'main.o'. Stop. make: *** Waiting for unfinished jobs.... If you want generated/map-shift.h to be rebuilt each time you run 'make' so that it can take a new SHIFT argument, that's fine, but let's not make users run 'make mapshift' before an actual 'make' will work, which is where we're at with v4.17 with my commit reverted. Incidentally, in the current linux/master the radix tree test suite again fails to build: $ make sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/radix-tree.c > radix-tree.c sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/idr.c > idr.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o main.o main.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o linux.o linux.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o test.o test.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o find_bit.o ../../lib/find_bit.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression1.o regression1.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression2.o regression2.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression3.o regression3.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o tag_check.o tag_check.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o multiorder.o multiorder.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr-test.o idr-test.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o iteration_check.o iteration_check.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o benchmark.o benchmark.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr.o idr.c cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o radix-tree.o radix-tree.c idr.c:7:10: fatal error: linux/xarray.h: No such file or directory #include <linux/xarray.h> ^~~~~~~~~~~~~~~~ compilation terminated. make: *** [<builtin>: idr.o] Error 1 Can you look into this? 0-day folks, would it be possible for you to add a radix tree build & test runs to your automated tests? We would really appreciate any help in reducing this kind of breakage, which seems to happen pretty often.
On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote: > Incidentally, in the current linux/master the radix tree test suite again > fails to build: > > $ make > sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/radix-tree.c > radix-tree.c > sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/idr.c > idr.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o main.o main.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o linux.o linux.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o test.o test.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o find_bit.o ../../lib/find_bit.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression1.o regression1.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression2.o regression2.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression3.o regression3.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o tag_check.o tag_check.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o multiorder.o multiorder.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr-test.o idr-test.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o iteration_check.o iteration_check.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o benchmark.o benchmark.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr.o idr.c > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o radix-tree.o radix-tree.c > idr.c:7:10: fatal error: linux/xarray.h: No such file or directory > #include <linux/xarray.h> > ^~~~~~~~~~~~~~~~ > compilation terminated. Umm. I think I know the problem here. I have a suspicion that either Fedora or you have changed make to be parallel by default (or you're lying to me and saying you typed 'make' when you actually typed 'make -j4', but I'm pretty sure you wouldn't do that). Because there's no way you'd get this output if you were compiling with make -j1. Indeed, if I revert your commit and then build with make -j4, I see the same error as you. I'll look at how to fix this properly tomorrow.
On Mon, Jul 16, 2018 at 08:18:11PM -0700, Matthew Wilcox wrote: > On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote: > > Incidentally, in the current linux/master the radix tree test suite again > > fails to build: > > > > $ make > > sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/radix-tree.c > radix-tree.c > > sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < ../../../lib/idr.c > idr.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o main.o main.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o linux.o linux.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o test.o test.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o find_bit.o ../../lib/find_bit.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression1.o regression1.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression2.o regression2.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o regression3.o regression3.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o tag_check.o tag_check.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o multiorder.o multiorder.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr-test.o idr-test.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o iteration_check.o iteration_check.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o benchmark.o benchmark.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o idr.o idr.c > > cc -I. -I../../include -g -O2 -Wall -D_LGPL_SOURCE -fsanitize=address -c -o radix-tree.o radix-tree.c > > idr.c:7:10: fatal error: linux/xarray.h: No such file or directory > > #include <linux/xarray.h> > > ^~~~~~~~~~~~~~~~ > > compilation terminated. > > Umm. I think I know the problem here. I have a suspicion that either > Fedora or you have changed make to be parallel by default (or you're > lying to me and saying you typed 'make' when you actually typed 'make > -j4', but I'm pretty sure you wouldn't do that). Because there's no > way you'd get this output if you were compiling with make -j1. > > Indeed, if I revert your commit and then build with make -j4, I see the > same error as you. I'll look at how to fix this properly tomorrow. Ah, yep. $ alias make alias make='make -j32' You've found it. :)
diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile index fa7ee369b3c9..db66f8a0d4be 100644 --- a/tools/testing/radix-tree/Makefile +++ b/tools/testing/radix-tree/Makefile @@ -17,7 +17,7 @@ ifeq ($(BUILD), 32) LDFLAGS += -m32 endif -targets: mapshift $(TARGETS) +targets: generated/map-shift.h $(TARGETS) main: $(OFILES) @@ -42,9 +42,7 @@ radix-tree.c: ../../../lib/radix-tree.c idr.c: ../../../lib/idr.c sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@ -.PHONY: mapshift - -mapshift: +generated/map-shift.h: @if ! grep -qws $(SHIFT) generated/map-shift.h; then \ echo "#define RADIX_TREE_MAP_SHIFT $(SHIFT)" > \ generated/map-shift.h; \
The following commit commit c6ce3e2fe3da ("radix tree test suite: Add config option for map shift") Introduced a phony makefile target called 'mapshift' that ends up generating the file generated/map-shift.h. This phony target was then added as a dependency of the top level 'targets' build target, which is what is run when you go to tools/testing/radix-tree and just type 'make'. Unfortunately, this phony target doesn't actually work as a dependency, so you end up getting: $ make make: *** No rule to make target 'generated/map-shift.h', needed by 'main.o'. Stop. make: *** Waiting for unfinished jobs.... Fix this by making the file generated/map-shift.h our real makefile target, and add this a dependency of the top level build target. Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> --- tools/testing/radix-tree/Makefile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)