[PATCH 0/2] tools: Add a toplevel Makefile

[PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Hi all,

this is a refresh and carve-out of an old patchset. It adds a toplevel
Makefile to tools/ so that one can build the tool of her/his liking by
simply doing

$ cd tools/
$ make <toolname>

By default, we build perf. There's also a scripts/Makefile.lib now which
should contain all make-related generic stuff which can be used by all
tools' build process after including this file.

Any comments/suggestions are welcome,
thanks.

[PATCH 1/2] tools: Add Makefile.lib

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Put generic enough build settings which could be reused by other tools
into a common Makefile.lib file.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 tools/perf/Makefile        |   45 +-----------------------------------
 tools/scripts/Makefile.lib |   55 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 44 deletions(-)
 create mode 100644 tools/scripts/Makefile.lib

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 8a4b9bccf8b2..3c0bfa39eb8b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -1,18 +1,10 @@
-ifeq ("$(origin O)", "command line")
-	OUTPUT := $(O)/
-endif
+include ../scripts/Makefile.lib
 
 # The default target of this Makefile is...
 all:
 
 include config/utilities.mak
 
-ifneq ($(OUTPUT),)
-# check that the output directory actually exists
-OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
-$(if $(OUTDIR),, $(error output directory "$(OUTPUT)" does not exist))
-endif
-
 # Define V to have a more verbose compile.
 #
 # Define PYTHON to point to the python binary if the default
@@ -70,31 +62,6 @@ ifneq ($(WERROR),0)
 	CFLAGS_WERROR := -Werror
 endif
 
-#
-# Include saner warnings here, which can catch bugs:
-#
-
-EXTRA_WARNINGS := -Wformat
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-y2k
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-prototypes
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wnested-externs
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement
-
 ifeq ("$(origin DEBUG)", "command line")
   PERF_DEBUG = $(DEBUG)
 endif
@@ -619,16 +586,6 @@ else
 	endif
 endif
 
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_MKDIR    = @echo '   ' MKDIR $@;
-	QUIET_GEN      = @echo '   ' GEN $@;
-endif
-endif
-
 ifdef ASCIIDOC8
 	export ASCIIDOC8
 endif
diff --git a/tools/scripts/Makefile.lib b/tools/scripts/Makefile.lib
new file mode 100644
index 000000000000..a2586d13bcf8
--- /dev/null
+++ b/tools/scripts/Makefile.lib
@@ -0,0 +1,55 @@
+ifeq ("$(origin O)", "command line")
+	OUTPUT := $(O)/
+endif
+
+ifneq ($(OUTPUT),)
+# check that the output directory actually exists
+OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
+$(if $(OUTDIR),, $(error output directory "$(OUTPUT)" does not exist))
+endif
+
+#
+# Include saner warnings here, which can catch bugs:
+#
+EXTRA_WARNINGS := -Wformat
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-y2k
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-prototypes
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wnested-externs
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
+EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement
+
+ifneq ($(findstring $(MAKEFLAGS), w),w)
+PRINT_DIR = --no-print-directory
+else
+NO_SUBDIR = :
+endif
+
+QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
+QUIET_SUBDIR1  =
+
+ifneq ($(findstring $(MAKEFLAGS),s),s)
+ifndef V
+	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_AR       = @echo '   ' AR $@;
+	QUIET_LINK     = @echo '   ' LINK $@;
+	QUIET_MKDIR    = @echo '   ' MKDIR $@;
+	QUIET_GEN      = @echo '   ' GEN $@;
+	QUIET_SUBDIR0  = +@subdir=
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+			 $(MAKE) $(PRINT_DIR) -C $$subdir
+endif
+endif
-- 
1.7.9.3.362.g71319

[PATCH 2/2] tools: Add a toplevel Makefile

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Add a Makefile with all the targets under tools/. Make perf the default
one and add a minimalistic Makefile to slub/ for completeness.

Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 tools/Makefile      |   24 ++++++++++++++++++++++++
 tools/slub/Makefile |    4 ++++
 2 files changed, 28 insertions(+)
 create mode 100644 tools/Makefile
 create mode 100644 tools/slub/Makefile

diff --git a/tools/Makefile b/tools/Makefile
new file mode 100644
index 000000000000..97631367e0db
--- /dev/null
+++ b/tools/Makefile
@@ -0,0 +1,24 @@
+include scripts/Makefile.lib
+
+perf firewire lguest slub usb virtio: FORCE
+	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
+
+cpupower: FORCE
+	$(QUIET_SUBDIR0)power/$@/ $(QUIET_SUBDIR1)
+
+turbostat x86_energy_perf_policy: FORCE
+	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
+
+firewire_clean lguest_clean perf_clean slub_clean usb_clean virtio_clean:
+	$(QUIET_SUBDIR0)$(@:_clean=)/ $(QUIET_SUBDIR1) clean
+
+cp_clean:
+	$(QUIET_SUBDIR0)power/cpupower/ $(QUIET_SUBDIR1) clean
+
+turbostat_clean x86_energy_perf_policy_clean:
+	$(QUIET_SUBDIR0)power/x86/$(@:_clean=)/ $(QUIET_SUBDIR1) clean
+
+clean: cp_clean firewire_clean lguest_clean perf_clean slub_clean turbostat_clean \
+		usb_clean virtio_clean x86_energy_perf_policy_clean
+
+.PHONY: FORCE
diff --git a/tools/slub/Makefile b/tools/slub/Makefile
new file mode 100644
index 000000000000..b2cf6b467bbe
--- /dev/null
+++ b/tools/slub/Makefile
@@ -0,0 +1,4 @@
+slabinfo: slabinfo.c
+
+clean:
+	rm -rf slabinfo
-- 
1.7.9.3.362.g71319

Re: [PATCH 2/2] tools: Add a toplevel Makefile

[Christoph Lameter]

On Thu, 22 Mar 2012, Borislav Petkov wrote:

> index 000000000000..97631367e0db
> --- /dev/null
> +++ b/tools/Makefile
> @@ -0,0 +1,24 @@
> +include scripts/Makefile.lib
> +
> +perf firewire lguest slub usb virtio: FORCE
> +	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
> +
> +cpupower: FORCE
> +	$(QUIET_SUBDIR0)power/$@/ $(QUIET_SUBDIR1)
> +
> +turbostat x86_energy_perf_policy: FORCE
> +	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
> +
> +firewire_clean lguest_clean perf_clean slub_clean usb_clean virtio_clean:
> +	$(QUIET_SUBDIR0)$(@:_clean=)/ $(QUIET_SUBDIR1) clean
> +
> +cp_clean:
> +	$(QUIET_SUBDIR0)power/cpupower/ $(QUIET_SUBDIR1) clean
> +
> +turbostat_clean x86_energy_perf_policy_clean:
> +	$(QUIET_SUBDIR0)power/x86/$(@:_clean=)/ $(QUIET_SUBDIR1) clean
> +
> +clean: cp_clean firewire_clean lguest_clean perf_clean slub_clean turbostat_clean \

slub_clean? Where is that target defined?

> diff --git a/tools/slub/Makefile b/tools/slub/Makefile
> new file mode 100644
> index 000000000000..b2cf6b467bbe
> --- /dev/null
> +++ b/tools/slub/Makefile
> @@ -0,0 +1,4 @@
> +slabinfo: slabinfo.c
> +
> +clean:
> +	rm -rf slabinfo

That portion looks okay.

Re: [PATCH 2/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Thu, Mar 22, 2012 at 01:33:18PM -0500, Christoph Lameter wrote:
> On Thu, 22 Mar 2012, Borislav Petkov wrote:
> 
> > index 000000000000..97631367e0db
> > --- /dev/null
> > +++ b/tools/Makefile
> > @@ -0,0 +1,24 @@
> > +include scripts/Makefile.lib
> > +
> > +perf firewire lguest slub usb virtio: FORCE
> > +	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
> > +
> > +cpupower: FORCE
> > +	$(QUIET_SUBDIR0)power/$@/ $(QUIET_SUBDIR1)
> > +
> > +turbostat x86_energy_perf_policy: FORCE
> > +	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
> > +
> > +firewire_clean lguest_clean perf_clean slub_clean usb_clean virtio_clean:

						^^^----------------------------------------------
											    	|
> > +	$(QUIET_SUBDIR0)$(@:_clean=)/ $(QUIET_SUBDIR1) clean				    	|
> >												|
> > +												|
> > +cp_clean:											|
> > +	$(QUIET_SUBDIR0)power/cpupower/ $(QUIET_SUBDIR1) clean					|
> > +												|
> > +turbostat_clean x86_energy_perf_policy_clean:						|
> > +	$(QUIET_SUBDIR0)power/x86/$(@:_clean=)/ $(QUIET_SUBDIR1) clean				|
> > +												|
> > +clean: cp_clean firewire_clean lguest_clean perf_clean slub_clean turbostat_clean \	|
> 												|
> slub_clean? Where is that target defined?							|
												|
Here---------------------------------------------------------------------------------------------

:-).

It basically does:

$ make -C slub/ clean

which runs the target of the same name in the Makefile in slub/.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 1/2] tools: Add Makefile.lib

