[1/5] radix tree test suite: fix mapshift build target

Commit Message

Matthew Wilcox July 16, 2018, 7:52 p.m. UTC

On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote:
> 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:
> > > 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....

OK ... what version of make are you using?  Because this works fine for me:

$ git clone linux clean
$ cd clean
$ git checkout v4.17
$ cd tools/testing/radix-tree/ 
$ git revert 8d9fa88edd5e360b71765feeadb915d4066c9684
$ make

$ make --version
GNU Make 4.1
Built for x86_64-pc-linux-gnu

It's Debian's Version: 4.1-9.1

> 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.

I don't want it to be rebuilt, I want it to be checked before each
build, regenerated if SHIFT has changed, and everything to rebuild if
it has changed.

> Incidentally, in the current linux/master the radix tree test suite again
> fails to build:
> 
>   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?  

Ah ... Andrew broke it fixing something else ;-)  The first commit in
my xarray branch fixes it -- looks like I tested my own commits but not
the commit I based on ;-)



> 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.

Yeah, that would be good.

Comments

Ross Zwisler July 16, 2018, 9:08 p.m. UTC | #1

On Mon, Jul 16, 2018 at 12:52:49PM -0700, Matthew Wilcox wrote:
> On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote:
<>
> OK ... what version of make are you using?  Because this works fine for me:
> 
> $ git clone linux clean
> $ cd clean
> $ git checkout v4.17
> $ cd tools/testing/radix-tree/ 
> $ git revert 8d9fa88edd5e360b71765feeadb915d4066c9684
> $ make
> 
> $ make --version
> GNU Make 4.1
> Built for x86_64-pc-linux-gnu
> 
> It's Debian's Version: 4.1-9.1

$ make --version
GNU Make 4.2.1
Built for x86_64-redhat-linux-gnu

The one from Fedora 27.

> > 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.
> 
> I don't want it to be rebuilt, I want it to be checked before each
> build, regenerated if SHIFT has changed, and everything to rebuild if
> it has changed.

Sure, sounds good.

Matthew Wilcox July 17, 2018, 2:41 a.m. UTC | #2

On Mon, Jul 16, 2018 at 03:08:20PM -0600, Ross Zwisler wrote:
> On Mon, Jul 16, 2018 at 12:52:49PM -0700, Matthew Wilcox wrote:
> > On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote:
> <>
> > OK ... what version of make are you using?  Because this works fine for me:
> > 
> > $ git clone linux clean
> > $ cd clean
> > $ git checkout v4.17
> > $ cd tools/testing/radix-tree/ 
> > $ git revert 8d9fa88edd5e360b71765feeadb915d4066c9684
> > $ make
> > 
> > $ make --version
> > GNU Make 4.1
> > Built for x86_64-pc-linux-gnu
> > 
> > It's Debian's Version: 4.1-9.1
> 
> $ make --version
> GNU Make 4.2.1
> Built for x86_64-redhat-linux-gnu
> 
> The one from Fedora 27.

Huh.  I just tried 4.2.1-1.1 from Debian unstable and that doesn't
produce the problem either.  I'm not sure how to proceed at this point.
I'm really not a makefile expert.

Dave Chinner July 21, 2018, 11:45 p.m. UTC | #3

On Mon, Jul 16, 2018 at 07:41:32PM -0700, Matthew Wilcox wrote:
> On Mon, Jul 16, 2018 at 03:08:20PM -0600, Ross Zwisler wrote:
> > On Mon, Jul 16, 2018 at 12:52:49PM -0700, Matthew Wilcox wrote:
> > > On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote:
> > <>
> > > OK ... what version of make are you using?  Because this works fine for me:
> > > 
> > > $ git clone linux clean
> > > $ cd clean
> > > $ git checkout v4.17
> > > $ cd tools/testing/radix-tree/ 
> > > $ git revert 8d9fa88edd5e360b71765feeadb915d4066c9684
> > > $ make
> > > 
> > > $ make --version
> > > GNU Make 4.1
> > > Built for x86_64-pc-linux-gnu
> > > 
> > > It's Debian's Version: 4.1-9.1
> > 
> > $ make --version
> > GNU Make 4.2.1
> > Built for x86_64-redhat-linux-gnu
> > 
> > The one from Fedora 27.
> 
> Huh.  I just tried 4.2.1-1.1 from Debian unstable and that doesn't
> produce the problem either.  I'm not sure how to proceed at this point.
> I'm really not a makefile expert.

