[PATCH] powerpc/configs: Enable function trace by default

[PATCH] powerpc/configs: Enable function trace by default

[Balbir Singh]

We expect to have these configs on by default, most
distros turn them off, its always good to have them on
so that we can use them and hopefully not break them

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/configs/powernv_defconfig | 4 ++++
 arch/powerpc/configs/ppc64_defconfig   | 3 +++
 arch/powerpc/configs/pseries_defconfig | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index 0695ce0..eff3968 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -295,7 +295,11 @@ CONFIG_DEBUG_STACK_USAGE=y
 CONFIG_DEBUG_STACKOVERFLOW=y
 CONFIG_LOCKUP_DETECTOR=y
 CONFIG_LATENCYTOP=y
+CONFIG_FTRACE=y
+CONFIG_FUNCTION_TRACER=y
+CONFIG_FUNCTION_GRAPH_TRACER=y
 CONFIG_SCHED_TRACER=y
+CONFIG_FTRACE_SYSCALLS=y
 CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_UPROBE_EVENT=y
 CONFIG_CODE_PATCHING_SELFTEST=y
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index e353168f9..17c6c12 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -328,6 +328,9 @@ CONFIG_DEBUG_STACKOVERFLOW=y
 CONFIG_LOCKUP_DETECTOR=y
 CONFIG_DEBUG_MUTEXES=y
 CONFIG_LATENCYTOP=y
+CONFIG_FTRACE=y
+CONFIG_FUNCTION_TRACER=y
+CONFIG_FUNCTION_GRAPH_TRACER=y
 CONFIG_SCHED_TRACER=y
 CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_UPROBE_EVENT=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index 1a61aa2..a445960 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -293,6 +293,9 @@ CONFIG_DEBUG_STACK_USAGE=y
 CONFIG_DEBUG_STACKOVERFLOW=y
 CONFIG_LOCKUP_DETECTOR=y
 CONFIG_LATENCYTOP=y
+CONFIG_FTRACE=y
+CONFIG_FUNCTION_TRACER=y
+CONFIG_FUNCTION_GRAPH_TRACER=y
 CONFIG_SCHED_TRACER=y
 CONFIG_BLK_DEV_IO_TRACE=y
 CONFIG_UPROBE_EVENT=y
-- 
2.9.3

Re: [PATCH] powerpc/configs: Enable function trace by default

[Christophe LEROY]

Hi Baldir

Le 13/04/2017 à 09:03, Balbir Singh a écrit :
> We expect to have these configs on by default, most
> distros turn them off, its always good to have them on
> so that we can use them and hopefully not break them

Isn't it the purpose of the target allyesconfig to allow such tests ?

FTRACE is quite CPU consumming, shouldn't it really be on by default ?

Christophe

>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
>  arch/powerpc/configs/powernv_defconfig | 4 ++++
>  arch/powerpc/configs/ppc64_defconfig   | 3 +++
>  arch/powerpc/configs/pseries_defconfig | 3 +++
>  3 files changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
> index 0695ce0..eff3968 100644
> --- a/arch/powerpc/configs/powernv_defconfig
> +++ b/arch/powerpc/configs/powernv_defconfig
> @@ -295,7 +295,11 @@ CONFIG_DEBUG_STACK_USAGE=y
>  CONFIG_DEBUG_STACKOVERFLOW=y
>  CONFIG_LOCKUP_DETECTOR=y
>  CONFIG_LATENCYTOP=y
> +CONFIG_FTRACE=y
> +CONFIG_FUNCTION_TRACER=y
> +CONFIG_FUNCTION_GRAPH_TRACER=y
>  CONFIG_SCHED_TRACER=y
> +CONFIG_FTRACE_SYSCALLS=y
>  CONFIG_BLK_DEV_IO_TRACE=y
>  CONFIG_UPROBE_EVENT=y
>  CONFIG_CODE_PATCHING_SELFTEST=y
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index e353168f9..17c6c12 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -328,6 +328,9 @@ CONFIG_DEBUG_STACKOVERFLOW=y
>  CONFIG_LOCKUP_DETECTOR=y
>  CONFIG_DEBUG_MUTEXES=y
>  CONFIG_LATENCYTOP=y
> +CONFIG_FTRACE=y
> +CONFIG_FUNCTION_TRACER=y
> +CONFIG_FUNCTION_GRAPH_TRACER=y
>  CONFIG_SCHED_TRACER=y
>  CONFIG_BLK_DEV_IO_TRACE=y
>  CONFIG_UPROBE_EVENT=y
> diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
> index 1a61aa2..a445960 100644
> --- a/arch/powerpc/configs/pseries_defconfig
> +++ b/arch/powerpc/configs/pseries_defconfig
> @@ -293,6 +293,9 @@ CONFIG_DEBUG_STACK_USAGE=y
>  CONFIG_DEBUG_STACKOVERFLOW=y
>  CONFIG_LOCKUP_DETECTOR=y
>  CONFIG_LATENCYTOP=y
> +CONFIG_FTRACE=y
> +CONFIG_FUNCTION_TRACER=y
> +CONFIG_FUNCTION_GRAPH_TRACER=y
>  CONFIG_SCHED_TRACER=y
>  CONFIG_BLK_DEV_IO_TRACE=y
>  CONFIG_UPROBE_EVENT=y
>

