[PATCH v3 0/4] tools: Add a toplevel Makefile

[PATCH v3 0/4] tools: Add a toplevel Makefile

[Borislav Petkov]

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


Hi all,

yet another version of the toplevel Makefile integration of tools/.
This round gives you the ability to build the tools from the toplevel
Makefile (explanation below can be found also in patch 4/4's commit
message):

"Now you can do

$ make tools/<toolname>

from the toplevel kernel directory and have the respective tool built.

If you want to build and install it, do

$ make tools/<toolname> tinstall

The install target is called "tinstall" so that there's no conflict with
the main kernel install target and should mean "tool install".

$ make tools/ <toolname>_clean

should clean the respective tool directories.

If you want to clean all in tools, simply do

$ make  tools/ cleanall

Also, if you want to get what the possible targets are, simply calling

$ make tools/

should give you the short help."

Also included are all suggestions from the last time.

Thanks.

Changelog:

* v2:

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


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

[PATCH 1/4] tools: Add Makefile.include

[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.include file.

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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 8a4b9bccf8b2..e5f0fb5a4670 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -1,18 +1,10 @@
-ifeq ("$(origin O)", "command line")
-	OUTPUT := $(O)/
-endif
+include ../scripts/Makefile.include
 
 # 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.include b/tools/scripts/Makefile.include
new file mode 100644
index 000000000000..a2586d13bcf8
--- /dev/null
+++ b/tools/scripts/Makefile.include
@@ -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/4] tools: Add a toplevel Makefile

[Borislav Petkov]

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

Add a Makefile with all the targets under tools/. Also, and add a
minimalistic Makefile to slub/ for completeness.

Cc: Pekka Enberg <penberg@kernel.org>
Acked-by: Christoph Lameter <cl@linux.com>
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..134b3604b0b3
--- /dev/null
+++ b/tools/Makefile
@@ -0,0 +1,24 @@
+include scripts/Makefile.include
+
+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

[PATCH 3/4] tools: Add a help target

[Borislav Petkov]

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

... and make it the default one so that calling 'make' without arguments
in the tools/ directory gives you the possible targets to build along
with a short description of what they are.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 tools/Makefile |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/Makefile b/tools/Makefile
index 134b3604b0b3..25566cd74937 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -1,5 +1,24 @@
 include scripts/Makefile.include
 
+help:
+	@echo 'Possible targets:'
+	@echo ''
+	@echo '  cpupower   - a tool for all things x86 CPU power'
+	@echo '  firewire   - the userspace part of nosy, an IEEE-1394 traffic sniffer'
+	@echo '  lguest     - a minimal 32-bit x86 hypervisor'
+	@echo '  perf       - Linux performance measurement and analysis tool'
+	@echo '  slub       - slabs reporting tool'
+	@echo '  turbostat  - Intel CPU idle stats and freq reporting tool'
+	@echo '  usb        - USB testing tools'
+	@echo '  virtio     - vhost test module'
+	@echo '  x86_energy_perf_policy - Intel energy policy tool'
+	@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'
+
 perf firewire lguest slub usb virtio: FORCE
 	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
 
-- 
1.7.9.3.362.g71319

[PATCH 4/4] tools: Connect to the kernel build system

[Borislav Petkov]

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

Now you can do

$ make tools/<toolname>

from the toplevel kernel directory and have the respective tool built.

If you want to build and install it, do

$ make tools/<toolname> tinstall

The install target is called "tinstall" so that there's no conflict with
the main kernel install target and should mean "tool install".

$ make tools/ <toolname>_clean

should clean the respective tool directories.

If you want to clean all in tools, simply do

$ make  tools/ cleanall

Also, if you want to get what the possible targets are, simply calling

$ make tools/

should give you the short help.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 Makefile       |   11 +++++++++++
 tools/Makefile |   31 +++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 1932984478c1..9e85648dd745 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,6 +1468,17 @@ kernelrelease:
 kernelversion:
 	@echo $(KERNELVERSION)
 
+# empty targets used in tools/Makefile, defined here to suppress error message
+tinstall:
+cleanall:
+
+# Clear a bunch of variables before executing the submake
+tools/: FORCE
+	$(Q)$(MAKE) LDFLAGS= TOOL=help -C tools/ $(patsubst tools/%,%,$(MAKECMDGOALS))
+
+tools/%: FORCE
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS= TOOL=$* -C $(src)/tools/ $(patsubst tools/%,%,$(MAKECMDGOALS))
+
 # Single targets
 # ---------------------------------------------------------------------------
 # Single targets are compatible with:
