| Message ID | 20160922075621.3725-1-jszhang@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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);
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(-)