[Sam Ravnborg]

On Thu, Mar 22, 2012 at 06:01:52PM +0100, Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Put generic enough build settings which could be reused by other tools
> into a common Makefile.lib file.

I assume that the reason to use the filename "Makefile.lib" is that
scripts/Makefile.lib uses the same extension.

But whoever idiot(*) that decided to use that extension deserve to be shot!
It is not a library per see, it is just a bunch of stuff you include
in another makefile.
So Makefile.include would be better - and as an added benefit vim
does not barf on it.

(*) I actually shave him quite often...

	Sam

Re: [PATCH 1/2] tools: Add Makefile.lib

[Borislav Petkov]

On Thu, Mar 22, 2012 at 10:07:32PM +0100, Sam Ravnborg wrote:
> On Thu, Mar 22, 2012 at 06:01:52PM +0100, Borislav Petkov wrote:
> > From: Borislav Petkov <borislav.petkov@amd.com>
> > 
> > Put generic enough build settings which could be reused by other tools
> > into a common Makefile.lib file.
> 
> I assume that the reason to use the filename "Makefile.lib" is that
> scripts/Makefile.lib uses the same extension.

Yep.

> But whoever idiot(*) that decided to use that extension deserve to be shot!

Ye^W^W.. oops, oh no, please no! :-)

