linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] perf tool: Fix Android build
@ 2016-07-01  5:12 Chris Phlipot
  2016-07-01  5:12 ` [PATCH 1/4] tools lib api: Respect WERROR=0 for build Chris Phlipot
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Chris Phlipot @ 2016-07-01  5:12 UTC (permalink / raw)
  To: acme, peterz, mingo; +Cc: linux-kernel, Chris Phlipot

It looks like the tools/perf/Documentation/android.txt hasn't been updated
in a while. Following the instructions in this document to cross-compile
perf for Android results in several build errors.

This patch-set aims to fix/workaround the incompatibilities introduced
since the android perf build was last tested.

The changes were tested to build for ubuntu 16.04 as well as cross compile
for android using NDK Versions 11 and 12.

to test android arm cross compile:

$ wget http://dl.google.com/android/repository/android-ndk-r12-linux-x86_64.zip
$ unzip android-ndk-r12-linux-x86_64.zip
$ export NDK_TOOLCHAIN=`pwd`/android-ndk-r12/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-
$ export NDK_SYSROOT=`pwd`/android-ndk-r12/platforms/android-24/arch-arm
$ make WERROR=0 ARCH=arm CROSS_COMPILE=${NDK_TOOLCHAIN} EXTRA_CFLAGS="-pie --sysroot=${NDK_SYSROOT}"


Chris Phlipot (4):
  tools lib api: Respect WERROR=0 for build
  tools lib subcmd: Respect WERROR=0 for build
  perf tool: Fix build when sysconf doesn't support cache line size
  perf tool: Update android build documentation

 tools/lib/api/Makefile               |  8 +++++++-
 tools/lib/subcmd/Makefile            |  8 +++++++-
 tools/perf/Documentation/android.txt | 16 ++++++++--------
 tools/perf/perf.c                    |  4 ++++
 4 files changed, 26 insertions(+), 10 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] tools lib api: Respect WERROR=0 for build
  2016-07-01  5:12 [PATCH 0/4] perf tool: Fix Android build Chris Phlipot
@ 2016-07-01  5:12 ` Chris Phlipot
  2016-07-05 10:22   ` [tip:perf/core] " tip-bot for Chris Phlipot
  2016-07-01  5:12 ` [PATCH 2/4] tools lib subcmd: " Chris Phlipot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-01  5:12 UTC (permalink / raw)
  To: acme, peterz, mingo; +Cc: linux-kernel, Chris Phlipot

