cpupower: add Makefile dependencies for install targets

PATCH In review Ivan Babrou 3 participants 7 messages Jan 4, 2021 – Jan 22, 2021 CPU power monitoring

lore

Share & tools

lore

Ivan Babrou

author

2021-01-04 23:58 UTC #0 lore raw

PATCH 1/1 cpupower: add Makefile dependencies for install targets

This allows building cpupower in parallel rather than serially.

Signed-off-by Ivan Babrou <[email protected]>

---
 tools/power/cpupower/Makefile       | 8 ++++----
 tools/power/cpupower/bench/Makefile | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

tools/power/cpupower/Makefile +4 -4

index c7bcddbd486d..3b1594447f29 100644
@@ -270,14 +270,14 @@ clean:
 	$(MAKE) -C bench O=$(OUTPUT) clean
 
 
-install-lib:
+install-lib: libcpupower
 	$(INSTALL) -d $(DESTDIR)${libdir}
 	$(CP) $(OUTPUT)libcpupower.so* $(DESTDIR)${libdir}/
 	$(INSTALL) -d $(DESTDIR)${includedir}
 	$(INSTALL_DATA) lib/cpufreq.h $(DESTDIR)${includedir}/cpufreq.h
 	$(INSTALL_DATA) lib/cpuidle.h $(DESTDIR)${includedir}/cpuidle.h
 
-install-tools:
+install-tools: $(OUTPUT)cpupower
 	$(INSTALL) -d $(DESTDIR)${bindir}
 	$(INSTALL_PROGRAM) $(OUTPUT)cpupower $(DESTDIR)${bindir}
 	$(INSTALL) -d $(DESTDIR)${bash_completion_dir}

@@ -293,14 +293,14 @@ install-man:
 	$(INSTALL_DATA) -D man/cpupower-info.1 $(DESTDIR)${mandir}/man1/cpupower-info.1
 	$(INSTALL_DATA) -D man/cpupower-monitor.1 $(DESTDIR)${mandir}/man1/cpupower-monitor.1
 