> It is not a library per see, it is just a bunch of stuff you include
> in another makefile.
> So Makefile.include would be better - and as an added benefit vim
> does not barf on it.

Makes sense, will do, thanks.

> (*) I actually shave him quite often...

LOL.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 2/2] tools: Add a toplevel Makefile

[Christoph Lameter]

On Thu, 22 Mar 2012, Borislav Petkov wrote:

> It basically does:
>
> $ make -C slub/ clean
>
> which runs the target of the same name in the Makefile in slub/.

Ok.

Acked-by: Christoph Lameter <cl@linux.com>

Re: [PATCH 2/2] tools: Add a toplevel Makefile

[Pekka Enberg]

On Thu, 22 Mar 2012, Christoph Lameter wrote:
> On Thu, 22 Mar 2012, Borislav Petkov wrote:
> 
> > It basically does:
> >
> > $ make -C slub/ clean
> >
> > which runs the target of the same name in the Makefile in slub/.
> 
> Ok.
> 
> Acked-by: Christoph Lameter <cl@linux.com>

Didn't Andrew already patches that moved this under tools/vm and added 
proper Makefiles for it?

Re: [PATCH 2/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Mon, Mar 26, 2012 at 01:27:11PM +0300, Pekka Enberg wrote:
> Didn't Andrew already patches that moved this under tools/vm and added
> proper Makefiles for it?

Yeah, he did. It went upstream on Wed:

commit 63e315535abe0d820d0e3db4c06bc5de74aeefc8
Author: Dave Young <dyoung@redhat.com>
Date:   Wed Mar 28 14:42:55 2012 -0700

    mm: move slabinfo.c to tools/vm

    We have tools/vm/ folder for vm tools, so move slabinfo.c from tools/slub/
    to tools/vm/

I'll have to readjust my patchset then.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Wed, Mar 28, 2012 at 07:57:01PM +0200, Borislav Petkov wrote:
> On Wed, Mar 28, 2012 at 11:51:25AM +0200, Borislav Petkov wrote:
> > I'm currently thinking I want to give the make tools/{perf,help,...}
> > approach a try because it is clear that it is a subdir of the kernel src
> > tree and it probably could support all the targets we thought about.
> > 
> > Let me play around with it a bit...
> 
> Yeah, nasty. I've almost got it to build a tool when doing
> 
> $ make tools/<toolname>
> 
> but it fails on all the targets which have implicit build rules. (It
> seems to build perf fine though because perf Makefile doesn't have
> implicit rules for %.c -> %.o etc). And I can see that implicit rules
> are being cleared in the toplevel kernel Makefile so that they don't
> kick in.
> 
> At this moment, I'm open for hints from the kbuild people, if we use the
> toplevel Makefile to build tools/, should all the tools' Makefiles have
> explicit rules or ...? Any suggestions are appreciated.

Ok, I got it:

when make is run from the toplevel kernel Makefile, MAKEFLAGS contains
'--no-print-directory -RrI /home/boris/kernel/linux-2.6' which is
passed down to the sub-make and the '-r' especially turns off the use
of built-in implicit rules, thus the problem. Clearing MAKEFLAGS in the
sub-make fixes the issue, sorry for the noise.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Wed, Mar 28, 2012 at 11:51:25AM +0200, Borislav Petkov wrote:
> I'm currently thinking I want to give the make tools/{perf,help,...}
> approach a try because it is clear that it is a subdir of the kernel src
> tree and it probably could support all the targets we thought about.
> 
> Let me play around with it a bit...

Yeah, nasty. I've almost got it to build a tool when doing

$ make tools/<toolname>

but it fails on all the targets which have implicit build rules. (It
seems to build perf fine though because perf Makefile doesn't have
implicit rules for %.c -> %.o etc). And I can see that implicit rules
are being cleared in the toplevel kernel Makefile so that they don't
kick in.

At this moment, I'm open for hints from the kbuild people, if we use the
toplevel Makefile to build tools/, should all the tools' Makefiles have
explicit rules or ...? Any suggestions are appreciated.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Wed, Mar 28, 2012 at 09:25:28AM +0200, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > On Mon, Mar 26, 2012 at 05:14:32PM +0200, Ingo Molnar wrote:
> > > 
> > > * Borislav Petkov <bp@amd64.org> wrote:
> > > 
> > > > On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> > > > > Could this also be accessible via 'make tools' in the toplevel 
> > > > > Makefile?
> > > > > 
> > > > > I'd love to be able to type:
> > > > > 
> > > > >   make -j tools perf install
> > > > 
> > > > Well,
> > > > 
> > > > you could do
> > > > 
> > > > $ make -j -C tools perf
> > > 
> > > Arguably I could also type:
> > > 
> > >   cd tools/perf; make -j install
> > > 
> > > :-)
> > > 
> > > So I just wanted to bring this up, that integrating it into the 
> > > top level Makefile might make sense. Or not.
> > 
> > Right,
> > 
> > so from dealing with make in the last days, I can say that it's not a
> > fun walk in the park :). The problem is having multiple targets like the
> > following:
> > 
> > $ make -j tools perf install
> > 
> > I have to be able to differentiate in the Makefile which of the targets
> > is a directory ("tools"), which is the actual tool name to build
> > ("perf") and which is a special target ("install") which relates to the
> > tool name coming before it on the command line and is not the "install"
> > target of the main Makefile.
> > 
> > And it becomes nasty very fast if you reorder them
> > 
> > $ make -j install perf tools
> > 
> > where all that sequential info doesn't mean anything anymore.
> > 
> > So, the question is, what we actually want?
> > 
> > $ make -j tools perf install
> > 
> > is pretty cryptic wrt to which target we're actually building and having
> > the -C switch makes stuff a bit clearer IMHO:
> > 
> > $ make -j -C tools perf install
> > 
> > Also, having "install" as the last target means IMO to build the tool
> > before it and then install it.
> > 
> > In the end of the day, probably the most important thing is what is
> > the use case for tools/ which makes most sense. I definitely think the
> > 'help' target is a step in that direction. Being able to do
> > 
> > $ make -C tools
> > 
> > and it give you a short description is pretty helpful.
> > 
> > How about we have:
> > 
> > make tools-<toolname> install
> > 
> > or
> > 
> > make tools/perf install
> > 
> > from the toplevel kernel directory? Would that make more sense from a
> > usability perspective?
> 
> I have no strong preference currently. I think since it affects 
> kbuild it would be nice to know the opinion of the kbuild folks 
> (Cc:-ed): how should the integration of tools/ proceed?