This enables the workaround for compilers that generate warnings when
compiling libapi.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/lib/api/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
index 67ff93e..c7ceea6 100644
--- a/tools/lib/api/Makefile
+++ b/tools/lib/api/Makefile
@@ -17,7 +17,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libapi.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+
+# Treat warnings as errors unless directed not to
+ifneq ($(WERROR),0)
+  CFLAGS += -Werror
+endif
+
 CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
 CFLAGS += -I$(srctree)/tools/lib/api
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] tools lib subcmd: Respect WERROR=0 for build
  2016-07-01  5:12 [PATCH 0/4] perf tool: Fix Android build Chris Phlipot
  2016-07-01  5:12 ` [PATCH 1/4] tools lib api: Respect WERROR=0 for build Chris Phlipot
@ 2016-07-01  5:12 ` Chris Phlipot
  2016-07-05 10:22   ` [tip:perf/core] " tip-bot for Chris Phlipot
  2016-07-01  5:12 ` [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size Chris Phlipot
  2016-07-01  5:12 ` [PATCH 4/4] perf tool: Update android build documentation Chris Phlipot
  3 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-01  5:12 UTC (permalink / raw)
  To: acme, peterz, mingo; +Cc: linux-kernel, Chris Phlipot

this enables the workaround for compilers that generate warnings when
compiling libsubcmd.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/lib/subcmd/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index a810370..ce4b7e5 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,7 +19,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libsubcmd.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+
+# Treat warnings as errors unless directed not to
+ifneq ($(WERROR),0)
+  CFLAGS += -Werror
+endif
+
 CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 
 CFLAGS += -I$(srctree)/tools/include/
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-01  5:12 [PATCH 0/4] perf tool: Fix Android build Chris Phlipot
  2016-07-01  5:12 ` [PATCH 1/4] tools lib api: Respect WERROR=0 for build Chris Phlipot
  2016-07-01  5:12 ` [PATCH 2/4] tools lib subcmd: " Chris Phlipot
@ 2016-07-01  5:12 ` Chris Phlipot
  2016-07-04 22:48   ` Arnaldo Carvalho de Melo
  2016-07-01  5:12 ` [PATCH 4/4] perf tool: Update android build documentation Chris Phlipot
  3 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-01  5:12 UTC (permalink / raw)
  To: acme, peterz, mingo; +Cc: linux-kernel, Chris Phlipot

Enable perf to build on libc implementations where sysconf() doesn't
support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.

For example, the Bionic implementation does not support this as a
paremter. Older versions of Bionic will throw an error when this is passed
in as a parameter, and more recent versions will just return 0 as the
cache line size.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/perf/perf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 8f21922..113ca5b 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -509,7 +509,11 @@ int main(int argc, const char **argv)
 
 	/* The page_size is placed in util object. */
 	page_size = sysconf(_SC_PAGE_SIZE);
+#ifdef _SC_LEVEL1_DCACHE_LINESIZE
 	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
+#else
+	cacheline_size = 0;
+#endif
 
 	if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0)
 		sysctl_perf_event_max_stack = value;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] perf tool: Update android build documentation
  2016-07-01  5:12 [PATCH 0/4] perf tool: Fix Android build Chris Phlipot
                   ` (2 preceding siblings ...)
  2016-07-01  5:12 ` [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size Chris Phlipot
@ 2016-07-01  5:12 ` Chris Phlipot
  2016-07-05 10:23   ` [tip:perf/core] perf tools: " tip-bot for Chris Phlipot
  3 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-01  5:12 UTC (permalink / raw)
  To: acme, peterz, mingo; +Cc: linux-kernel, Chris Phlipot

Update the android build documentation according to recent android build
fixes. The instructions for step 1a and step 2 were updated to work with
NDK version 11(oldest supported version) and NDK version 12(current
version).

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
---
 tools/perf/Documentation/android.txt | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Documentation/android.txt b/tools/perf/Documentation/android.txt
index 8484c3a..24a5999 100644
--- a/tools/perf/Documentation/android.txt
+++ b/tools/perf/Documentation/android.txt
@@ -12,14 +12,14 @@ Set the NDK variable to point to the path where you installed the NDK:
 
 2. Set cross-compiling environment variables for NDK toolchain and sysroot.
 For arm:
-  export NDK_TOOLCHAIN=${NDK}/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/arm-linux-androideabi-
-  export NDK_SYSROOT=${NDK}/platforms/android-9/arch-arm
+  export NDK_TOOLCHAIN=${NDK}/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-
+  export NDK_SYSROOT=${NDK}/platforms/android-24/arch-arm
 For x86:
-  export NDK_TOOLCHAIN=${NDK}/toolchains/x86-4.6/prebuilt/linux-x86/bin/i686-linux-android-
-  export NDK_SYSROOT=${NDK}/platforms/android-9/arch-x86
+  export NDK_TOOLCHAIN=${NDK}/toolchains/x86-4.9/prebuilt/linux-x86_64/bin/i686-linux-android-
+  export NDK_SYSROOT=${NDK}/platforms/android-24/arch-x86
 
-This method is not working for Android NDK versions up to Revision 8b.
-perf uses some bionic enhancements that are not included in these NDK versions.
+This method is only tested for Android NDK versions Revision 11b and later.
+perf uses some bionic enhancements that are not included in prior NDK versions.
 You can use method (b) described below instead.
 
 (b). Use the Android source tree
@@ -49,9 +49,9 @@ II. Compile perf for Android
 ------------------------------------------------
 You need to run make with the NDK toolchain and sysroot defined above:
 For arm:
-  make ARCH=arm CROSS_COMPILE=${NDK_TOOLCHAIN} CFLAGS="--sysroot=${NDK_SYSROOT}"
+  make WERROR=0 ARCH=arm CROSS_COMPILE=${NDK_TOOLCHAIN} EXTRA_CFLAGS="-pie --sysroot=${NDK_SYSROOT}"
 For x86:
-  make ARCH=x86 CROSS_COMPILE=${NDK_TOOLCHAIN} CFLAGS="--sysroot=${NDK_SYSROOT}"
+  make WERROR=0 ARCH=x86 CROSS_COMPILE=${NDK_TOOLCHAIN} EXTRA_CFLAGS="-pie --sysroot=${NDK_SYSROOT}"
 
 III. Install perf
 -----------------------------------------------
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-01  5:12 ` [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size Chris Phlipot
@ 2016-07-04 22:48   ` Arnaldo Carvalho de Melo
  2016-07-05  0:19     ` Chris Phlipot
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-07-04 22:48 UTC (permalink / raw)
  To: Chris Phlipot; +Cc: peterz, mingo, linux-kernel

Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu:
> Enable perf to build on libc implementations where sysconf() doesn't
> support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.
> 
> For example, the Bionic implementation does not support this as a
> paremter. Older versions of Bionic will throw an error when this is passed
> in as a parameter, and more recent versions will just return 0 as the
> cache line size.
> 
> Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
> ---
>  tools/perf/perf.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 8f21922..113ca5b 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -509,7 +509,11 @@ int main(int argc, const char **argv)
>  
>  	/* The page_size is placed in util object. */
>  	page_size = sysconf(_SC_PAGE_SIZE);
> +#ifdef _SC_LEVEL1_DCACHE_LINESIZE
>  	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> +#else
> +	cacheline_size = 0;
> +#endif

Couldn't we instead fallback to:

sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size)

?
  
>  	if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0)
>  		sysctl_perf_event_max_stack = value;
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-04 22:48   ` Arnaldo Carvalho de Melo
@ 2016-07-05  0:19     ` Chris Phlipot
  2016-07-05  0:26       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-05  0:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: peterz, mingo, linux-kernel



On 07/04/2016 03:48 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu:
>> Enable perf to build on libc implementations where sysconf() doesn't
>> support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.
>>
>> For example, the Bionic implementation does not support this as a
>> paremter. Older versions of Bionic will throw an error when this is passed
>> in as a parameter, and more recent versions will just return 0 as the
>> cache line size.
>>
>> Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
>> ---
>>   tools/perf/perf.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> index 8f21922..113ca5b 100644
>> --- a/tools/perf/perf.c
>> +++ b/tools/perf/perf.c
>> @@ -509,7 +509,11 @@ int main(int argc, const char **argv)
>>
>>   	/* The page_size is placed in util object. */
>>   	page_size = sysconf(_SC_PAGE_SIZE);
>> +#ifdef _SC_LEVEL1_DCACHE_LINESIZE
>>   	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
>> +#else
>> +	cacheline_size = 0;
>> +#endif
>
> Couldn't we instead fallback to:
>
> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size)
>
> ?

I agree that in general this would be a better fallback, but in  all 
Android images I have tested so far, "devices/system/cpu/cpu0/cache" 
does not exist. I know not know of a good way to retrieve cache line 
size in this case.

I would be ok with attempting to get cacheline size using using the 
following methods, unless you have other ideas:

1. attempt to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE)
2. attempt to use 
sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", 
&cacheline_size)
3. set to zero if both of the above fail.


>
>>   	if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0)
>>   		sysctl_perf_event_max_stack = value;
>> --
>> 2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-05  0:19     ` Chris Phlipot
@ 2016-07-05  0:26       ` Arnaldo Carvalho de Melo
  2016-07-05  0:47         ` Chris Phlipot
  0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-07-05  0:26 UTC (permalink / raw)
  To: Chris Phlipot; +Cc: Arnaldo Carvalho de Melo, peterz, mingo, linux-kernel

Em Mon, Jul 04, 2016 at 05:19:20PM -0700, Chris Phlipot escreveu:
> 
> 
> On 07/04/2016 03:48 PM, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu:
> > > Enable perf to build on libc implementations where sysconf() doesn't
> > > support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.
> > > 
> > > For example, the Bionic implementation does not support this as a
> > > paremter. Older versions of Bionic will throw an error when this is passed
> > > in as a parameter, and more recent versions will just return 0 as the
> > > cache line size.
> > > 
> > > Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
> > > ---
> > >   tools/perf/perf.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > > index 8f21922..113ca5b 100644
> > > --- a/tools/perf/perf.c
> > > +++ b/tools/perf/perf.c
> > > @@ -509,7 +509,11 @@ int main(int argc, const char **argv)
> > > 
> > >   	/* The page_size is placed in util object. */
> > >   	page_size = sysconf(_SC_PAGE_SIZE);
> > > +#ifdef _SC_LEVEL1_DCACHE_LINESIZE
> > >   	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> > > +#else
> > > +	cacheline_size = 0;
> > > +#endif
> > 
> > Couldn't we instead fallback to:
> > 
> > sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size)
> > 
> > ?
> 
> I agree that in general this would be a better fallback, but in  all Android
> images I have tested so far, "devices/system/cpu/cpu0/cache" does not exist.
> I know not know of a good way to retrieve cache line size in this case.
> 
> I would be ok with attempting to get cacheline size using using the
> following methods, unless you have other ideas:
> 
> 1. attempt to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE)
> 2. attempt to use
> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size",
> &cacheline_size)
> 3. set to zero if both of the above fail.