-install-gmo:
+install-gmo: create-gmo
 	$(INSTALL) -d $(DESTDIR)${localedir}
 	for HLANG in $(LANGUAGES); do \
 		echo '$(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo $(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo'; \
 		$(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo $(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo; \
 	done;
 
-install-bench:
+install-bench: compile-bench
 	@#DESTDIR must be set from outside to survive
 	@sbindir=$(sbindir) bindir=$(bindir) docdir=$(docdir) confdir=$(confdir) $(MAKE) -C bench O=$(OUTPUT) install

tools/power/cpupower/bench/Makefile +1 -1

index f68b4bc55273..d9d9923af85c 100644
@@ -27,7 +27,7 @@ $(OUTPUT)cpufreq-bench: $(OBJS)
 
 all: $(OUTPUT)cpufreq-bench
 
-install:
+install: $(OUTPUT)cpufreq-bench
 	mkdir -p $(DESTDIR)/$(sbindir)
 	mkdir -p $(DESTDIR)/$(bindir)
 	mkdir -p $(DESTDIR)/$(docdir)

Signature · 1 line

2.29.2

Thomas Renninger

maintainer

2021-01-07 10:08 UTC #1 lore raw

Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:

Quoted 1 line

This allows building cpupower in parallel rather than serially.

cpupower is built serially:

[ make clean ]

time make

real    0m3,742s
user    0m3,330s
sys     0m1,105s

[ make clean ]

time make -j10

real    0m1,045s
user    0m3,153s
sys     0m1,037s

Only advantage I see is that you can call make install-xy
targets without calling the corresponding build target make xy
similar to the general install target:
install: all install-lib ...

Not sure anyone needs this and whether all targets successfully work this way.
If you'd show a useful usecase example...

Thomas

Quoted 42 lines

Signed-off-by Ivan Babrou <[email protected]>

---
 tools/power/cpupower/Makefile       | 8 ++++----
 tools/power/cpupower/bench/Makefile | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

tools/power/cpupower/Makefile +2 -2

index c7bcddbd486d..3b1594447f29 100644
@@ -270,14 +270,14 @@ clean:
 	$(MAKE) -C bench O=$(OUTPUT) clean
 
 
-install-lib:
+install-lib: libcpupower
 	$(INSTALL) -d $(DESTDIR)${libdir}
 	$(CP) $(OUTPUT)libcpupower.so* $(DESTDIR)${libdir}/
 	$(INSTALL) -d $(DESTDIR)${includedir}
 	$(INSTALL_DATA) lib/cpufreq.h $(DESTDIR)${includedir}/cpufreq.h
 	$(INSTALL_DATA) lib/cpuidle.h $(DESTDIR)${includedir}/cpuidle.h
 
-install-tools:
+install-tools: $(OUTPUT)cpupower
 	$(INSTALL) -d $(DESTDIR)${bindir}
 	$(INSTALL_PROGRAM) $(OUTPUT)cpupower $(DESTDIR)${bindir}
 	$(INSTALL) -d $(DESTDIR)${bash_completion_dir}

@@ -293,14 +293,14 @@ install-man:
 	$(INSTALL_DATA) -D man/cpupower-info.1

$(DESTDIR)${mandir}/man1/cpupower-info.1 $(INSTALL_DATA) -D man/cpupower-monitor.1 $(DESTDIR)${mandir}/man1/cpupower-monitor.1

tools/power/cpupower/Makefile +1 -1

-install-gmo:
+install-gmo: create-gmo
 	$(INSTALL) -d $(DESTDIR)${localedir}
 	for HLANG in $(LANGUAGES); do \
 		echo '$(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo

$(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo'; \ $(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo $(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo; \ done;

tools/power/cpupower/Makefile install-man:

Quoted 4 lines

tools/power/cpupower/Makefile +1 -1

-install-bench:
+install-bench: compile-bench
 	@#DESTDIR must be set from outside to survive
 	@sbindir=$(sbindir) bindir=$(bindir) docdir=$(docdir) confdir=$

(confdir)

Quoted 16 lines

$(MAKE) -C bench O=$(OUTPUT) install
(quoted diff) +0 -0
b/tools/power/cpupower/bench/Makefile index f68b4bc55273..d9d9923af85c 100644
tools/power/cpupower/bench/Makefile +1 -1
@@ -27,7 +27,7 @@ $(OUTPUT)cpufreq-bench: $(OBJS)
 
 all: $(OUTPUT)cpufreq-bench
 
-install:
+install: $(OUTPUT)cpufreq-bench
 	mkdir -p $(DESTDIR)/$(sbindir)
 	mkdir -p $(DESTDIR)/$(bindir)
 	mkdir -p $(DESTDIR)/$(docdir)

Ivan Babrou

author

2021-01-07 17:43 UTC #2 lore raw

Quoted 27 lines · Thomas Renninger

Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:
Quoted 1 line
This allows building cpupower in parallel rather than serially.
cpupower is built serially:
[ make clean ]
time make
real    0m3,742s
user    0m3,330s
sys     0m1,105s
[ make clean ]
time make -j10
real    0m1,045s
user    0m3,153s
sys     0m1,037s
Only advantage I see is that you can call make install-xy
targets without calling the corresponding build target make xy
similar to the general install target:
install: all install-lib ...

Quoted 3 lines

Not sure anyone needs this and whether all targets successfully work this way.
If you'd show a useful usecase example...

We build a bunch of kernel related tools (perf, cpupower, bpftool, etc.) from our own top level Makefile, propagating parallelism downwards like one should. Without this patch we have to remove parallelism for cpupower, which doesn't seem like a very clean thing to do, especially if you consider that it's 3x faster with parallelism enabled in wall clock terms.

Quoted 69 lines

Thomas

Quoted 42 lines

Signed-off-by Ivan Babrou <[email protected]>

---
 tools/power/cpupower/Makefile       | 8 ++++----
 tools/power/cpupower/bench/Makefile | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

tools/power/cpupower/Makefile +2 -2

index c7bcddbd486d..3b1594447f29 100644
@@ -270,14 +270,14 @@ clean:
      $(MAKE) -C bench O=$(OUTPUT) clean
 
 
-install-lib:
+install-lib: libcpupower
      $(INSTALL) -d $(DESTDIR)${libdir}
      $(CP) $(OUTPUT)libcpupower.so* $(DESTDIR)${libdir}/
      $(INSTALL) -d $(DESTDIR)${includedir}
      $(INSTALL_DATA) lib/cpufreq.h $(DESTDIR)${includedir}/cpufreq.h
      $(INSTALL_DATA) lib/cpuidle.h $(DESTDIR)${includedir}/cpuidle.h
 
-install-tools:
+install-tools: $(OUTPUT)cpupower
      $(INSTALL) -d $(DESTDIR)${bindir}
      $(INSTALL_PROGRAM) $(OUTPUT)cpupower $(DESTDIR)${bindir}
      $(INSTALL) -d $(DESTDIR)${bash_completion_dir}

@@ -293,14 +293,14 @@ install-man:
      $(INSTALL_DATA) -D man/cpupower-info.1

$(DESTDIR)${mandir}/man1/cpupower-info.1 $(INSTALL_DATA) -D man/cpupower-monitor.1 $(DESTDIR)${mandir}/man1/cpupower-monitor.1

tools/power/cpupower/Makefile +1 -1

-install-gmo:
+install-gmo: create-gmo
      $(INSTALL) -d $(DESTDIR)${localedir}
      for HLANG in $(LANGUAGES); do \
              echo '$(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo

$(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo'; \ $(INSTALL_DATA) -D $(OUTPUT)po/$$HLANG.gmo $(DESTDIR)${localedir}/$$HLANG/LC_MESSAGES/cpupower.mo; \ done;

tools/power/cpupower/Makefile install-man:

Quoted 4 lines

tools/power/cpupower/Makefile +1 -1

-install-bench:
+install-bench: compile-bench
      @#DESTDIR must be set from outside to survive
      @sbindir=$(sbindir) bindir=$(bindir) docdir=$(docdir) confdir=$

(confdir)

Quoted 16 lines

$(MAKE) -C bench O=$(OUTPUT) install
(quoted diff) +0 -0
b/tools/power/cpupower/bench/Makefile index f68b4bc55273..d9d9923af85c 100644
tools/power/cpupower/bench/Makefile +1 -1
@@ -27,7 +27,7 @@ $(OUTPUT)cpufreq-bench: $(OBJS)
 
 all: $(OUTPUT)cpufreq-bench
 
-install:
+install: $(OUTPUT)cpufreq-bench
      mkdir -p $(DESTDIR)/$(sbindir)
      mkdir -p $(DESTDIR)/$(bindir)
      mkdir -p $(DESTDIR)/$(docdir)

Thomas Renninger

maintainer

2021-01-07 21:00 UTC #3 lore raw

Am Donnerstag, 7. Januar 2021, 18:42:25 CET schrieb Ivan Babrou:

Quoted 27 lines · Thomas Renninger

Quoted 26 lines
Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:
Quoted 1 line
This allows building cpupower in parallel rather than serially.
cpupower is built serially:
[ make clean ]
time make
real    0m3,742s
user    0m3,330s
sys     0m1,105s
[ make clean ]
time make -j10
real    0m1,045s
user    0m3,153s
sys     0m1,037s
Only advantage I see is that you can call make install-xy
targets without calling the corresponding build target make xy
similar to the general install target:
install: all install-lib ...

Quoted 7 lines

Quoted 3 lines
Not sure anyone needs this and whether all targets successfully work this way.
If you'd show a useful usecase example...
We build a bunch of kernel related tools (perf, cpupower, bpftool, etc.) from our own top level Makefile, propagating parallelism downwards like one should.

I still do not understand why you do not simply build:
Also if I call this from /tools directory I get a quick build:
make -j20 cpupower

Can you please show the make calls, ideally with a timing to better understand and also to reproduce the advantages this patch introduces.
From what I can see, it only helps if one calls "sub-install" targets directly?
And I still do not understand why things should be more parallel now.

Quoted 1 line

Without this patch we have to remove parallelism for cpupower,

Why?

Quoted 3 lines

which doesn't seem like a very clean thing
to do, especially if you consider that it's 3x faster with parallelism enabled in wall clock terms.

Sure, you want to build in parallel. I still do not understand how this patch helps in this regard.

BTW, I recently had a bunch of userspace tools Makefile patches. I'd like to add you to CC for a review if they are not submitted already.

Thomas

Ivan Babrou

author

2021-01-07 21:16 UTC #4 lore raw

Quoted 36 lines · Thomas Renninger

Am Donnerstag, 7. Januar 2021, 18:42:25 CET schrieb Ivan Babrou:
Quoted 27 lines · Thomas Renninger
Quoted 26 lines
Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:
Quoted 1 line
This allows building cpupower in parallel rather than serially.
cpupower is built serially:
[ make clean ]
time make
real    0m3,742s
user    0m3,330s
sys     0m1,105s
[ make clean ]
time make -j10
real    0m1,045s
user    0m3,153s
sys     0m1,037s
Only advantage I see is that you can call make install-xy
targets without calling the corresponding build target make xy
similar to the general install target:
install: all install-lib ...
Quoted 7 lines
Quoted 3 lines
Not sure anyone needs this and whether all targets successfully work this way.
If you'd show a useful usecase example...
We build a bunch of kernel related tools (perf, cpupower, bpftool, etc.) from our own top level Makefile, propagating parallelism downwards like one should.

Quoted 8 lines

I still do not understand why you do not simply build:
Also if I call this from /tools directory I get a quick build:
make -j20 cpupower
Can you please show the make calls, ideally with a timing to better understand and also to reproduce the advantages this patch introduces.
From what I can see, it only helps if one calls "sub-install" targets directly?

That's exactly what we do: make install directly:

/linux-5.10.5$ make -C ./tools/power/cpupower DESTDIR=/tmp/cpupower install -j $(nproc)

make: Entering directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower'

  CC       cpupower
  MSGFMT   po/de.gmo
  MSGFMT   po/fr.gmo
  MSGFMT   po/it.gmo
  MSGFMT   po/cs.gmo

/usr/bin/install -c -d /tmp/cpupower/usr/lib64
MSGFMT po/pt.gmo
/usr/bin/install -c -d /tmp/cpupower/usr/bin
/usr/bin/install -c -m 644 -D man/cpupower.1 /tmp/cpupower/usr/man/man1/cpupower.1
/usr/bin/install -c -d /tmp/cpupower/usr/share/locale
cp -fpR ./libcpupower.so* /tmp/cpupower/usr/lib64/ /usr/bin/install -c ./cpupower /tmp/cpupower/usr/bin /usr/bin/install -c -m 644 -D man/cpupower-frequency-set.1 /tmp/cpupower/usr/man/man1/cpupower-frequency-set.1
make[1]: Entering directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower/bench'
for HLANG in de fr it cs pt; do \
echo '/usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo'; \
/usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo; \
done;
CC main.o
/usr/bin/install -c -d /tmp/cpupower/usr/share/bash-completion/completions /usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo
CC parse.o
/usr/bin/install -c -m 644 -D man/cpupower-frequency-info.1 /tmp/cpupower/usr/man/man1/cpupower-frequency-info.1
CC system.o
/usr/bin/install: cannot stat './po/de.gmo'/usr/bin/install -c -d /tmp/cpupower/usr/include
: No such file or directory
make[1]: Entering directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower/bench'
mkdir -p /tmp/cpupower//usr/sbin
/usr/bin/install -c -m 644 cpupower-completion.sh '/tmp/cpupower/usr/share/bash-completion/completions/cpupower' /usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo
CC benchmark.o
/usr/bin/install -c -m 644 -D man/cpupower-idle-set.1 /tmp/cpupower/usr/man/man1/cpupower-idle-set.1
/usr/bin/install: cannot stat './po/fr.gmo': No such file or directory /usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo mkdir -p /tmp/cpupower//usr/bin
/usr/bin/install -c -m 644 lib/cpufreq.h /tmp/cpupower/usr/include/cpufreq.h /usr/bin/install -c -m 644 -D man/cpupower-idle-info.1 /tmp/cpupower/usr/man/man1/cpupower-idle-info.1
mkdir -p /tmp/cpupower//usr/share/doc/packages/cpupower /usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo /usr/bin/install -c -m 644 lib/cpuidle.h /tmp/cpupower/usr/include/cpuidle.h mkdir -p /tmp/cpupower//etc/
/usr/bin/install -c -m 644 -D man/cpupower-set.1 /tmp/cpupower/usr/man/man1/cpupower-set.1
/usr/bin/install -c -m 644 -D ./po/$HLANG.gmo /tmp/cpupower/usr/share/locale/$HLANG/LC_MESSAGES/cpupower.mo install -m 755 .//cpufreq-bench /tmp/cpupower//usr/sbin/cpufreq-bench /usr/bin/install -c -m 644 -D man/cpupower-info.1 /tmp/cpupower/usr/man/man1/cpupower-info.1
/usr/bin/install -c -m 644 -D man/cpupower-monitor.1 /tmp/cpupower/usr/man/man1/cpupower-monitor.1
install: cannot stat './/cpufreq-bench': No such file or directory Makefile:31: recipe for target 'install' failed
make[1]: *** [install] Error 1
make[1]: Leaving directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower/bench' Makefile:304: recipe for target 'install-bench' failed
make: *** [install-bench] Error 2
make: *** Waiting for unfinished jobs....
CC cpufreq-bench
make[1]: Leaving directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower/bench'
make: Leaving directory '/state/home/ivan/linux-5.10.5/tools/power/cpupower'

It works with -j 1. My patch makes it work with arbitrary parallelism, same as any other tool we build internally (perf, bpftool, turbostat, etc.).

Quoted 5 lines

And I still do not understand why things should be more parallel now.
Quoted 1 line
Without this patch we have to remove parallelism for cpupower,
Why?

Quoted 8 lines

Quoted 3 lines
which doesn't seem like a very clean thing
to do, especially if you consider that it's 3x faster with parallelism enabled in wall clock terms.
Sure, you want to build in parallel. I still do not understand how this patch helps in this regard.
BTW, I recently had a bunch of userspace tools Makefile patches. I'd like to add you to CC for a review if they are not submitted already.

Don't know how useful I can be, but feel free to.

Quoted 3 lines

Thomas

Thomas Renninger

maintainer

2021-01-07 21:30 UTC #5 lore raw

Am Donnerstag, 7. Januar 2021, 22:15:16 CET schrieb Ivan Babrou:

Quoted 5 lines · Thomas Renninger

Quoted 4 lines
Am Donnerstag, 7. Januar 2021, 18:42:25 CET schrieb Ivan Babrou:
Quoted 3 lines · Thomas Renninger
Quoted 2 lines
Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:
Quoted 1 line
This allows building cpupower in parallel rather than serially.

...

Quoted 9 lines

Quoted 4 lines
Can you please show the make calls, ideally with a timing to better understand and also to reproduce the advantages this patch introduces. From what I can see, it only helps if one calls "sub-install" targets directly?
That's exactly what we do: make install directly:
/linux-5.10.5$ make -C ./tools/power/cpupower DESTDIR=/tmp/cpupower install -j $(nproc)

This makes sense then.
@Shuah @Rafael: Can this patch be queued up, please.

Thanks for the patch and the details,

Thomas

Shuah Khan

maintainer

2021-01-22 18:06 UTC #6 lore raw

Quoted 14 lines · Thomas Renninger

Am Donnerstag, 7. Januar 2021, 22:15:16 CET schrieb Ivan Babrou:
Quoted 5 lines · Thomas Renninger
Quoted 4 lines
Am Donnerstag, 7. Januar 2021, 18:42:25 CET schrieb Ivan Babrou:
Quoted 3 lines · Thomas Renninger
Quoted 2 lines
Am Dienstag, 5. Januar 2021, 00:57:18 CET schrieb Ivan Babrou:
Quoted 1 line
This allows building cpupower in parallel rather than serially.
...
Quoted 7 lines
Quoted 4 lines
Can you please show the make calls, ideally with a timing to better understand and also to reproduce the advantages this patch introduces.
From what I can see, it only helps if one calls "sub-install" targets
directly?
That's exactly what we do: make install directly:

Quoted 8 lines

Quoted 2 lines
/linux-5.10.5$ make -C ./tools/power/cpupower DESTDIR=/tmp/cpupower install -j $(nproc)
This makes sense then.
@Shuah @Rafael: Can this patch be queued up, please.
Thanks for the patch and the details,

Thank you both.

I applied it https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux.git/commit/?h=cpupower

Will send this in my next pull request to Rafael.

thanks,
-- Shuah