Btw,

I'm currently thinking I want to give the make tools/{perf,help,...}
approach a try because it is clear that it is a subdir of the kernel src
tree and it probably could support all the targets we thought about.

Let me play around with it a bit...

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> On Mon, Mar 26, 2012 at 05:14:32PM +0200, Ingo Molnar wrote:
> > 
> > * Borislav Petkov <bp@amd64.org> wrote:
> > 
> > > On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> > > > Could this also be accessible via 'make tools' in the toplevel 
> > > > Makefile?
> > > > 
> > > > I'd love to be able to type:
> > > > 
> > > >   make -j tools perf install
> > > 
> > > Well,
> > > 
> > > you could do
> > > 
> > > $ make -j -C tools perf
> > 
> > Arguably I could also type:
> > 
> >   cd tools/perf; make -j install
> > 
> > :-)
> > 
> > So I just wanted to bring this up, that integrating it into the 
> > top level Makefile might make sense. Or not.
> 
> Right,
> 
> so from dealing with make in the last days, I can say that it's not a
> fun walk in the park :). The problem is having multiple targets like the
> following:
> 
> $ make -j tools perf install
> 
> I have to be able to differentiate in the Makefile which of the targets
> is a directory ("tools"), which is the actual tool name to build
> ("perf") and which is a special target ("install") which relates to the
> tool name coming before it on the command line and is not the "install"
> target of the main Makefile.
> 
> And it becomes nasty very fast if you reorder them
> 
> $ make -j install perf tools
> 
> where all that sequential info doesn't mean anything anymore.
> 
> So, the question is, what we actually want?
> 
> $ make -j tools perf install
> 
> is pretty cryptic wrt to which target we're actually building and having
> the -C switch makes stuff a bit clearer IMHO:
> 
> $ make -j -C tools perf install
> 
> Also, having "install" as the last target means IMO to build the tool
> before it and then install it.
> 
> In the end of the day, probably the most important thing is what is
> the use case for tools/ which makes most sense. I definitely think the
> 'help' target is a step in that direction. Being able to do
> 
> $ make -C tools
> 
> and it give you a short description is pretty helpful.
> 
> How about we have:
> 
> make tools-<toolname> install
> 
> or
> 
> make tools/perf install
> 
> from the toplevel kernel directory? Would that make more sense from a
> usability perspective?

