2015-04-23 11:29:22

by Will Deacon

[permalink] [raw]
Subject: 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


2015-04-23 14:04:59

by Will Deacon

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Thu, Apr 23, 2015 at 12:29:16PM +0100, Will Deacon wrote:
> 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?

Scratch that, how about using sysconf instead? Patch below...

Will

--->8

>From 28740111e81aa9247bf48e3125dc43cc31d94e6f Mon 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.

Signed-off-by: Will Deacon <[email protected]>
---
tools/perf/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc35eef9..c26cb04ce6bd 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
--
2.1.4

2015-04-23 14:16:29

by David Ahern

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On 4/23/15 8:04 AM, Will Deacon wrote:
> From 28740111e81aa9247bf48e3125dc43cc31d94e6f Mon 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.
>
> Signed-off-by: Will Deacon <[email protected]>
> ---
> tools/perf/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index c699dc35eef9..c26cb04ce6bd 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -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.

2015-04-23 14:29:35

by David Ahern

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On 4/23/15 5:29 AM, Will Deacon wrote:
> 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?


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

2015-04-24 16:11:18

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Thu, Apr 23, 2015 at 08:16:15AM -0600, David Ahern escreveu:
> On 4/23/15 8:04 AM, Will Deacon wrote:
> >+++ b/tools/perf/Makefile
> >@@ -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.

Hum, perhaps check if getconf is present, if not fallback to the egrep
-c? I'll try.

- Arnaldo

2015-04-24 16:22:43

by Will Deacon

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Fri, Apr 24, 2015 at 05:10:58PM +0100, Arnaldo Carvalho de Melo wrote:
> Em Thu, Apr 23, 2015 at 08:16:15AM -0600, David Ahern escreveu:
> > On 4/23/15 8:04 AM, Will Deacon wrote:
> > >+++ b/tools/perf/Makefile
> > >@@ -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.
>
> 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

2015-04-24 17:22:06

by David Ahern

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On 4/24/15 10:22 AM, Will Deacon wrote:
> 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

2015-04-27 17:08:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
> On 4/24/15 10:22 AM, Will Deacon wrote:
> >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
> ...

Ok, any progress here?

- Arnaldo

2015-04-27 17:13:12

by David Ahern

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On 4/27/15 10:26 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
>> On 4/24/15 10:22 AM, Will Deacon wrote:
>>> 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
>> ...
>
> 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

2015-04-27 17:40:24

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
> On 4/27/15 10:26 AM, Arnaldo Carvalho de Melo wrote:
> >Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
> >>On 4/24/15 10:22 AM, Will Deacon wrote:
> >>>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
> >>...
> >
> >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?

> Barring that use of getconf with a fallback to grepping /proc/cpuinfo.

Ok, but no patch so far doing that, right? 8-)

- Arnaldo

2015-04-27 17:49:19

by Jiri Olsa

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Mon, Apr 27, 2015 at 02:40:17PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
> > On 4/27/15 10:26 AM, Arnaldo Carvalho de Melo wrote:
> > >Em Fri, Apr 24, 2015 at 11:21:54AM -0600, David Ahern escreveu:
> > >>On 4/24/15 10:22 AM, Will Deacon wrote:
> > >>>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
> > >>...
> > >
> > >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?
>
> > 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

2015-04-27 18:40:07

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
> On Mon, Apr 27, 2015 at 02:40:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
> > >
> > > I think the right thing to do is to remove JOBS completely.
> >
> > That is up for discussion, Ingo, Jiri, Namhyung, others?
> >
> > > 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 ;-)

[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

2015-04-27 18:41:28

by Jiri Olsa

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Mon, Apr 27, 2015 at 03:39:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
> > On Mon, Apr 27, 2015 at 02:40:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
> > > >
> > > > I think the right thing to do is to remove JOBS completely.
> > >
> > > That is up for discussion, Ingo, Jiri, Namhyung, others?
> > >
> > > > 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 ;-)
>
> [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

---
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc35eef9..ed79b10de1d6 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
diff --git a/tools/perf/ncpus.sh b/tools/perf/ncpus.sh
new file mode 100755
index 000000000000..4466b657336b
--- /dev/null
+++ b/tools/perf/ncpus.sh
@@ -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

2015-04-27 18:45:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, Apr 27, 2015 at 08:41:20PM +0200, Jiri Olsa escreveu:
> On Mon, Apr 27, 2015 at 03:39:53PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Apr 27, 2015 at 07:49:06PM +0200, Jiri Olsa escreveu:
> > > On Mon, Apr 27, 2015 at 02:40:17PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Mon, Apr 27, 2015 at 11:13:02AM -0600, David Ahern escreveu:
> > > > >
> > > > > I think the right thing to do is to remove JOBS completely.
> > > >
> > > > That is up for discussion, Ingo, Jiri, Namhyung, others?
> > > >
> > > > > 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 ;-)
> >
> > [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:

>From e8155c06652a05f2307d53823a7937be5dad4e32 Mon 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.

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]>
Link: http://lkml.kernel.org/r/[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(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc35eef9..bc846b83c295 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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
--
1.8.3.1

2015-04-27 18:52:17