Re: [PATCH] powerpc/configs: Enable function trace by default

[Balbir Singh]

On Thu, 2017-04-13 at 09:24 +0200, Christophe LEROY wrote:
> Hi Baldir

Hi Christophe, It's Balbir

> 
> Le 13/04/2017 à 09:03, Balbir Singh a écrit :
> > We expect to have these configs on by default, most
> > distros turn them off, its always good to have them on
> > so that we can use them and hopefully not break them
> 
> Isn't it the purpose of the target allyesconfig to allow such tests ?
> 

As a developer I don't use allyesconfig for submitting patches or testing
the kernel.

> FTRACE is quite CPU consumming, shouldn't it really be on by default ?

It does some work at boot to NOP out function entry points at _mcount
locations. Is that what you are referring to? Or the overhead of the
code in terms of size? Most distro kernels have tracing on by default.

The rest of the overhead is enablement based.

Thanks for the review!
Balbir Singh

Re: [PATCH] powerpc/configs: Enable function trace by default

[Christophe LEROY]

Le 13/04/2017 à 10:19, Balbir Singh a écrit :
> On Thu, 2017-04-13 at 09:24 +0200, Christophe LEROY wrote:
>> Hi Baldir
>
> Hi Christophe, It's Balbir

Sorry

>
>>
>> Le 13/04/2017 à 09:03, Balbir Singh a écrit :
>>> We expect to have these configs on by default, most
>>> distros turn them off, its always good to have them on
>>> so that we can use them and hopefully not break them
>>
>> Isn't it the purpose of the target allyesconfig to allow such tests ?
>>
>
> As a developer I don't use allyesconfig for submitting patches or testing
> the kernel.

I do it most of the time to check compilation doesn't fail.

>
>> FTRACE is quite CPU consumming, shouldn't it really be on by default ?
>
> It does some work at boot to NOP out function entry points at _mcount
> locations. Is that what you are referring to? Or the overhead of the
> code in terms of size? Most distro kernels have tracing on by default.

You said in you patch text: "most distros turn them off".

If most distros turn it ON (and not OFF as you said), then I can 
understand we have it ON as well in defconfigs.

Christophe

>
> The rest of the overhead is enablement based.
>
> Thanks for the review!
> Balbir Singh
>

Re: [PATCH] powerpc/configs: Enable function trace by default

[Balbir Singh]

On Thu, Apr 13, 2017 at 6:41 PM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 13/04/2017 =C3=A0 10:19, Balbir Singh a =C3=A9crit :
>>
>> On Thu, 2017-04-13 at 09:24 +0200, Christophe LEROY wrote:
>>>
>>> Hi Baldir
>>
>>
>> Hi Christophe, It's Balbir
>
>
> Sorry
>
No worries!

>>
>>>
>>> Le 13/04/2017 =C3=A0 09:03, Balbir Singh a =C3=A9crit :
>>>>
>>>> We expect to have these configs on by default, most
>>>> distros turn them off, its always good to have them on
>>>> so that we can use them and hopefully not break them
>>>
>>>
>>> Isn't it the purpose of the target allyesconfig to allow such tests ?
>>>
>>
>> As a developer I don't use allyesconfig for submitting patches or testin=
g
>> the kernel.
>
>
> I do it most of the time to check compilation doesn't fail.
>
>>
>>> FTRACE is quite CPU consumming, shouldn't it really be on by default ?
>>
>>
>> It does some work at boot to NOP out function entry points at _mcount
>> locations. Is that what you are referring to? Or the overhead of the
>> code in terms of size? Most distro kernels have tracing on by default.
>
>
> You said in you patch text: "most distros turn them off".
>