diff --git a/tools/Makefile b/tools/Makefile
index 25566cd74937..dcc86b42021b 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -1,5 +1,7 @@
 include scripts/Makefile.include
 
+default: $(TOOL)
+
 help:
 	@echo 'Possible targets:'
 	@echo ''
@@ -13,13 +15,19 @@ 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> tinstall'
+	@echo ''
+	@echo '  from the kernel command line to build and install one of'
+	@echo '  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 mytest: FORCE
 	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
 
 cpupower: FORCE
@@ -28,16 +36,31 @@ cpupower: FORCE
 turbostat x86_energy_perf_policy: FORCE
 	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
 
+tinstall: $(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
-- 
1.7.9.3.362.g71319

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

[Sam Ravnborg]

On Thu, Mar 29, 2012 at 02:25:53PM +0200, Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> 
> Hi all,
> 
> yet another version of the toplevel Makefile integration of tools/.
> This round gives you the ability to build the tools from the toplevel
> Makefile (explanation below can be found also in patch 4/4's commit
> message):
> 
> "Now you can do
> 
> $ make tools/<toolname>
Makes sense.
> 
> from the toplevel kernel directory and have the respective tool built.
> 
> If you want to build and install it, do
> 
> $ make tools/<toolname> tinstall
But this makes no sense.

It would be better to be consistent - so the user does not need to remember
when to add a space and when not.

make tools/<command> where <command> is one of help, install, clean, "nothing"
make tools/<toolname>
make tools/<toolname>_<comand> where command is the same set of commands


then a user could do:

    make tools/clean
    make tools/perf
    make tools/perf_install

or

    make tools/clean
    make tools/
    make tools/install


The install target could implicitly include the build target.

With this scheme the user is up to less suprises.

All the above are only minor adjustments compared to what you already did.
bt the consistency here is a gain (IMO).

	Sam

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

[Borislav Petkov]

On Fri, Mar 30, 2012 at 07:26:05AM +0200, Sam Ravnborg wrote:
> > $ make tools/<toolname> tinstall
> But this makes no sense.
> 
> It would be better to be consistent - so the user does not need to remember
> when to add a space and when not.
> 
> make tools/<command> where <command> is one of help, install, clean, "nothing"
> make tools/<toolname>
> make tools/<toolname>_<comand> where command is the same set of commands
> 
> 
> then a user could do:
> 
>     make tools/clean
>     make tools/perf
>     make tools/perf_install
> 
> or
> 
>     make tools/clean
>     make tools/
>     make tools/install

This one I had hard time imagining: who would install all tools but I
guess it could have it's use...

> The install target could implicitly include the build target.
> 
> With this scheme the user is up to less suprises.
> 
> All the above are only minor adjustments compared to what you already did.
> bt the consistency here is a gain (IMO).

... but yeah, those make sense to me too, let's see what the others
think, Arnaldo, Ingo?

-- 
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 v3 0/4] tools: Add a toplevel Makefile

[Ingo Molnar]

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

> On Fri, Mar 30, 2012 at 07:26:05AM +0200, Sam Ravnborg wrote:
> > > $ make tools/<toolname> tinstall
> > But this makes no sense.
> > 
> > It would be better to be consistent - so the user does not need to remember
> > when to add a space and when not.
> > 
> > make tools/<command> where <command> is one of help, install, clean, "nothing"
> > make tools/<toolname>
> > make tools/<toolname>_<comand> where command is the same set of commands
> > 
> > 
> > then a user could do:
> > 
> >     make tools/clean
> >     make tools/perf
> >     make tools/perf_install
> > 
> > or
> > 
> >     make tools/clean
> >     make tools/
> >     make tools/install
> 
> This one I had hard time imagining: who would install all 
> tools but I guess it could have it's use...

regression testing?

> > The install target could implicitly include the build 
> > target.
> > 
> > With this scheme the user is up to less suprises.
> > 
> > All the above are only minor adjustments compared to what 
> > you already did. bt the consistency here is a gain (IMO).
> 
> ... but yeah, those make sense to me too, let's see what the 
> others think, Arnaldo, Ingo?

Well, if Sam and Michal are fine with it I'm a happy camper.

One question. Instead of:

  make tools/perf_install

Couldnt we beat kbuild into submission to allow the much more 
obvious:

  make tools/perf install

?

I don't think anyone would expect the *kernel* to be installed 
in such a circumstance - so it's only a question of making the 
Makefile understand it, right?

Thanks,

	Ingo

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

[Sam Ravnborg]

