[PATCH] Export save_stack_trace_tsk for ARM and ARM64

[PATCH] Export save_stack_trace_tsk for ARM and ARM64

[Dustin Brown]

To help in debugging driver modules on ARM platforms, export
save_stack_trace_tsk() for ARM and ARM64 architectures.
---
 arch/arm/kernel/stacktrace.c   | 1 +
 arch/arm64/kernel/stacktrace.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index 3a2fa203637a..564bb468b7ba 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -171,6 +171,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
 	__save_stack_trace(tsk, trace, 1);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index feac80c22f61..09d37d66b630 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -210,6 +210,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 
 	put_task_stack(tsk);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH] Export save_stack_trace_tsk for ARM and ARM64

[Will Deacon]

On Mon, Jun 12, 2017 at 03:24:31PM -0700, Dustin Brown wrote:
> To help in debugging driver modules on ARM platforms, export
> save_stack_trace_tsk() for ARM and ARM64 architectures.
> ---
>  arch/arm/kernel/stacktrace.c   | 1 +
>  arch/arm64/kernel/stacktrace.c | 1 +
>  2 files changed, 2 insertions(+)

If you split this into two patches, I can pick up the arm64 one.

Will

[PATCH 1/2] arm: Export save_stack_trace_tsk()

[Dustin Brown]

To aid in debugging driver modules, export save_stack_trace_tsk() for
the ARM architecture.
---
 arch/arm/kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index 3a2fa203637a..564bb468b7ba 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -171,6 +171,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
 	__save_stack_trace(tsk, trace, 1);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Dustin Brown]

To aid in debugging driver modules, export save_stack_trace_tsk() for
the ARM64 architecture.
---
 arch/arm64/kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index feac80c22f61..09d37d66b630 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -210,6 +210,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 
 	put_task_stack(tsk);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Will Deacon]

On Tue, Jun 13, 2017 at 10:33:08AM -0700, Dustin Brown wrote:
> To aid in debugging driver modules, export save_stack_trace_tsk() for
> the ARM64 architecture.
> ---
>  arch/arm64/kernel/stacktrace.c | 1 +
>  1 file changed, 1 insertion(+)

Please can you repost with your Signed-off-by line added to the patch? I
can't merge anything from you without that.

Thanks,

Will

> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index feac80c22f61..09d37d66b630 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -210,6 +210,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
>  
>  	put_task_stack(tsk);
>  }
> +EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
>  
>  void save_stack_trace(struct stack_trace *trace)
>  {
> -- 
> 2.12.2
> 

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Russell King - ARM Linux]

On Tue, Jun 13, 2017 at 06:44:06PM +0100, Will Deacon wrote:
> On Tue, Jun 13, 2017 at 10:33:08AM -0700, Dustin Brown wrote:
> > To aid in debugging driver modules, export save_stack_trace_tsk() for
> > the ARM64 architecture.
> > ---
> >  arch/arm64/kernel/stacktrace.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Please can you repost with your Signed-off-by line added to the patch? I
> can't merge anything from you without that.

I think it would also be a good thing to have a little more information
about how this is being used in driver modules.

If it's for ad-hoc debugging, then I see no reason for it to be merged
into mainline kernels just for the utility of people adding code to
drivers to perform that debug - they might as well temporarily add the
export at the same time IMHO.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 1/2] arm: Export save_stack_trace_tsk()

[Dustin Brown]

The kernel watchdog is a great debugging tool for finding tasks that
consume a disproportionate amount of CPU time in contiguous chunks. One
can imagine building a similar watchdog for arbitrary driver threads
using save_stack_trace_tsk() and print_stack_trace(). However, this is
not viable for dynamically loaded driver modules on ARM platforms
because save_stack_trace_tsk() is not exported for those architectures.
Export save_stack_trace_tsk() for the ARM architecture to align with x86
and support various debugging use cases such as arbitrary driver thread
watchdog timers.

Signed-off-by: Dustin Brown <dustinb@codeaurora.org>
---
 arch/arm/kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index 3a2fa203637a..564bb468b7ba 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -171,6 +171,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 {
 	__save_stack_trace(tsk, trace, 1);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Dustin Brown]

The kernel watchdog is a great debugging tool for finding tasks that
consume a disproportionate amount of CPU time in contiguous chunks. One
can imagine building a similar watchdog for arbitrary driver threads
using save_stack_trace_tsk() and print_stack_trace(). However, this is
not viable for dynamically loaded driver modules on ARM platforms
because save_stack_trace_tsk() is not exported for those architectures.
Export save_stack_trace_tsk() for the ARM64 architecture to align with
x86 and support various debugging use cases such as arbitrary driver
thread watchdog timers.

Signed-off-by: Dustin Brown <dustinb@codeaurora.org>
---
 arch/arm64/kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index feac80c22f61..09d37d66b630 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -210,6 +210,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
 
 	put_task_stack(tsk);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH 1/2] arm: Export save_stack_trace_tsk()

[Dustin Brown]