I have no strong preference currently. I think since it affects 
kbuild it would be nice to know the opinion of the kbuild folks 
(Cc:-ed): how should the integration of tools/ proceed?

Thanks,

	Ingo

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Mon, Mar 26, 2012 at 05:14:32PM +0200, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@amd64.org> wrote:
> 
> > On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> > > Could this also be accessible via 'make tools' in the toplevel 
> > > Makefile?
> > > 
> > > I'd love to be able to type:
> > > 
> > >   make -j tools perf install
> > 
> > Well,
> > 
> > you could do
> > 
> > $ make -j -C tools perf
> 
> Arguably I could also type:
> 
>   cd tools/perf; make -j install
> 
> :-)
> 
> So I just wanted to bring this up, that integrating it into the 
> top level Makefile might make sense. Or not.

Right,

so from dealing with make in the last days, I can say that it's not a
fun walk in the park :). The problem is having multiple targets like the
following:

$ make -j tools perf install

I have to be able to differentiate in the Makefile which of the targets
is a directory ("tools"), which is the actual tool name to build
("perf") and which is a special target ("install") which relates to the
tool name coming before it on the command line and is not the "install"
target of the main Makefile.

And it becomes nasty very fast if you reorder them

$ make -j install perf tools

where all that sequential info doesn't mean anything anymore.