by Jiri Olsa

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Mon, Apr 27, 2015 at 03:45:35PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > > [acme@ssdandy linux]$
> > >
> >
> > how about this one
>
> I came up with this one, that doesn't introduces a new file:

ook

>
> From e8155c06652a05f2307d53823a7937be5dad4e32 Mon 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.
>
> 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]>
> Link: http://lkml.kernel.org/r/[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(-)
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index c699dc35eef9..bc846b83c295 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -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

2015-04-27 19:00:13

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, Apr 27, 2015 at 08:52:08PM +0200, Jiri Olsa escreveu:
> On Mon, Apr 27, 2015 at 03:45:35PM -0300, Arnaldo Carvalho de Melo wrote:
>
> SNIP
>
> > > > [acme@ssdandy linux]$
> > > >
> > >
> > > how about this one
> >
> > I came up with this one, that doesn't introduces a new file:
>
> ook
>
> >
> > From e8155c06652a05f2307d53823a7937be5dad4e32 Mon 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.
> >
> > 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]>
> > Link: http://lkml.kernel.org/r/[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(-)
> >
> > diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> > index c699dc35eef9..bc846b83c295 100644
> > --- a/tools/perf/Makefile
> > +++ b/tools/perf/Makefile
> > @@ -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

2015-04-27 19:04:11

by Jiri Olsa

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Mon, Apr 27, 2015 at 04:00:04PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> > > index c699dc35eef9..bc846b83c295 100644
> > > --- a/tools/perf/Makefile
> > > +++ b/tools/perf/Makefile
> > > @@ -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?

yep

jirka

Subject: [tip:perf/core] perf 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.

Signed-off-by: Will Deacon <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[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(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc3..d31a7bb 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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

2015-05-11 08:21:27

by Ingo Molnar

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline


* David Ahern <[email protected]> wrote:

> On 4/23/15 5:29 AM, Will Deacon wrote:
> >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?

That regex needs to be fixed or replaced with a more robust 'number of
CPUs on the system' discovery method.

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

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

2015-05-11 12:31:23

by Will Deacon

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

On Mon, May 11, 2015 at 09:21:20AM +0100, Ingo Molnar wrote:
> * David Ahern <[email protected]> wrote:
> > On 4/23/15 5:29 AM, Will Deacon wrote:
> > >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?
>
> 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

2015-05-11 12:33:12

by Ingo Molnar

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline


* Will Deacon <[email protected]> wrote:

> On Mon, May 11, 2015 at 09:21:20AM +0100, Ingo Molnar wrote:
> > * David Ahern <[email protected]> wrote:
> > > On 4/23/15 5:29 AM, Will Deacon wrote:
> > > >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?
> >
> > 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.

Acked-by: Ingo Molnar <[email protected]>

Thanks,

Ingo

2015-05-11 15:58:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
>
> * Will Deacon <[email protected]> wrote:
>
> > On Mon, May 11, 2015 at 09:21:20AM +0100, Ingo Molnar wrote:
> > > * David Ahern <[email protected]> wrote:
> > > > On 4/23/15 5:29 AM, Will Deacon wrote:
> > > > >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?
> > >
> > > 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.
>
> Acked-by: Ingo Molnar <[email protected]>

IIRC it was merged already, lemme check...

- Arnaldo

2015-05-11 16:00:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, May 11, 2015 at 12:58:14PM -0300, [email protected] escreveu:
> Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
> > > 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]>
>
> 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.

Signed-off-by: Will Deacon <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Made it fall back to parsing /proc when getconf not found ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc35eef9..d31a7bbd7cee 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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]$

2015-05-11 17:29:36

by Will Deacon

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

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:
> > Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
> > > > 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]>
> >
> > 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

>
> [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.
>
> Signed-off-by: Will Deacon <[email protected]>
> Acked-by: Jiri Olsa <[email protected]>
> Tested-by: Arnaldo Carvalho de Melo <[email protected]>
> Cc: David Ahern <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> [ Made it fall back to parsing /proc when getconf not found ]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index c699dc35eef9..d31a7bbd7cee 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -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]$
>

2015-05-11 19:01:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: arm/arm64 perf build issue with mainline

Em Mon, May 11, 2015 at 06:29:28PM +0100, Will Deacon escreveu:
> 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:
> > > Em Mon, May 11, 2015 at 02:33:04PM +0200, Ingo Molnar escreveu:
> > > > > 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]>
> > >
> > > 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

> Cheers,
>
> Will
>
> >
> > [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.
> >
> > Signed-off-by: Will Deacon <[email protected]>
> > Acked-by: Jiri Olsa <[email protected]>
> > Tested-by: Arnaldo Carvalho de Melo <[email protected]>
> > Cc: David Ahern <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Namhyung Kim <[email protected]>
> > Link: http://lkml.kernel.org/r/[email protected]
> > [ Made it fall back to parsing /proc when getconf not found ]
> > Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> >
> > diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> > index c699dc35eef9..d31a7bbd7cee 100644
> > --- a/tools/perf/Makefile
> > +++ b/tools/perf/Makefile
> > @@ -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]$
> >

Subject: [tip:perf/urgent] perf 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.

Signed-off-by: Will Deacon <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[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(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c699dc3..d31a7bb 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -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