Ok, but perhaps we should have some sort of warning in places using
this?

- Arnaldo
 
> 
> > 
> > >   	if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0)
> > >   		sysctl_perf_event_max_stack = value;
> > > --
> > > 2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-05  0:26       ` Arnaldo Carvalho de Melo
@ 2016-07-05  0:47         ` Chris Phlipot
  2016-07-05  0:55           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Phlipot @ 2016-07-05  0:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: peterz, mingo, linux-kernel



On 07/04/2016 05:26 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 04, 2016 at 05:19:20PM -0700, Chris Phlipot escreveu:
>>
>>
>> On 07/04/2016 03:48 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu:
>>>> Enable perf to build on libc implementations where sysconf() doesn't
>>>> support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.
>>>>
>>>> For example, the Bionic implementation does not support this as a
>>>> paremter. Older versions of Bionic will throw an error when this is passed
>>>> in as a parameter, and more recent versions will just return 0 as the
>>>> cache line size.
>>>>
>>>> Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
>>>> ---
>>>>    tools/perf/perf.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>>>> index 8f21922..113ca5b 100644
>>>> --- a/tools/perf/perf.c
>>>> +++ b/tools/perf/perf.c
>>>> @@ -509,7 +509,11 @@ int main(int argc, const char **argv)
>>>>
>>>>    	/* The page_size is placed in util object. */
>>>>    	page_size = sysconf(_SC_PAGE_SIZE);
>>>> +#ifdef _SC_LEVEL1_DCACHE_LINESIZE
>>>>    	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
>>>> +#else
>>>> +	cacheline_size = 0;
>>>> +#endif
>>>
>>> Couldn't we instead fallback to:
>>>
>>> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size)
>>>
>>> ?
>>
>> I agree that in general this would be a better fallback, but in  all Android
>> images I have tested so far, "devices/system/cpu/cpu0/cache" does not exist.
>> I know not know of a good way to retrieve cache line size in this case.
>>
>> I would be ok with attempting to get cacheline size using using the
>> following methods, unless you have other ideas:
>>
>> 1. attempt to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE)
>> 2. attempt to use
>> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size",
>> &cacheline_size)
>> 3. set to zero if both of the above fail.
>
> Ok, but perhaps we should have some sort of warning in places using
> this?
>
> - Arnaldo