So, the question is, what we actually want?

$ make -j tools perf install

is pretty cryptic wrt to which target we're actually building and having
the -C switch makes stuff a bit clearer IMHO:

$ make -j -C tools perf install

Also, having "install" as the last target means IMO to build the tool
before it and then install it.

In the end of the day, probably the most important thing is what is
the use case for tools/ which makes most sense. I definitely think the
'help' target is a step in that direction. Being able to do

$ make -C tools

and it give you a short description is pretty helpful.

How about we have:

make tools-<toolname> install

or

make tools/perf install

from the toplevel kernel directory? Would that make more sense from a
usability perspective?

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> > Could this also be accessible via 'make tools' in the toplevel 
> > Makefile?
> > 
> > I'd love to be able to type:
> > 
> >   make -j tools perf install
> 
> Well,
> 
> you could do
> 
> $ make -j -C tools perf

Arguably I could also type:

  cd tools/perf; make -j install

:-)

So I just wanted to bring this up, that integrating it into the 
top level Makefile might make sense. Or not.

Thanks,

	Ingo

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Mon, Mar 26, 2012 at 02:26:32PM +0200, Borislav Petkov wrote:
> On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> > Could this also be accessible via 'make tools' in the toplevel 
> > Makefile?
> > 
> > I'd love to be able to type:
> > 
> >   make -j tools perf install
> 
> Well,
> 
> you could do
> 
> $ make -j -C tools perf
> 
> which builds perf. I'd need to make it work with the install target too
> so that when you do
> 
> $ make -j -C tools perf install
> 
> it knows that you mean the install target of perf. Let me play with it a
> bit ...

Yep, we can do that, below is a possible version:

This way you can do

$ make -j -C tools <tool> install

for example. It cannot handle multiple tools yet, i.e.

$ make -j -C tools <tool1> <tool2> install

because it doesn't seem make a whole lot of sense to me right now
(besides, make is really clumsy for such stuff :)

help is still the default target when you do

$ make -C tools

with no explicit build target.

Hmm...

--
diff --git a/tools/Makefile b/tools/Makefile
index 25566cd74937..03fa9432d54b 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -1,5 +1,11 @@
 include scripts/Makefile.include
 
+TOOL:=perf
+
+ifeq ($(findstring install,$(MAKECMDGOALS)), install)
+	TOOL:=$(filter-out install, $(MAKECMDGOALS))
+endif
+
 help:
 	@echo 'Possible targets:'
 	@echo ''
@@ -13,13 +19,18 @@ help:
 	@echo '  virtio     - vhost test module'
 	@echo '  x86_energy_perf_policy - Intel energy policy tool'
 	@echo ''
+	@echo 'You can do:'
+	@echo ' $$ make -C tools/ <tool> install'
+	@echo ''
+	@echo '  from the kernel command line to build and install one of the tools above'
+	@echo ''
 	@echo 'Cleaning targets:'
 	@echo ''
 	@echo '  all of the above with the "_clean" string appended cleans'
 	@echo '    the respective build directory.'
-	@echo '  clean: a summary clean target to clean _all_ folders'
+	@echo '  cleanall: a summary clean target to clean _all_ folders'
 
-perf firewire lguest slub usb virtio: FORCE
+firewire lguest perf slub usb virtio: FORCE
 	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
 
 cpupower: FORCE
@@ -28,16 +39,31 @@ cpupower: FORCE
 turbostat x86_energy_perf_policy: FORCE
 	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
 
+install: $(TOOL)
+ifeq ($(TOOL),cpupower)
+	$(MAKE) -C power/$(TOOL) install
+else
+  ifeq ($(TOOL),turbostat)
+	$(MAKE) -C power/x86/$(TOOL) install
+  else
+    ifeq ($(TOOL),x86_energy_perf_policy)
+	$(MAKE) -C power/x86/$(TOOL) install
+    else
+	$(MAKE) -C $(TOOL)/ install
+    endif
+  endif
+endif
+
 firewire_clean lguest_clean perf_clean slub_clean usb_clean virtio_clean:
 	$(QUIET_SUBDIR0)$(@:_clean=)/ $(QUIET_SUBDIR1) clean
 