My bad.. sometimes one can type faster than one can think, this is one
of those occasions :)

> If most distros turn it ON (and not OFF as you said), then I can understa=
nd

Yes

Balbir Singh.

Re: [PATCH] powerpc/configs: Enable function trace by default

[Naveen N. Rao]

Excerpts from Balbir Singh's message of April 13, 2017 12:33:
> We expect to have these configs on by default, most
> distros turn them off, its always good to have them on
> so that we can use them and hopefully not break them

Thanks! I find myself constantly enabling these options.

>=20
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
>  arch/powerpc/configs/powernv_defconfig | 4 ++++
>  arch/powerpc/configs/ppc64_defconfig   | 3 +++
>  arch/powerpc/configs/pseries_defconfig | 3 +++
>  3 files changed, 10 insertions(+)
>=20
> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/config=
s/powernv_defconfig
> index 0695ce0..eff3968 100644
> --- a/arch/powerpc/configs/powernv_defconfig
> +++ b/arch/powerpc/configs/powernv_defconfig
> @@ -295,7 +295,11 @@ CONFIG_DEBUG_STACK_USAGE=3Dy
>  CONFIG_DEBUG_STACKOVERFLOW=3Dy
>  CONFIG_LOCKUP_DETECTOR=3Dy
>  CONFIG_LATENCYTOP=3Dy
> +CONFIG_FTRACE=3Dy
> +CONFIG_FUNCTION_TRACER=3Dy
> +CONFIG_FUNCTION_GRAPH_TRACER=3Dy
>  CONFIG_SCHED_TRACER=3Dy
> +CONFIG_FTRACE_SYSCALLS=3Dy

Any reason to not enable this for ppc64 and pseries defconfigs?
Apart from that, for this patch:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>


- Naveen

>  CONFIG_BLK_DEV_IO_TRACE=3Dy
>  CONFIG_UPROBE_EVENT=3Dy
>  CONFIG_CODE_PATCHING_SELFTEST=3Dy
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/=
ppc64_defconfig
> index e353168f9..17c6c12 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -328,6 +328,9 @@ CONFIG_DEBUG_STACKOVERFLOW=3Dy
>  CONFIG_LOCKUP_DETECTOR=3Dy
>  CONFIG_DEBUG_MUTEXES=3Dy
>  CONFIG_LATENCYTOP=3Dy
> +CONFIG_FTRACE=3Dy
> +CONFIG_FUNCTION_TRACER=3Dy
> +CONFIG_FUNCTION_GRAPH_TRACER=3Dy
>  CONFIG_SCHED_TRACER=3Dy
>  CONFIG_BLK_DEV_IO_TRACE=3Dy
>  CONFIG_UPROBE_EVENT=3Dy
> diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/config=
s/pseries_defconfig
> index 1a61aa2..a445960 100644
> --- a/arch/powerpc/configs/pseries_defconfig
> +++ b/arch/powerpc/configs/pseries_defconfig
> @@ -293,6 +293,9 @@ CONFIG_DEBUG_STACK_USAGE=3Dy
>  CONFIG_DEBUG_STACKOVERFLOW=3Dy
>  CONFIG_LOCKUP_DETECTOR=3Dy
>  CONFIG_LATENCYTOP=3Dy
> +CONFIG_FTRACE=3Dy
> +CONFIG_FUNCTION_TRACER=3Dy
> +CONFIG_FUNCTION_GRAPH_TRACER=3Dy
>  CONFIG_SCHED_TRACER=3Dy
>  CONFIG_BLK_DEV_IO_TRACE=3Dy
>  CONFIG_UPROBE_EVENT=3Dy
> --=20
> 2.9.3
>=20
>=20
=

Re: [PATCH] powerpc/configs: Enable function trace by default

[Anton Blanchard]

Hi Balbir,

> > FTRACE is quite CPU consumming, shouldn't it really be on by
> > default ?  
> 
> It does some work at boot to NOP out function entry points at _mcount
> locations. Is that what you are referring to? Or the overhead of the
> code in terms of size? Most distro kernels have tracing on by default.
> 
> The rest of the overhead is enablement based.

Unfortunately the overhead is somewhat high without
CONFIG_MPROFILE_KERNEL, and enabling that option will break old
toolchains. It would be great if we could automatically enable it based
on the toolchain.

Even with CONFIG_MPROFILE_KERNEL enabled, we aren't noping out the
redundant mflr at the start of each function.