Such as printing a warning when cacheline_size is set to zero, or simply 
adding comments to the code in areas where cacheline_size is used?

-Chris

>
>>
>>>
>>>>    	if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0)
>>>>    		sysctl_perf_event_max_stack = value;
>>>> --
>>>> 2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size
  2016-07-05  0:47         ` Chris Phlipot
@ 2016-07-05  0:55           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-07-05  0:55 UTC (permalink / raw)
  To: Chris Phlipot; +Cc: Arnaldo Carvalho de Melo, peterz, mingo, linux-kernel

Em Mon, Jul 04, 2016 at 05:47:12PM -0700, Chris Phlipot escreveu:
> On 07/04/2016 05:26 PM, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jul 04, 2016 at 05:19:20PM -0700, Chris Phlipot escreveu:
> > > On 07/04/2016 03:48 PM, Arnaldo Carvalho de Melo wrote:
> > > > Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu:
> > > > > Enable perf to build on libc implementations where sysconf() doesn't
> > > > > support _SC_LEVEL1_DCACHE_LINESIZE as a parameter.
> > > > > 
> > > > > For example, the Bionic implementation does not support this as a
> > > > > paremter. Older versions of Bionic will throw an error when this is passed
> > > > > in as a parameter, and more recent versions will just return 0 as the
> > > > > cache line size.
> > > > > 
> > > > > Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
> > > > > ---
> > > > >    tools/perf/perf.c | 4 ++++
> > > > >    1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> > > > > index 8f21922..113ca5b 100644
> > > > > --- a/tools/perf/perf.c
> > > > > +++ b/tools/perf/perf.c
> > > > > @@ -509,7 +509,11 @@ int main(int argc, const char **argv)
> > > > > 
> > > > >    	/* The page_size is placed in util object. */
> > > > >    	page_size = sysconf(_SC_PAGE_SIZE);
> > > > > +#ifdef _SC_LEVEL1_DCACHE_LINESIZE
> > > > >    	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
> > > > > +#else
> > > > > +	cacheline_size = 0;
> > > > > +#endif
> > > > 
> > > > Couldn't we instead fallback to:
> > > > 
> > > > sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size)
> > > > 
> > > > ?
> > > 
> > > I agree that in general this would be a better fallback, but in  all Android
> > > images I have tested so far, "devices/system/cpu/cpu0/cache" does not exist.
> > > I know not know of a good way to retrieve cache line size in this case.
> > > 
> > > I would be ok with attempting to get cacheline size using using the
> > > following methods, unless you have other ideas:
> > > 
> > > 1. attempt to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE)
> > > 2. attempt to use
> > > sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size",
> > > &cacheline_size)
> > > 3. set to zero if both of the above fail.
> > 
> > Ok, but perhaps we should have some sort of warning in places using
> > this?
> 
> Such as printing a warning when cacheline_size is set to zero, or simply
> adding comments to the code in areas where cacheline_size is used?

So:

[acme@jouet linux]$ find tools/perf -name "*.[ch]" | xargs grep cacheline_size
tools/perf/perf.c:	cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE);
tools/perf/util/util.c:int cacheline_size;
tools/perf/util/util.h:extern int cacheline_size;
tools/perf/util/sort.h:	return (address & ~(cacheline_size - 1));
tools/perf/util/sort.h:	return (address & (cacheline_size - 1));
[acme@jouet linux]$ 

So it seems this is used by some of the sort keys, i.e. if the user
uses some of:

[acme@jouet linux]$ perf report -h -s

 Usage: perf report [<options>]

    -s, --sort <key[,key2...]>
                          sort by key(s): pid, comm, dso, symbol,
parent, cpu, srcline, ... Please refer the man page for the complete
list.

[acme@jouet linux]$ 

At least this one:

	   ·   dcacheline: the cacheline the data address is on at the
time of the sample

There may be others, need to thoroughly check.

I.e. if "dcacheline" is in -s/--sort, this needs to fail and the user be
informed that it is not possible, perhaps we should add a --cacheline
option to allow the user to tell the tool what is the cacheline size?

What is the usual cacheline size for Android class devices? Perhaps we
should fallback to that and give a warning?

- Arnaldo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [tip:perf/core] tools lib api: Respect WERROR=0 for build
  2016-07-01  5:12 ` [PATCH 1/4] tools lib api: Respect WERROR=0 for build Chris Phlipot
