| Message ID | 20170413070309.10497-1-bsingharora@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 539df7fcb303f2cbe8021a27839928485937cd6b |
| Headers | show |
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 >
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
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 >
On Thu, Apr 13, 2017 at 6:41 PM, Christophe LEROY <christophe.leroy@c-s.fr> wrote: > > > 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 > No worries! >> >>> >>> 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". > 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 understand Yes Balbir Singh.
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. > > 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 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=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 > >
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
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
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
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
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
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(+)