arm/arm64 perf build issue with mainline
Hi all,
Commit 6428c59a97de ("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:
CPU implementer : 0x41
CPU architecture: 8
CPU variant : 0x0
CPU part : 0xd03
CPU revision : 0
in /proc/cpuinfo. Consequently, a 6 core machine ends up doing:
will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel build
which is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
Will
Quoted 12 lines · Will Deacon
Hi all,
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0
Quoted 7 lines
in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
Scratch that, how about using sysconf instead? Patch below...
Will
--->8
Quoted 1 line
From
28740111e81aa9247bf48e3125dc43cc31d94e6fMon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Thu, 23 Apr 2015 15:00:16 +0100
Subject: [PATCH] tools: perf: use getconf to determine number of online CPUs
Parsing /proc/cpuinfo is a fiddly, arch-dependent business anda recent change to get it working for Sparc broke arm and arm64 platforms.
Instead, use sysconf to determine the number of online CPUs and avoid parsing /proc/cpuinfo entirely.
--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell getconf _NPROCESSORS_ONLN 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
Signature · 1 line
2.1.4
Quoted 22 lines · Will Deacon
From
28740111e81aa9247bf48e3125dc43cc31d94e6fMon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Thu, 23 Apr 2015 15:00:16 +0100
Subject: [PATCH] tools: perf: use getconf to determine number of online CPUsParsing
/proc/cpuinfois a fiddly, arch-dependent business anda recent change to get it working for Sparc broke arm and arm64 platforms.Instead, use sysconf to determine the number of online CPUs and avoid parsing
/proc/cpuinfoentirely.Signed-off-by Will Deacon <[email protected]>--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/Makefile +0
-0
tools/perf/Makefile unexport MAKEFLAGS · line 30
Quoted 8 lines
tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell getconf _NPROCESSORS_ONLN 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
Certainly a more robust way of doing it but I am concerned this might end up breaking others. For Fedora at least getconf is in the glibc-common package and there are users that do not build with glibc. It is not clear if the build system for those environments will have getconf.
Em Thu, Apr 23, 2015 at 08:16:15AM -0600, David Ahern escreveu:
Quoted 9 lines · Will Deacon
Quoted 8 lines
+++ b/tools/perf/Makefile
tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell getconf _NPROCESSORS_ONLN 2>/dev/null) ifeq ($(JOBS),0)
Quoted 8 lines
tools/perf/Makefile unexport MAKEFLAGS · line 30
Quoted 2 lines
tools/perf/Makefile +0
-0JOBS := 1 endifCertainly a more robust way of doing it but I am concerned this might end up breaking others. For Fedora at least getconf is in the glibc-common package and there are users that do not build with glibc. It is not clear if the build system for those environments will have getconf.
Hum, perhaps check if getconf is present, if not fallback to the egrep -c? I'll try.
- Arnaldo
Quoted 19 lines · Arnaldo Carvalho de Melo
Em Thu, Apr 23, 2015 at 08:16:15AM -0600, David Ahern escreveu:
Quoted 9 lines · Will Deacon
Quoted 8 lines
+++ b/tools/perf/Makefile
tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell getconf _NPROCESSORS_ONLN 2>/dev/null) ifeq ($(JOBS),0)Quoted 8 lines
tools/perf/Makefile unexport MAKEFLAGS · line 30
Quoted 2 lines
tools/perf/Makefile +0
-0JOBS := 1 endifCertainly a more robust way of doing it but I am concerned this might end up breaking others. For Fedora at least getconf is in the glibc-common package and there are users that do not build with glibc. It is not clear if the build system for those environments will have getconf.
Quoted 2 lines
Hum, perhaps check if getconf is present, if not fallback to the egrep -c? I'll try.
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
Will
Quoted 2 lines · Will Deacon
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
I have access to 1 or 2 or ...
Yes, it needs start with CPU and be all caps (there are other Cpu lines) and a cpu number follows:
CPU0: online CPU1: online CPU2: online CPU3: online
...
David
Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
Quoted 6 lines · Will Deacon
Quoted 2 lines
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
I have access to 1 or 2 or ...
Quoted 8 lines
Yes, it needs start with CPU and be all caps (there are other Cpu lines) and a cpu number follows:
CPU0: online CPU1: online CPU2: online CPU3: online...
Ok, any progress here?
- Arnaldo
Quoted 10 lines · Arnaldo Carvalho de Melo
Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
Quoted 9 lines · Will Deacon
Quoted 2 lines
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
I have access to 1 or 2 or ...
Yes, it needs start with CPU and be all caps (there are other Cpu lines) and a cpu number follows:
Quoted 7 lines
Quoted 5 lines
CPU0: online CPU1: online CPU2: online CPU3: online...
Ok, any progress here?
I think the right thing to do is to remove JOBS completely.
Barring that use of getconf with a fallback to grepping /proc/cpuinfo.
David
Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
Quoted 17 lines · Arnaldo Carvalho de Melo
Quoted 16 lines
Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
Quoted 14 lines · Will Deacon
Quoted 2 lines
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
I have access to 1 or 2 or ...
Yes, it needs start with CPU and be all caps (there are other Cpu lines) and a cpu number follows:
CPU0: online CPU1: online CPU2: online CPU3: online...
Quoted 3 lines
Quoted 1 line
Ok, any progress here?
I think the right thing to do is to remove JOBS completely.
That is up for discussion, Ingo, Jiri, Namhyung, others?
Quoted 1 line
Barring that use of getconf with a fallback to grepping
/proc/cpuinfo.
Ok, but no patch so far doing that, right? 8-)
- Arnaldo
Quoted 20 lines · Arnaldo Carvalho de Melo
Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
Quoted 19 lines · Arnaldo Carvalho de Melo
Quoted 17 lines
Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
Quoted 6 lines · Will Deacon
Quoted 2 lines
I don't have a SPARC machine to hand, but we could check that the character immediately following "CPU" is a number [0-9].
I have access to 1 or 2 or ...
Quoted 8 lines
Yes, it needs start with CPU and be all caps (there are other Cpu lines) and a cpu number follows:
CPU0: online CPU1: online CPU2: online CPU3: online...
Ok, any progress here?
Quoted 7 lines
Quoted 1 line
I think the right thing to do is to remove JOBS completely.
That is up for discussion, Ingo, Jiri, Namhyung, others?
Quoted 1 line
Barring that use of getconf with a fallback to grepping
/proc/cpuinfo.Ok, but no patch so far doing that, right? 8-)
heh, I've got the idea that you're preparing something ;-)
as for me I've got used to the automatic -jX being added, so I'd prefer we fix that JOBS setup for arm
I'll try to send something ;-)
jirka
Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 8 lines
Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
Quoted 2 lines
I think the right thing to do is to remove JOBS completely.
That is up for discussion, Ingo, Jiri, Namhyung, others?
Quoted 1 line
Barring that use of getconf with a fallback to grepping
/proc/cpuinfo.
Quoted 8 lines
Quoted 1 line
Ok, but no patch so far doing that, right? 8-)
heh, I've got the idea that you're preparing something ;-)
as for me I've got used to the automatic -jX being added, so I'd prefer we fix that JOBS setup for arm
I'll try to send something ;-)
[acme@ssdandy linux]$ (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$ (not-found-getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$
Testing it in the makefile...
- Arnaldo
Quoted 17 lines · Arnaldo Carvalho de Melo
Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
Quoted 16 lines · Arnaldo Carvalho de Melo
Quoted 9 lines
Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
Quoted 2 lines
I think the right thing to do is to remove JOBS completely.
That is up for discussion, Ingo, Jiri, Namhyung, others?
Quoted 1 line
Barring that use of getconf with a fallback to grepping
/proc/cpuinfo.Ok, but no patch so far doing that, right? 8-)
heh, I've got the idea that you're preparing something ;-)
as for me I've got used to the automatic -jX being added, so I'd prefer we fix that JOBS setup for arm
Quoted 8 lines
Quoted 1 line
I'll try to send something ;-)
[acme@ssdandy linux]$ (getconf
_NPROCESSORS_ONLN|| egrep -c '^processor|^CPU'/proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$ (not-found-getconf_NPROCESSORS_ONLN|| egrep -c '^processor|^CPU'/proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$
how about this one
jirka
---
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell ./ncpus.sh) ifeq ($(JOBS),0) JOBS := 1 endif
tools/perf/ncpus.sh new file +11 -0
@@ -0,0 +1,11 @@ +#!/bin/sh + +set -x + +GETCONF=`which getconf 2>/dev/null` +if [ $? -eq 0 ]; then + $GETCONF _NPROCESSORS_ONLN + exit 0 +fi + +egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo 2>/dev/null
Em Mon, Apr 27, 2015 at 08:41:20PM +0200, Jiri Olsa escreveu:
Quoted 20 lines · Arnaldo Carvalho de Melo
Quoted 19 lines
Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
Quoted 17 lines · Arnaldo Carvalho de Melo
Quoted 9 lines
Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
Quoted 2 lines
I think the right thing to do is to remove JOBS completely.
That is up for discussion, Ingo, Jiri, Namhyung, others?
Quoted 1 line
Barring that use of getconf with a fallback to grepping
/proc/cpuinfo.Ok, but no patch so far doing that, right? 8-)
heh, I've got the idea that you're preparing something ;-)
as for me I've got used to the automatic -jX being added, so I'd prefer we fix that JOBS setup for arm
I'll try to send something ;-)
Quoted 8 lines
Quoted 6 lines
[acme@ssdandy linux]$ (getconf
_NPROCESSORS_ONLN|| egrep -c '^processor|^CPU'/proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$ (not-found-getconf_NPROCESSORS_ONLN|| egrep -c '^processor|^CPU'/proc/cpuinfo) 2>/dev/null 8
[acme@ssdandy linux]$how about this one
I came up with this one, that doesn't introduces a new file:
Quoted 1 line
From
e8155c06652a05f2307d53823a7937be5dad4e32Mon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Thu, 23 Apr 2015 15:00:16 +0100
Subject: [PATCH] tools perf: Use getconf to determine number of online CPUs
Parsing /proc/cpuinfo is a fiddly, arch-dependent business anda recent change to get it working for Sparc broke arm and arm64 platforms.
Use sysconf to determine the number of online CPUs only parsing /proc/cpuinfo when sysconf is not available.
--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
Signature · 1 line
1.8.3.1
On Mon, Apr 27, 2015 at 03:45:35PM -0300, Arnaldo Carvalho de Melo wrote:
SNIP
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
[acme@ssdandy linux]$
how about this one
I came up with this one, that doesn't introduces a new file:
ook
Quoted 29 lines
From
e8155c06652a05f2307d53823a7937be5dad4e32Mon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Thu, 23 Apr 2015 15:00:16 +0100
Subject: [PATCH] tools perf: Use getconf to determine number of online CPUsParsing
/proc/cpuinfois a fiddly, arch-dependent business anda recent change to get it working for Sparc broke arm and arm64 platforms.Use sysconf to determine the number of online CPUs only parsing
/proc/cpuinfowhen sysconf is not available.Signed-off-by Will Deacon <[email protected]>Cc David Ahern <[email protected]>Cc Jiri Olsa <[email protected]>Cc Mark Rutland <[email protected]>Cc Namhyung Kim <[email protected]>[ Made it fall back to parsing /proc when getconf not found ]Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/Makefile +0
-0
tools/perf/Makefile unexport MAKEFLAGS · line 27
Quoted 5 lines
tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null)
how about the 'CPU[0-9]' someone asked for?
jirka
Em Mon, Apr 27, 2015 at 08:52:08PM +0200, Jiri Olsa escreveu:
Quoted 37 lines · Arnaldo Carvalho de Melo
SNIP
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
[acme@ssdandy linux]$
how about this one
I came up with this one, that doesn't introduces a new file:
ook
Quoted 24 lines
From
e8155c06652a05f2307d53823a7937be5dad4e32Mon Sep 17 00:00:00 2001
From: Will Deacon <[email protected]>
Date: Thu, 23 Apr 2015 15:00:16 +0100
Subject: [PATCH] tools perf: Use getconf to determine number of online CPUsParsing
/proc/cpuinfois a fiddly, arch-dependent business anda recent change to get it working for Sparc broke arm and arm64 platforms.Use sysconf to determine the number of online CPUs only parsing
/proc/cpuinfowhen sysconf is not available.Signed-off-by Will Deacon <[email protected]>Cc David Ahern <[email protected]>Cc Jiri Olsa <[email protected]>Cc Mark Rutland <[email protected]>Cc Namhyung Kim <[email protected]>[ Made it fall back to parsing /proc when getconf not found ]Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines
tools/perf/Makefile unexport MAKEFLAGS · line 27
Quoted 10 lines
tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null)how about the 'CPU[0-9]' someone asked for?
Ok, will fold that in, are you ok with going with this patch plus this [0-9] change? I.e. acked?
- Arnaldo
On Mon, Apr 27, 2015 at 04:00:04PM -0300, Arnaldo Carvalho de Melo wrote:
SNIP
Quoted 15 lines
Quoted 14 lines
tools/perf/Makefile unexport MAKEFLAGS · line 27
Quoted 12 lines
1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU' /proc/cpuinfo) 2>/dev/null)how about the 'CPU[0-9]' someone asked for?
Quoted 2 lines
Ok, will fold that in, are you ok with going with this patch plus this [0-9] change? I.e. acked?
yep
jirka
tip-bot for Will Deacon
merge bot[tip:perf/core] perf tools: Use getconf to determine number of online CPUs
In reply to Will Deaconperf tools: Use getconf to determine number of online CPUs
Commit-ID: 762abdc0c6c013425958cd9f5105f4e32268d434 Gitweb: http://git.kernel.org/tip/762abdc0c6c013425958cd9f5105f4e32268d434 Author: Will Deacon <[email protected]> AuthorDate: Thu, 23 Apr 2015 15:00:16 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Mon, 4 May 2015 12:43:40 -0300
perf tools: Use getconf to determine number of online CPUs
Parsing /proc/cpuinfo is a fiddly, arch-dependent business and a recent change to get it working for Sparc broke arm and arm64 platforms.
Use sysconf to determine the number of online CPUs only parsing /proc/cpuinfo when sysconf is not available.
--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
tip-bot for Will Deacon
merge bot[tip:perf/urgent] perf tools: Use getconf to determine number of online CPUs
In reply to Will Deaconperf tools: Use getconf to determine number of online CPUs
Commit-ID: 466c1eb07f42bd27825af24d86b46d05e5e350b9 Gitweb: http://git.kernel.org/tip/466c1eb07f42bd27825af24d86b46d05e5e350b9 Author: Will Deacon <[email protected]> AuthorDate: Thu, 23 Apr 2015 15:00:16 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 12 May 2015 18:11:16 -0300
perf tools: Use getconf to determine number of online CPUs
Parsing /proc/cpuinfo is a fiddly, arch-dependent business and a recent change to get it working for Sparc broke arm and arm64 platforms.
Use sysconf to determine the number of online CPUs only parsing /proc/cpuinfo when sysconf is not available.
--- tools/perf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
Quoted 12 lines · Will Deacon
Hi all,
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0
Quoted 7 lines
in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
Frankly, I think the JOBS parameter needs to be removed. It's non-standard way of controlling parallelism in the build and it makes the assumption that if a system has N processors all of those can be used to build perf which is not true if you are building perf as part of bigger image builds -- like Yocto for example.
Ingo: As I recall you put this in? Opinions on removing it? Users can always add the standard '-j N' for parallelism just like they do for kernel builds.
David
Quoted 13 lines · David Ahern
Quoted 12 lines · Will Deacon
Hi all,
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0
Quoted 7 lines
Quoted 7 lines
in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
That regex needs to be fixed or replaced with a more robust 'number of CPUs on the system' discovery method.
Quoted 7 lines
Frankly, I think the JOBS parameter needs to be removed. It's non-standard way of controlling parallelism in the build and it makes the assumption that if a system has N processors all of those can be used to build perf which is not true if you are building perf as part of bigger image builds -- like Yocto for example.
Ingo: As I recall you put this in? Opinions on removing it?
I disagree strongly!
Quoted 2 lines
[...] Users can always add the standard '-j N' for parallelism just like they do for kernel builds.
So for a few oddball cases we remove something that improves usability quite visibly? 'make' or 'make install' will do the right thing today on 99.9% of the systems, and it should do that in the future as well.
We should not add extra usability barriers that hurts the regular case.
This usability concept permeates all of perf: that's why 'perf top' will measure all CPUs by default, although there are certainly cases where that's not what the user wants.
Thanks,
Ingo
Quoted 14 lines · Ingo Molnar
Quoted 13 lines · David Ahern
Quoted 12 lines · Will Deacon
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
That regex needs to be fixed or replaced with a more robust 'number of CPUs on the system' discovery method.
That was already proposed here (as part of the fallback from getconf):
https://lkml.kernel.org/r/[email protected]
but I'm not sure what happened to the patch.
Will
Ingo Molnar
Acked-byQuoted 21 lines · Will Deacon
Quoted 20 lines · Ingo Molnar
Quoted 18 lines · David Ahern
Quoted 17 lines · Will Deacon
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
Quoted 8 lines
Quoted 2 lines
That regex needs to be fixed or replaced with a more robust 'number of CPUs on the system' discovery method.
That was already proposed here (as part of the fallback from getconf):
https://lkml.kernel.org/r/[email protected]
but I'm not sure what happened to the patch.
Sending out the latest/best version as a reminder for Arnaldo will sure help it along.
Thanks,
Ingo
Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
Quoted 29 lines · Will Deacon
Quoted 27 lines · Ingo Molnar
Quoted 14 lines · David Ahern
Quoted 13 lines · Will Deacon
Quoted 12 lines
Commit
6428c59a97de("perf tools: Set JOBS based on CPU or processor") causes weird behaviour on arm/arm64 platforms because we use the "CPU" prefix for things like:CPU implementer :
0x41
CPU architecture: 8
CPU variant :0x0
CPU part :0xd03
CPU revision : 0in
/proc/cpuinfo. Consequently, a 6 core machine ends up doing:Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
will@confinement-loaf:~/linux/tools/perf$ make BUILD: Doing 'make -j36' parallel buildwhich is a little overwhelming. Any chance we can predicate the extra part of the regex on $(ARCH) being sparc?
That regex needs to be fixed or replaced with a more robust 'number of CPUs on the system' discovery method.
That was already proposed here (as part of the fallback from getconf):
Quoted 8 lines
Quoted 3 lines
https://lkml.kernel.org/r/[email protected]
but I'm not sure what happened to the patch.
Sending out the latest/best version as a reminder for Arnaldo will sure help it along.
Acked-by Ingo Molnar <[email protected]>
IIRC it was merged already, lemme check...
- Arnaldo
Em Mon, May 11, 2015 at 12:58:14PM -0300, [email protected] escreveu:
Quoted 5 lines
Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
Quoted 4 lines
Quoted 4 lines
That was already proposed here (as part of the fallback from getconf):
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
but I'm not sure what happened to the patch.
Sending out the latest/best version as a reminder for Arnaldo will sure help it along.
Acked-by Ingo Molnar <[email protected]>IIRC it was merged already, lemme check...
Yes, only for perf/core:
[acme@zoo linux]$ git show 762abdc0c6c013425958cd9f5105f4e32268d434 commit 762abdc0c6c013425958cd9f5105f4e32268d434
Author: Will Deacon <[email protected]> Date: Thu Apr 23 15:00:16 2015 +0100
perf tools: Use getconf to determine number of online CPUs
Parsing /proc/cpuinfo is a fiddly, arch-dependent business and a recent change to get it working for Sparc broke arm and arm64 platforms.
Use sysconf to determine the number of online CPUs only parsing /proc/cpuinfo when sysconf is not available.
tools/perf/Makefile +1 -1
@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif
[acme@zoo linux]$
Quoted 8 lines · On Mon, May 11, 2015 at 04:59:58PM +0100, [email protected] wrote:
Em Mon, May 11, 2015 at 12:58:14PM -0300, [email protected] escreveu:
Quoted 7 lines
Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
Quoted 6 lines
Quoted 5 lines
That was already proposed here (as part of the fallback from getconf):
https://lkml.kernel.org/r/[email protected]
but I'm not sure what happened to the patch.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Sending out the latest/best version as a reminder for Arnaldo will sure help it along.
Acked-by Ingo Molnar <[email protected]>IIRC it was merged already, lemme check...
Yes, only for perf/core:
Ah, that explains why I couldn't see it in my tree. Could we get it in for 4.1 please, as this is a regression for arm/arm64 (where the auto-detection used to work fine)?
Cheers,
Will
Quoted 39 lines
[acme@zoo linux]$ git show
762abdc0c6c013425958cd9f5105f4e32268d434commit762abdc0c6c013425958cd9f5105f4e32268d434Author: Will Deacon <[email protected]> Date: Thu Apr 23 15:00:16 2015 +0100perf tools: Use getconf to determine number of online CPUs
Parsing
/proc/cpuinfois a fiddly, arch-dependent business and a recent change to get it working for Sparc broke arm and arm64 platforms.Use sysconf to determine the number of online CPUs only parsing
/proc/cpuinfowhen sysconf is not available.Signed-off-by Will Deacon <[email protected]>Acked-by Jiri Olsa <[email protected]>Cc David Ahern <[email protected]>Cc Mark Rutland <[email protected]>Cc Namhyung Kim <[email protected]>[ Made it fall back to parsing /proc when getconf not found ]Signed-off-by Arnaldo Carvalho de Melo <[email protected]>tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif[acme@zoo linux]$
Em Mon, May 11, 2015 at 06:29:28PM +0100, Will Deacon escreveu:
Quoted 14 lines · On Mon, May 11, 2015 at 04:59:58PM +0100, [email protected] wrote:
Quoted 13 lines
Em Mon, May 11, 2015 at 12:58:14PM -0300, [email protected] escreveu:
Quoted 12 lines
Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
Quoted 10 lines
Quoted 5 lines
That was already proposed here (as part of the fallback from getconf):
https://lkml.kernel.org/r/[email protected]
but I'm not sure what happened to the patch.
Sending out the latest/best version as a reminder for Arnaldo will sure help it along.
Acked-by Ingo Molnar <[email protected]>
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
IIRC it was merged already, lemme check...
Yes, only for perf/core:
Ah, that explains why I couldn't see it in my tree. Could we get it in for 4.1 please, as this is a regression for arm/arm64 (where the auto-detection used to work fine)?
Ok, I'll cherry-pick it into a perf/urgent branch, will see if there are more cherries and then ask Ingo to pull it.
- Arnaldo
Quoted 43 lines
Cheers,
Will
Quoted 39 lines
[acme@zoo linux]$ git show
762abdc0c6c013425958cd9f5105f4e32268d434commit762abdc0c6c013425958cd9f5105f4e32268d434Author: Will Deacon <[email protected]> Date: Thu Apr 23 15:00:16 2015 +0100perf tools: Use getconf to determine number of online CPUs
Parsing
/proc/cpuinfois a fiddly, arch-dependent business and a recent change to get it working for Sparc broke arm and arm64 platforms.Use sysconf to determine the number of online CPUs only parsing
/proc/cpuinfowhen sysconf is not available.Signed-off-by Will Deacon <[email protected]>Acked-by Jiri Olsa <[email protected]>Cc David Ahern <[email protected]>Cc Mark Rutland <[email protected]>Cc Namhyung Kim <[email protected]>[ Made it fall back to parsing /proc when getconf not found ]Signed-off-by Arnaldo Carvalho de Melo <[email protected]>tools/perf/Makefile +1
-1@@ -24,7 +24,7 @@ unexport MAKEFLAGS # (To override it, run 'make JOBS=1' and similar.) # ifeq ($(JOBS),) - JOBS := $(shell egrep -c '^processor|^CPU' /proc/cpuinfo 2>/dev/null) + JOBS := $(shell (getconf _NPROCESSORS_ONLN || egrep -c '^processor|^CPU[0-9]' /proc/cpuinfo) 2>/dev/null) ifeq ($(JOBS),0) JOBS := 1 endif[acme@zoo linux]$