Hi Everyone,

I just wanted to touch base and see where we are with respect to this one. I
see that the arm64 version of this patch has already landed for 4.13. Is
there anything needed from me for this? I apologize in advance if I am just
being impatient.

Thanks,

-Dustin

-----Original Message-----
From: Dustin Brown [mailto:dustinb at codeaurora.org] 
Sent: Tuesday, June 13, 2017 11:41 AM
To: linux at armlinux.org.uk; catalin.marinas at arm.com; will.deacon at arm.com;
linux-arm-kernel at lists.infradead.org
Cc: Dustin Brown <dustinb@codeaurora.org>
Subject: [PATCH 1/2] arm: Export save_stack_trace_tsk()

The kernel watchdog is a great debugging tool for finding tasks that
consume a disproportionate amount of CPU time in contiguous chunks. One
can imagine building a similar watchdog for arbitrary driver threads
using save_stack_trace_tsk() and print_stack_trace(). However, this is
not viable for dynamically loaded driver modules on ARM platforms
because save_stack_trace_tsk() is not exported for those architectures.
Export save_stack_trace_tsk() for the ARM architecture to align with x86
and support various debugging use cases such as arbitrary driver thread
watchdog timers.

Signed-off-by: Dustin Brown <dustinb@codeaurora.org>
---
 arch/arm/kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index 3a2fa203637a..564bb468b7ba 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -171,6 +171,7 @@ void save_stack_trace_tsk(struct task_struct *tsk,
struct stack_trace *trace)
 {
 	__save_stack_trace(tsk, trace, 1);
 }
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
 
 void save_stack_trace(struct stack_trace *trace)
 {
-- 
2.12.2

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Will Deacon]

On Tue, Jun 13, 2017 at 07:16:03PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 13, 2017 at 06:44:06PM +0100, Will Deacon wrote:
> > On Tue, Jun 13, 2017 at 10:33:08AM -0700, Dustin Brown wrote:
> > > To aid in debugging driver modules, export save_stack_trace_tsk() for
> > > the ARM64 architecture.
> > > ---
> > >  arch/arm64/kernel/stacktrace.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > 
> > Please can you repost with your Signed-off-by line added to the patch? I
> > can't merge anything from you without that.
> 
> I think it would also be a good thing to have a little more information
> about how this is being used in driver modules.

That would certainly help to motivate the change; I was just going by the
fact that most other architectures export this symbol and therefore it is
assumedly useful to somebody.

> If it's for ad-hoc debugging, then I see no reason for it to be merged
> into mainline kernels just for the utility of people adding code to
> drivers to perform that debug - they might as well temporarily add the
> export at the same time IMHO.

A quick look at the Debian archive suggests that it's used by "systemtap"
(although this seems to be part of an autoconf helper) and "kpatch".

Dustin -- do you have another use-case behind this?

Will

[PATCH 2/2] arm64: Export save_stack_trace_tsk()

[Dustin Brown]

Hi Will,

I sent out updated patches yesterday with better descriptions, but I am not
sure I got the threads associated correctly.

We have a dynamically loaded kernel driver with a worker thread which
processes a queue. From time to time, defects get introduced that cause this
thread to hang for long periods of time, or deadlock. To catch these
quickly, we introduced a watchdog timer that does a bug-on when individual
work items take longer than some threshold. This works great when the worker
thread is on the CPU at the point of timeout, but in cases where it went to
sleep on a mutex or a kmalloc, we want to dump the worker thread's stack
trace specifically.

This is code that is expected to live for years, and is not a one-off
debugging instance.

Thanks,

-Dustin

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com] 
Sent: Wednesday, June 14, 2017 3:07 AM
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Dustin Brown <dustinb@codeaurora.org>; catalin.marinas at arm.com;
linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 2/2] arm64: Export save_stack_trace_tsk()

On Tue, Jun 13, 2017 at 07:16:03PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 13, 2017 at 06:44:06PM +0100, Will Deacon wrote:
> > On Tue, Jun 13, 2017 at 10:33:08AM -0700, Dustin Brown wrote:
> > > To aid in debugging driver modules, export save_stack_trace_tsk() 
> > > for the ARM64 architecture.
> > > ---
> > >  arch/arm64/kernel/stacktrace.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > 
> > Please can you repost with your Signed-off-by line added to the 
> > patch? I can't merge anything from you without that.
> 
> I think it would also be a good thing to have a little more 
> information about how this is being used in driver modules.

That would certainly help to motivate the change; I was just going by the
fact that most other architectures export this symbol and therefore it is
assumedly useful to somebody.

> If it's for ad-hoc debugging, then I see no reason for it to be merged 
> into mainline kernels just for the utility of people adding code to 
> drivers to perform that debug - they might as well temporarily add the 
> export at the same time IMHO.

A quick look at the Debian archive suggests that it's used by "systemtap"
(although this seems to be part of an autoconf helper) and "kpatch".

Dustin -- do you have another use-case behind this?

Will