* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
@ 2016-09-22 7:56 Jisheng Zhang
2016-09-22 13:58 ` Thomas Gleixner
0 siblings, 1 reply; 6+ messages in thread
From: Jisheng Zhang @ 2016-09-22 7:56 UTC (permalink / raw)
To: linux-arm-kernel
Currently ti-32k can be used as a scheduler clock. We properly marked
omap_32k_read_sched_clock() as notrace but we then call another
function ti_32k_read_cycles() that _wasn't_ notrace.
Having a traceable function in the sched_clock() path leads to a
recursion within ftrace and a kernel crash.
Fix this by adding notrace attribute to the ti_32k_read_cycles()
function.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
drivers/clocksource/timer-ti-32k.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-ti-32k.c b/drivers/clocksource/timer-ti-32k.c
index 92b7e39..cf5b14e 100644
--- a/drivers/clocksource/timer-ti-32k.c
+++ b/drivers/clocksource/timer-ti-32k.c
@@ -65,7 +65,7 @@ static inline struct ti_32k *to_ti_32k(struct clocksource *cs)
return container_of(cs, struct ti_32k, cs);
}
-static cycle_t ti_32k_read_cycles(struct clocksource *cs)
+static cycle_t notrace ti_32k_read_cycles(struct clocksource *cs)
{
struct ti_32k *ti = to_ti_32k(cs);
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
2016-09-22 7:56 [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion Jisheng Zhang
@ 2016-09-22 13:58 ` Thomas Gleixner
2016-09-22 14:29 ` Steven Rostedt
2016-09-23 2:04 ` Jisheng Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2016-09-22 13:58 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016, Jisheng Zhang wrote:
> Currently ti-32k can be used as a scheduler clock. We properly marked
> omap_32k_read_sched_clock() as notrace but we then call another
> function ti_32k_read_cycles() that _wasn't_ notrace.
>
> Having a traceable function in the sched_clock() path leads to a
> recursion within ftrace and a kernel crash.
Kernel crash? Doesn't ftrace core prevent recursion?
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
2016-09-22 13:58 ` Thomas Gleixner
@ 2016-09-22 14:29 ` Steven Rostedt
2016-09-23 2:04 ` Jisheng Zhang
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2016-09-22 14:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016 15:58:03 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 22 Sep 2016, Jisheng Zhang wrote:
>
> > Currently ti-32k can be used as a scheduler clock. We properly marked
> > omap_32k_read_sched_clock() as notrace but we then call another
> > function ti_32k_read_cycles() that _wasn't_ notrace.
> >
> > Having a traceable function in the sched_clock() path leads to a
> > recursion within ftrace and a kernel crash.
>
> Kernel crash? Doesn't ftrace core prevent recursion?
>
There is recursion protection, but there are some holes, as well as
calls where ftrace can't protect itself.
What triggered the bug? Just simple enabling of function tracing? And
what arch? I would like to close these holes. Although, I should add
some kind of flag to notify the user (or at least for me) that recursion
is happening, because that can really be a performance hit on tracing.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
2016-09-22 13:58 ` Thomas Gleixner
2016-09-22 14:29 ` Steven Rostedt
@ 2016-09-23 2:04 ` Jisheng Zhang
2016-09-23 2:45 ` Steven Rostedt
1 sibling, 1 reply; 6+ messages in thread
From: Jisheng Zhang @ 2016-09-23 2:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
On Thu, 22 Sep 2016 15:58:03 +0200 Thomas Gleixner wrote:
> On Thu, 22 Sep 2016, Jisheng Zhang wrote:
>
> > Currently ti-32k can be used as a scheduler clock. We properly marked
> > omap_32k_read_sched_clock() as notrace but we then call another
> > function ti_32k_read_cycles() that _wasn't_ notrace.
> >
> > Having a traceable function in the sched_clock() path leads to a
> > recursion within ftrace and a kernel crash.
>
> Kernel crash? Doesn't ftrace core prevent recursion?
a recent similar issue:
http://www.spinics.net/lists/arm-kernel/msg533480.html
Thanks,
Jisheng
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
2016-09-23 2:04 ` Jisheng Zhang
@ 2016-09-23 2:45 ` Steven Rostedt
2016-09-23 2:48 ` Jisheng Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2016-09-23 2:45 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 23 Sep 2016 10:04:31 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi Thomas,
>
> On Thu, 22 Sep 2016 15:58:03 +0200 Thomas Gleixner wrote:
>
> > On Thu, 22 Sep 2016, Jisheng Zhang wrote:
> >
> > > Currently ti-32k can be used as a scheduler clock. We properly marked
> > > omap_32k_read_sched_clock() as notrace but we then call another
> > > function ti_32k_read_cycles() that _wasn't_ notrace.
> > >
> > > Having a traceable function in the sched_clock() path leads to a
> > > recursion within ftrace and a kernel crash.
> >
> > Kernel crash? Doesn't ftrace core prevent recursion?
>
> a recent similar issue:
>
> http://www.spinics.net/lists/arm-kernel/msg533480.html
Right. But Thomas brought up recursion detection. And I said that would
be the fix, but now thinking about it, I've updated the recursion
protection so that timer issues should not cause a crash.
I'd like to know more, as this appears to be mostly arm related.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
2016-09-23 2:45 ` Steven Rostedt
@ 2016-09-23 2:48 ` Jisheng Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2016-09-23 2:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 22 Sep 2016 22:45:14 -0400 Steven Rostedt wrote:
> On Fri, 23 Sep 2016 10:04:31 +0800
> Jisheng Zhang <jszhang@marvell.com> wrote:
>
> > Hi Thomas,
> >
> > On Thu, 22 Sep 2016 15:58:03 +0200 Thomas Gleixner wrote:
> >
> > > On Thu, 22 Sep 2016, Jisheng Zhang wrote:
> > >
> > > > Currently ti-32k can be used as a scheduler clock. We properly marked
> > > > omap_32k_read_sched_clock() as notrace but we then call another
> > > > function ti_32k_read_cycles() that _wasn't_ notrace.
> > > >
> > > > Having a traceable function in the sched_clock() path leads to a
> > > > recursion within ftrace and a kernel crash.
> > >
> > > Kernel crash? Doesn't ftrace core prevent recursion?
> >
> > a recent similar issue:
> >
> > http://www.spinics.net/lists/arm-kernel/msg533480.html
>
> Right. But Thomas brought up recursion detection. And I said that would
> be the fix, but now thinking about it, I've updated the recursion
> protection so that timer issues should not cause a crash.
>
Got it. Thanks for the clarification
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-23 2:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-22 7:56 [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion Jisheng Zhang
2016-09-22 13:58 ` Thomas Gleixner
2016-09-22 14:29 ` Steven Rostedt
2016-09-23 2:04 ` Jisheng Zhang
2016-09-23 2:45 ` Steven Rostedt
2016-09-23 2:48 ` Jisheng Zhang
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).