Anton

Re: [PATCH] powerpc/configs: Enable function trace by default

[Balbir Singh]

On Wed, 2017-04-19 at 21:13 +1000, Anton Blanchard wrote:
> Hi Balbir,
> 
> > > FTRACE is quite CPU consumming, shouldn't it really be on by
> > > default ?  
> > 
> > It does some work at boot to NOP out function entry points at _mcount
> > locations. Is that what you are referring to? Or the overhead of the
> > code in terms of size? Most distro kernels have tracing on by default.
> > 
> > The rest of the overhead is enablement based.
> 
> Unfortunately the overhead is somewhat high without
> CONFIG_MPROFILE_KERNEL, and enabling that option will break old
> toolchains. It would be great if we could automatically enable it based
> on the toolchain.
> 
> Even with CONFIG_MPROFILE_KERNEL enabled, we aren't noping out the
> redundant mflr at the start of each function.
>

Very good catch! I sent the enablement assuming that we want to have these
enabled to ensure we generally have these enabled in the distro kernels
and ideally want to test with these enabled and many of us turn them
on in any case. Do you see an issue with this being enabled by default?
I presume most workloads run on kernels that have them enabled these days?

CONFIG_MPROFILE_KERNEL depends on CC_USING_MPROFILE_KERNEL which is
automatically detected and is LE only.

Balbir Singh

Re: [PATCH] powerpc/configs: Enable function trace by default

[Michael Ellerman]

Balbir Singh <bsingharora@gmail.com> writes:

> On Wed, 2017-04-19 at 21:13 +1000, Anton Blanchard wrote:
>> Hi Balbir,
>> 
>> > > FTRACE is quite CPU consumming, shouldn't it really be on by
>> > > default ?  
>> > 
>> > It does some work at boot to NOP out function entry points at _mcount
>> > locations. Is that what you are referring to? Or the overhead of the
>> > code in terms of size? Most distro kernels have tracing on by default.
>> > 
>> > The rest of the overhead is enablement based.
>> 
>> Unfortunately the overhead is somewhat high without
>> CONFIG_MPROFILE_KERNEL, and enabling that option will break old
>> toolchains. It would be great if we could automatically enable it based
>> on the toolchain.
>> 
>> Even with CONFIG_MPROFILE_KERNEL enabled, we aren't noping out the
>> redundant mflr at the start of each function.
>
> Very good catch! I sent the enablement assuming that we want to have these
> enabled to ensure we generally have these enabled in the distro kernels
> and ideally want to test with these enabled and many of us turn them
> on in any case. Do you see an issue with this being enabled by default?
> I presume most workloads run on kernels that have them enabled these days?

No definitely not. Most distro kernels don't even have the code, it only
went into 4.6.

> CONFIG_MPROFILE_KERNEL depends on CC_USING_MPROFILE_KERNEL which is

No it doesn't:

config MPROFILE_KERNEL
	depends on PPC64 && CPU_LITTLE_ENDIAN
	def_bool !DISABLE_MPROFILE_KERNEL

> automatically detected and is LE only.

It's automatically detected at build time, which is too late, so our
only option is to break the build:

ifdef CONFIG_MPROFILE_KERNEL
    ifeq ($(shell $(srctree)/arch/powerpc/scripts/gcc-check-mprofile-kernel.sh $(CC) -I$(srctree)/include -D__KERNEL__),OK)
        CC_FLAGS_FTRACE := -pg -mprofile-kernel
        KBUILD_CPPFLAGS += -DCC_USING_MPROFILE_KERNEL
    else
        # If the user asked for mprofile-kernel but the toolchain doesn't
        # support it, emit a warning and deliberately break the build later
        # with mprofile-kernel-not-supported. We would prefer to make this an
        # error right here, but then the user would never be able to run
        # oldconfig to change their configuration.
        $(warning Compiler does not support mprofile-kernel, set CONFIG_DISABLE_MPROFILE_KERNEL)
        CC_FLAGS_FTRACE := -mprofile-kernel-not-supported
    endif
endif


So in short it's a big PITA.

cheers

Re: powerpc/configs: Enable function trace by default

[Michael Ellerman]

On Thu, 2017-04-13 at 07:03:09 UTC, Balbir Singh wrote:
> We expect to have these configs on by default, most
> distros turn them off, its always good to have them on
> so that we can use them and hopefully not break them
> 
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/539df7fcb303f2cbe8021a27839928

cheers