> 
> One question. Instead of:
> 
>   make tools/perf_install
> 
> Couldnt we beat kbuild into submission to allow the much more 
> obvious:
> 
>   make tools/perf install
> 
> ?
It is more obvious if you look at it alone.
But when you look at it with the other commands then you suddenly
end up confused when you need to specify the command as a
separate target "tools/perf install - and when it is just
one target "tools/perf_install".

> 
> I don't think anyone would expect the *kernel* to be installed 
> in such a circumstance - so it's only a question of making the 
> Makefile understand it, right?
Make will try to update the two targets "tools/perf" and "install"
in parallel. And it does not look easy to teach make that when you
specify the target "tools/*" then the install target should just
be ignored and passed down to the sub-make.

Anything that adds more complexity to the top-level Makefile should
be avoided if at all possible. It is un-maintainable as-is.
And the consistency issue is also important.

I know that if I do "make install" the kernel will be installed.
So one could argue that the same should apply to
the targets below tools/.
But then this should be for all targets and not just install.
If someone come up with a clean way to do so it is fine.
but the original proposal with "tinstall" just do not cut it.

	Sam

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

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> > 
> > One question. Instead of:
> > 
> >   make tools/perf_install
> > 
> > Couldnt we beat kbuild into submission to allow the much more 
> > obvious:
> > 
> >   make tools/perf install
> > 
> > ?
> It is more obvious if you look at it alone.
> But when you look at it with the other commands then you suddenly
> end up confused when you need to specify the command as a
> separate target "tools/perf install - and when it is just
> one target "tools/perf_install".
> 
> > 
> > I don't think anyone would expect the *kernel* to be installed 
> > in such a circumstance - so it's only a question of making the 
> > Makefile understand it, right?
> Make will try to update the two targets "tools/perf" and "install"
> in parallel. And it does not look easy to teach make that when you
> specify the target "tools/*" then the install target should just
> be ignored and passed down to the sub-make.
> 
> Anything that adds more complexity to the top-level Makefile should
> be avoided if at all possible. It is un-maintainable as-is.
> And the consistency issue is also important.
> 
> I know that if I do "make install" the kernel will be installed.
> So one could argue that the same should apply to
> the targets below tools/.
> But then this should be for all targets and not just install.
> If someone come up with a clean way to do so it is fine.
> but the original proposal with "tinstall" just do not cut it.

'tinstall' is definitely out, no argument about that.

Viable options are:

  tools/perf install
  tools/perf_install
  tools/perf-install

I'm fine with either one.

Thanks,

	Ingo

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

[Borislav Petkov]

On Sun, Apr 01, 2012 at 10:42:53AM +0200, Ingo Molnar wrote:
> > Make will try to update the two targets "tools/perf" and "install"
> > in parallel. And it does not look easy to teach make that when you
> > specify the target "tools/*" then the install target should just
> > be ignored and passed down to the sub-make.
> > 
> > Anything that adds more complexity to the top-level Makefile should
> > be avoided if at all possible. It is un-maintainable as-is.
> > And the consistency issue is also important.
> > 
> > I know that if I do "make install" the kernel will be installed.
> > So one could argue that the same should apply to
> > the targets below tools/.
> > But then this should be for all targets and not just install.
> > If someone come up with a clean way to do so it is fine.
> > but the original proposal with "tinstall" just do not cut it.
> 
> 'tinstall' is definitely out, no argument about that.
> 
> Viable options are:
> 
>   tools/perf install
>   tools/perf_install
>   tools/perf-install
> 
> I'm fine with either one.

What Sam said - it is not that easy and probably cannot be done
without trickery to tell make that "install" becomes a sub-target when
"tools/<toolname>" is in front of it.

OTOH, "perf_install" is easily doable and the "perf-install" one with
the "-" could be misleading since all make targets have a "_" in their
names.

I'll redo the patchset with <toolname>_install targets since this is the
most straightforward option. In that case, if one wants to build and
install perf, she simply does

$ make tools/perf_install

and the same would work for the rest of the tools in there.

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 v3 0/4] tools: Add a toplevel Makefile

[Milton Miller]

[fix a missing comma in cc ]

