diff mbox

arm64: add HAVE_LATENCYTOP_SUPPORT config

Message ID 1446825478-24824-1-git-send-email-yalin.wang2010@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

yalin wang Nov. 6, 2015, 3:57 p.m. UTC
Add HAVE_LATENCYTOP_SUPPORT in Kconfig, so that
we can enable this feature on ARM64

Signed-off-by: yalin wang <yalin.wang2010@gmail.com>
---
 arch/arm64/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Will Deacon Nov. 6, 2015, 4:05 p.m. UTC | #1
On Fri, Nov 06, 2015 at 11:57:58PM +0800, yalin wang wrote:
> Add HAVE_LATENCYTOP_SUPPORT in Kconfig, so that
> we can enable this feature on ARM64

Do you know what the prerequisites for HAVE_LATENCYTOP_SUPPORT actually
are (beyond those explicitly listed as dependencies for CONFIG_LATENCYTOP)?

Will
yalin wang Nov. 6, 2015, 4:11 p.m. UTC | #2
i just enable it on ARM64,
and it can work,
i don’t see some special requirement to enable this config .
> On Nov 7, 2015, at 00:05, Will Deacon <will.deacon@arm.com> wrote:
> 
> On Fri, Nov 06, 2015 at 11:57:58PM +0800, yalin wang wrote:
>> Add HAVE_LATENCYTOP_SUPPORT in Kconfig, so that
>> we can enable this feature on ARM64
> 
> Do you know what the prerequisites for HAVE_LATENCYTOP_SUPPORT actually
> are (beyond those explicitly listed as dependencies for CONFIG_LATENCYTOP)?
> 
> Will
Will Deacon Nov. 6, 2015, 4:21 p.m. UTC | #3
On Sat, Nov 07, 2015 at 12:11:16AM +0800, yalin wang wrote:
> i just enable it on ARM64,
> and it can work,
> i don’t see some special requirement to enable this config .

Right, so why does HAVE_LATENCYTOP_SUPPORT exist?

Will
Heiko Carstens Nov. 10, 2015, 7:41 a.m. UTC | #4
On Fri, Nov 06, 2015 at 04:21:10PM +0000, Will Deacon wrote:
> On Sat, Nov 07, 2015 at 12:11:16AM +0800, yalin wang wrote:
> > i just enable it on ARM64,
> > and it can work,
> > i don’t see some special requirement to enable this config .
> 
> Right, so why does HAVE_LATENCYTOP_SUPPORT exist?

If I remember correctly then the only dependency was that an architecture
must have implemented save_stack_trace_tsk().
See git commit a3afe70b83fdbbd4d757d2911900d168bc798a31.

So the name of HAVE_LATENCYTOP_SUPPORT is surely a not well chosen, and I
think I introduced it back then. Oh, well.

And looking through the kernel there is at least avr32 which would break
at build time if the config option would be removed completely.

So.. renaming it to STACKTRACE_TSK_SUPPORT would be a good idea.

Will do when time permits, unless somebody else volunteers.
Will Deacon Nov. 10, 2015, 10:05 a.m. UTC | #5
Hi Heiko,

On Tue, Nov 10, 2015 at 08:41:24AM +0100, Heiko Carstens wrote:
> On Fri, Nov 06, 2015 at 04:21:10PM +0000, Will Deacon wrote:
> > On Sat, Nov 07, 2015 at 12:11:16AM +0800, yalin wang wrote:
> > > i just enable it on ARM64,
> > > and it can work,
> > > i don’t see some special requirement to enable this config .
> > 
> > Right, so why does HAVE_LATENCYTOP_SUPPORT exist?
> 
> If I remember correctly then the only dependency was that an architecture
> must have implemented save_stack_trace_tsk().
> See git commit a3afe70b83fdbbd4d757d2911900d168bc798a31.

Thanks for the pointer.

> So the name of HAVE_LATENCYTOP_SUPPORT is surely a not well chosen, and I
> think I introduced it back then. Oh, well.
> 
> And looking through the kernel there is at least avr32 which would break
> at build time if the config option would be removed completely.
> 
> So.. renaming it to STACKTRACE_TSK_SUPPORT would be a good idea.

ftrace has a similar issue and solves it by having architectures define
a `config STACKTRACE_SUPPORT' symbol. Over in kernel/trace/Kconfig,
there's a `select STACKTRACE if STACKTRACE_SUPPORT', which means
that kernel/stacktrace.c gets built and a dummy (weak symbol) version of
save_stack_trace_tsk appears.

I don't think adding another STACKTRACE-related Kconfig option is
necessarily the best thing to do. Maybe we should instead have LATENCYTOP
depend on STACKTRACE_SUPPORT (already the case) and select STACKTRACE?

Will
Heiko Carstens Nov. 10, 2015, 11:01 a.m. UTC | #6
On Tue, Nov 10, 2015 at 10:05:48AM +0000, Will Deacon wrote:
> Hi Heiko,
> 
> On Tue, Nov 10, 2015 at 08:41:24AM +0100, Heiko Carstens wrote:
> > On Fri, Nov 06, 2015 at 04:21:10PM +0000, Will Deacon wrote:
> > > On Sat, Nov 07, 2015 at 12:11:16AM +0800, yalin wang wrote:
> > > > i just enable it on ARM64,
> > > > and it can work,
> > > > i don’t see some special requirement to enable this config .
> > > 
> > > Right, so why does HAVE_LATENCYTOP_SUPPORT exist?
[...]
> > And looking through the kernel there is at least avr32 which would break
> > at build time if the config option would be removed completely.
> > 
> > So.. renaming it to STACKTRACE_TSK_SUPPORT would be a good idea.
> 
> ftrace has a similar issue and solves it by having architectures define
> a `config STACKTRACE_SUPPORT' symbol. Over in kernel/trace/Kconfig,
> there's a `select STACKTRACE if STACKTRACE_SUPPORT', which means
> that kernel/stacktrace.c gets built and a dummy (weak symbol) version of
> save_stack_trace_tsk appears.

Ah, I wasn't aware of the weak symbol.

> I don't think adding another STACKTRACE-related Kconfig option is
> necessarily the best thing to do. Maybe we should instead have LATENCYTOP
> depend on STACKTRACE_SUPPORT (already the case) and select STACKTRACE?

LATENCYTOP also already selects STACKTRACE. So it looks like
HAVE_LATENCYTOP_SUPPORT could simply be removed.
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 851fe11..782b5bd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -103,6 +103,10 @@  config ARCH_PHYS_ADDR_T_64BIT
 config MMU
 	def_bool y
 
+config HAVE_LATENCYTOP_SUPPORT
+	bool
+	default y
+
 config NO_IOPORT_MAP
 	def_bool y if !PCI