This smells like a problem we just hit with make 4.2.1 in fedora 28
in fstests - the regex expanstion has been screwed up such that
things like [a-z] will match [A-Z] and other things as well. Debian
is unaffected, apparently fedora has a backport of stuff from the
as-yet-unreleased next version of make/glibc. See this thread:

https://www.spinics.net/lists/fstests/msg10200.html

Try setting LANG=C and seeing if the problem goes away....

Cheers,

Dave.

Ross Zwisler July 22, 2018, 3:11 a.m. UTC | #4

On Sun, Jul 22, 2018 at 09:45:50AM +1000, Dave Chinner wrote:
> On Mon, Jul 16, 2018 at 07:41:32PM -0700, Matthew Wilcox wrote:
> > On Mon, Jul 16, 2018 at 03:08:20PM -0600, Ross Zwisler wrote:
> > > On Mon, Jul 16, 2018 at 12:52:49PM -0700, Matthew Wilcox wrote:
> > > > On Mon, Jul 16, 2018 at 10:07:10AM -0600, Ross Zwisler wrote:
> > > <>
> > > > OK ... what version of make are you using?  Because this works fine for me:
> > > > 
> > > > $ git clone linux clean
> > > > $ cd clean
> > > > $ git checkout v4.17
> > > > $ cd tools/testing/radix-tree/ 
> > > > $ git revert 8d9fa88edd5e360b71765feeadb915d4066c9684
> > > > $ make
> > > > 
> > > > $ make --version
> > > > GNU Make 4.1
> > > > Built for x86_64-pc-linux-gnu
> > > > 
> > > > It's Debian's Version: 4.1-9.1
> > > 
> > > $ make --version
> > > GNU Make 4.2.1
> > > Built for x86_64-redhat-linux-gnu
> > > 
> > > The one from Fedora 27.
> > 
> > Huh.  I just tried 4.2.1-1.1 from Debian unstable and that doesn't
> > produce the problem either.  I'm not sure how to proceed at this point.
> > I'm really not a makefile expert.
> 
> This smells like a problem we just hit with make 4.2.1 in fedora 28
> in fstests - the regex expanstion has been screwed up such that
> things like [a-z] will match [A-Z] and other things as well. Debian
> is unaffected, apparently fedora has a backport of stuff from the
> as-yet-unreleased next version of make/glibc. See this thread:
> 
> https://www.spinics.net/lists/fstests/msg10200.html
> 
> Try setting LANG=C and seeing if the problem goes away....

Hey Dave, we root caused this difference to to be the fact that I had 'make'
aliased to 'make -j32' in my .bashrc.  Matthew was running a singled threaded
build, while I was running a multi-threaded one.  When Matthew ran a
multi-threaded build, he was able to reproduce the issue.

So, essentially I think that the makefile just needs to be enhanced so that
all the dependencies are explicit to allow multi-threaded builds to work
properly.

Patch

diff --git a/tools/testing/radix-tree/linux/xarray.h b/tools/testing/radix-tree/linux/xarray.h
new file mode 100644
index 000000000000..df3812cda376
--- /dev/null
+++ b/tools/testing/radix-tree/linux/xarray.h
@@ -0,0 +1,2 @@ 
+#include "generated/map-shift.h"
+#include "../../../../include/linux/xarray.h"