On Sat, 31 Mar 2012 20:49:06 +0200, Sam Ravnborg wrote:
> 
> > 
> > One question. Instead of:
> > 
> >   make tools/perf_install
> > 
> > Couldnt we beat kbuild into submission to allow the much more 
> > obvious:
> > 
> >   make tools/perf install
> > 
> > ?
> It is more obvious if you look at it alone.
> But when you look at it with the other commands then you suddenly
> end up confused when you need to specify the command as a
> separate target "tools/perf install - and when it is just
> one target "tools/perf_install".
> 
> > 
> > I don't think anyone would expect the *kernel* to be installed 
> > in such a circumstance - so it's only a question of making the 
> > Makefile understand it, right?
>
> Make will try to update the two targets "tools/perf" and "install"
> in parallel. And it does not look easy to teach make that when you
> specify the target "tools/*" then the install target should just
> be ignored and passed down to the sub-make.

When I saw this concept, my thought was we should add a T= option,
similar to M= option to build a single module.  The T would take
the path under tools/ .  This would also be similar to how we add O=
for output directory and M= for building "external" modules (and also
similar to $(build)= for subdirectories).

> Anything that adds more complexity to the top-level Makefile should
> be avoided if at all possible. It is un-maintainable as-is.
> And the consistency issue is also important.

I think this could be a simple rule, if we find the variable on the
command line we pass everything to the tools Makefile (after processing
O= I guess).

I'll leave the implementation to someone else, I have more than enough
on my plate right now.

milton

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

[Borislav Petkov]

On Sun, Apr 01, 2012 at 11:22:36AM +0200, Borislav Petkov wrote:
> and the same would work for the rest of the tools in there.

Ok, instead of spamming the ML, I'm sending only the 4th patch which
basically implements what Sam suggested.

Thanks.

--
>From 1ac61da491fa4db8fb4e151b0daa3f4415bfd44d Mon Sep 17 00:00:00 2001
From: Borislav Petkov <borislav.petkov@amd.com>
Date: Tue, 27 Mar 2012 11:50:47 +0200
Subject: [PATCH] tools: Connect to the kernel build system

Now you can do

$ make tools/<toolname>

from the toplevel kernel directory and have the respective tool built.

If you want to build and install it, do

$ make tools/<toolname>_install

$ make tools/<toolname>_clean

should clean the respective tool directories.

If you want to clean all in tools, simply do

$ make tools/clean

Also, if you want to get what the possible targets are, simply calling

$ make tools/

should give you the short help.

$ make tools/install

installs all tools, of course. Doh.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 Makefile       |    7 +++++++
 tools/Makefile |   31 +++++++++++++++++++++++++++----
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 1932984478c1..a277c0474500 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,6 +1468,13 @@ kernelrelease:
 kernelversion:
 	@echo $(KERNELVERSION)
 
+# Clear a bunch of variables before executing the submake
+tools/: FORCE
+	$(Q)$(MAKE) LDFLAGS= -C $(src)/tools/
+
+tools/%: FORCE
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS= -C $(src)/tools/ $*
+
 # Single targets
 # ---------------------------------------------------------------------------
 # Single targets are compatible with:
diff --git a/tools/Makefile b/tools/Makefile
index 25566cd74937..3c850de935f8 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -13,13 +13,23 @@ 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'
+	@echo '  the tools above'
+	@echo ''
+	@echo '  $$ make tools/install'
+	@echo ''
+	@echo '  installs all tools.'
+	@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'
 
-perf firewire lguest slub usb virtio: FORCE
+firewire lguest perf slub usb virtio: FORCE
 	$(QUIET_SUBDIR0)$@/ $(QUIET_SUBDIR1)
 
 cpupower: FORCE
@@ -28,16 +38,29 @@ cpupower: FORCE
 turbostat x86_energy_perf_policy: FORCE
 	$(QUIET_SUBDIR0)power/x86/$@/ $(QUIET_SUBDIR1)
 
+firewire_install lguest_install perf_install slub_install usb_install virtio_install:
+	$(QUIET_SUBDIR0)$(@:_install=)/ $(QUIET_SUBDIR1) install
+
+cpupower_install:
+	$(QUIET_SUBDIR0)power/$(@:_install=)/ $(QUIET_SUBDIR1) install
+
+turbostat_install x86_energy_perf_policy_install:
+	$(QUIET_SUBDIR0)power/x86/$(@:_install=)/ $(QUIET_SUBDIR1) install
+
+install: firewire_install lguest_install perf_install slub_install \
+		usb_install virtio_install cpupower_install turbostat_install \
+		x86_energy_perf_policy_install
+
 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 \
-		usb_clean virtio_clean x86_energy_perf_policy_clean
+clean: cpupower_clean firewire_clean lguest_clean perf_clean slub_clean \
+		turbostat_clean usb_clean virtio_clean x86_energy_perf_policy_clean
 
 .PHONY: FORCE
-- 
1.7.9.3.362.g71319


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