@ 2016-07-05 10:22   ` tip-bot for Chris Phlipot
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Chris Phlipot @ 2016-07-05 10:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: hpa, acme, cphlipot0, mingo, linux-kernel, tglx, peterz

Commit-ID:  b983d54473344a9ef524a231943478047a779796
Gitweb:     http://git.kernel.org/tip/b983d54473344a9ef524a231943478047a779796
Author:     Chris Phlipot <cphlipot0@gmail.com>
AuthorDate: Thu, 30 Jun 2016 22:12:32 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 20:27:26 -0300

tools lib api: Respect WERROR=0 for build

This enables the workaround for compilers that generate warnings when
compiling libapi.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1467349955-1135-2-git-send-email-cphlipot0@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/api/Makefile b/tools/lib/api/Makefile
index 67ff93e..c7ceea6 100644
--- a/tools/lib/api/Makefile
+++ b/tools/lib/api/Makefile
@@ -17,7 +17,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libapi.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+
+# Treat warnings as errors unless directed not to
+ifneq ($(WERROR),0)
+  CFLAGS += -Werror
+endif
+
 CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64
 CFLAGS += -I$(srctree)/tools/lib/api
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:perf/core] tools lib subcmd: Respect WERROR=0 for build
  2016-07-01  5:12 ` [PATCH 2/4] tools lib subcmd: " Chris Phlipot