-cp_clean:
+cpupower_clean:
 	$(QUIET_SUBDIR0)power/cpupower/ $(QUIET_SUBDIR1) clean
 
 turbostat_clean x86_energy_perf_policy_clean:
 	$(QUIET_SUBDIR0)power/x86/$(@:_clean=)/ $(QUIET_SUBDIR1) clean
 
-clean: cp_clean firewire_clean lguest_clean perf_clean slub_clean turbostat_clean \
+cleanall: cpupower_clean firewire_clean lguest_clean perf_clean slub_clean turbostat_clean \
 		usb_clean virtio_clean x86_energy_perf_policy_clean
 
 .PHONY: FORCE

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

On Sat, Mar 24, 2012 at 08:44:12AM +0100, Ingo Molnar wrote:
> Could this also be accessible via 'make tools' in the toplevel 
> Makefile?
> 
> I'd love to be able to type:
> 
>   make -j tools perf install

Well,

you could do

$ make -j -C tools perf

which builds perf. I'd need to make it work with the install target too
so that when you do

$ make -j -C tools perf install

it knows that you mean the install target of perf. Let me play with it a
bit ...

[..]

> Btw., please fix the perf line to say:
> 
> s/performance measurements tool/performance measurement and analysis tool

Done,
thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

Re: [PATCH 0/2] tools: Add a toplevel Makefile

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> Hi all,
> 
> here's a refreshed version from yesterday incorporating all comments and
> suggestions along with a third patch that adds a 'help' target as the
> default one causing the following below. Btw, Arnaldo, could you please
> pick those up if there are no complaints since the first patch touches
> perf and I don't have a clear idea who else to send it to anyway :).
> 
> Thanks.
> 
> $ make

Could this also be accessible via 'make tools' in the toplevel 
Makefile?

I'd love to be able to type:

  make -j tools perf install

But ... your current patches are an improvement as well, this 
list alone:

> $ make
> Possible targets:
> 
>   cpupower   - a tool for all things x86 CPU power
>   firewire   - the userspace part of nosy, an IEEE-1394 traffic sniffer
>   lguest     - a minimal 32-bit x86 hypervisor
>   perf       - Linux performance measurements tool
>   slub       - slabs reporting tool
>   turbostat  - Intel CPU idle stats and freq reporting tool
>   usb        - USB testing tools
>   virtio     - vhost test module
>   x86_energy_perf_policy - Intel energy policy tool

is already pretty useful.

Btw., please fix the perf line to say:

s/performance measurements tool/performance measurement and analysis tool

Thanks,

	Ingo

[PATCH 0/2] tools: Add a toplevel Makefile

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Hi all,

here's a refreshed version from yesterday incorporating all comments and
suggestions along with a third patch that adds a 'help' target as the
default one causing the following below. Btw, Arnaldo, could you please
pick those up if there are no complaints since the first patch touches
perf and I don't have a clear idea who else to send it to anyway :).

Thanks.

$ make
Possible targets:

  cpupower   - a tool for all things x86 CPU power
  firewire   - the userspace part of nosy, an IEEE-1394 traffic sniffer
  lguest     - a minimal 32-bit x86 hypervisor
  perf       - Linux performance measurements tool
  slub       - slabs reporting tool
  turbostat  - Intel CPU idle stats and freq reporting tool
  usb        - USB testing tools
  virtio     - vhost test module
  x86_energy_perf_policy - Intel energy policy tool

Cleaning targets:

  all of the above with the "_clean" string appended cleans
    the respective build directory.
  clean: a summary clean target to clean _all_ folders


Changelog:

* v1:

this is a refresh and carve-out of an old patchset. It adds a toplevel
Makefile to tools/ so that one can build the tool of her/his liking by
simply doing

$ cd tools/
$ make <toolname>

By default, we build perf. There's also a scripts/Makefile.lib now which
should contain all make-related generic stuff which can be used by all
tools' build process after including this file.

</Changelog>

Any comments/suggestions are welcome,
thanks.