@ 2016-07-05 10:22   ` tip-bot for Chris Phlipot
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Chris Phlipot @ 2016-07-05 10:22 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, acme, hpa, cphlipot0, peterz, mingo, linux-kernel

Commit-ID:  fd01d06ae33be63cff7d133e650cd1eb32f1d548
Gitweb:     http://git.kernel.org/tip/fd01d06ae33be63cff7d133e650cd1eb32f1d548
Author:     Chris Phlipot <cphlipot0@gmail.com>
AuthorDate: Thu, 30 Jun 2016 22:12:33 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 20:27:26 -0300

tools lib subcmd: Respect WERROR=0 for build

this enables the workaround for compilers that generate warnings when
compiling libsubcmd.

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1467349955-1135-3-git-send-email-cphlipot0@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/subcmd/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index a810370..ce4b7e5 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -19,7 +19,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libsubcmd.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -O6 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+
+# Treat warnings as errors unless directed not to
+ifneq ($(WERROR),0)
+  CFLAGS += -Werror
+endif
+
 CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE
 
 CFLAGS += -I$(srctree)/tools/include/

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:perf/core] perf tools: Update android build documentation
  2016-07-01  5:12 ` [PATCH 4/4] perf tool: Update android build documentation Chris Phlipot
@ 2016-07-05 10:23   ` tip-bot for Chris Phlipot
  0 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Chris Phlipot @ 2016-07-05 10:23 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, peterz, hpa, cphlipot0, acme, mingo, linux-kernel

Commit-ID:  3d0376113ed9cf92b86885bf5102944b61523f5b
Gitweb:     http://git.kernel.org/tip/3d0376113ed9cf92b86885bf5102944b61523f5b
Author:     Chris Phlipot <cphlipot0@gmail.com>
AuthorDate: Thu, 30 Jun 2016 22:12:35 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Jul 2016 20:27:27 -0300

perf tools: Update android build documentation

Update the android build documentation according to recent android build
fixes. The instructions for step 1a and step 2 were updated to work with
NDK version 11(oldest supported version) and NDK version 12(current
version).

Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1467349955-1135-5-git-send-email-cphlipot0@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/android.txt | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Documentation/android.txt b/tools/perf/Documentation/android.txt
index 8484c3a..24a5999 100644
--- a/tools/perf/Documentation/android.txt
+++ b/tools/perf/Documentation/android.txt
@@ -12,14 +12,14 @@ Set the NDK variable to point to the path where you installed the NDK:
 
 2. Set cross-compiling environment variables for NDK toolchain and sysroot.
 For arm:
-  export NDK_TOOLCHAIN=${NDK}/toolchains/arm-linux-androideabi-4.6/prebuilt/linux-x86/bin/arm-linux-androideabi-
-  export NDK_SYSROOT=${NDK}/platforms/android-9/arch-arm
+  export NDK_TOOLCHAIN=${NDK}/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-
+  export NDK_SYSROOT=${NDK}/platforms/android-24/arch-arm
 For x86:
-  export NDK_TOOLCHAIN=${NDK}/toolchains/x86-4.6/prebuilt/linux-x86/bin/i686-linux-android-
-  export NDK_SYSROOT=${NDK}/platforms/android-9/arch-x86
+  export NDK_TOOLCHAIN=${NDK}/toolchains/x86-4.9/prebuilt/linux-x86_64/bin/i686-linux-android-
+  export NDK_SYSROOT=${NDK}/platforms/android-24/arch-x86
 
-This method is not working for Android NDK versions up to Revision 8b.
-perf uses some bionic enhancements that are not included in these NDK versions.
+This method is only tested for Android NDK versions Revision 11b and later.
+perf uses some bionic enhancements that are not included in prior NDK versions.
 You can use method (b) described below instead.
 
 (b). Use the Android source tree
@@ -49,9 +49,9 @@ II. Compile perf for Android
 ------------------------------------------------
 You need to run make with the NDK toolchain and sysroot defined above:
 For arm:
-  make ARCH=arm CROSS_COMPILE=${NDK_TOOLCHAIN} CFLAGS="--sysroot=${NDK_SYSROOT}"
+  make WERROR=0 ARCH=arm CROSS_COMPILE=${NDK_TOOLCHAIN} EXTRA_CFLAGS="-pie --sysroot=${NDK_SYSROOT}"
 For x86:
-  make ARCH=x86 CROSS_COMPILE=${NDK_TOOLCHAIN} CFLAGS="--sysroot=${NDK_SYSROOT}"
+  make WERROR=0 ARCH=x86 CROSS_COMPILE=${NDK_TOOLCHAIN} EXTRA_CFLAGS="-pie --sysroot=${NDK_SYSROOT}"
 
 III. Install perf
 -----------------------------------------------

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-07-05 10:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01  5:12 [PATCH 0/4] perf tool: Fix Android build Chris Phlipot
2016-07-01  5:12 ` [PATCH 1/4] tools lib api: Respect WERROR=0 for build Chris Phlipot
2016-07-05 10:22   ` [tip:perf/core] " tip-bot for Chris Phlipot
2016-07-01  5:12 ` [PATCH 2/4] tools lib subcmd: " Chris Phlipot
2016-07-05 10:22   ` [tip:perf/core] " tip-bot for Chris Phlipot
2016-07-01  5:12 ` [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size Chris Phlipot
2016-07-04 22:48   ` Arnaldo Carvalho de Melo
2016-07-05  0:19     ` Chris Phlipot
2016-07-05  0:26       ` Arnaldo Carvalho de Melo
2016-07-05  0:47         ` Chris Phlipot
2016-07-05  0:55           ` Arnaldo Carvalho de Melo
2016-07-01  5:12 ` [PATCH 4/4] perf tool: Update android build documentation Chris Phlipot
2016-07-05 10:23   ` [tip:perf/core] perf tools: " tip-bot for Chris Phlipot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).