Messages in this thread | | | Date | Tue, 23 Apr 2019 19:58:50 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] trace: Fix preempt_enable_no_resched() abuse |
| |
On Tue, 23 Apr 2019 22:03:18 +0200 Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Apr 23, 2019 at 09:55:59PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 23, 2019 at 03:41:32PM -0400, Waiman Long wrote: > > > > I saw a number of instances of > > > preempt_enable_no_resched() without right next a schedule(). > > > > Look more closely.. and let me know, if true, those are bugs that need > > fixing. > > > > Argghhh.. BPF... > > /me shakes head, Steve...
/me points finger to Frederic ;-)
> > --- > Subject: trace: Fix preempt_enable_no_resched() abuse > > Unless the very next line is schedule(), or implies it, one must not use > preempt_enable_no_resched(). It can cause a preemption to go missing and > thereby cause arbitrary delays, breaking the PREEMPT=y invariant. > > Cc: Steven Rostedt <rostedt@goodmis.org> > Fixes: 37886f6a9f62 ("ring-buffer: add api to allow a tracer to change clock source")
That commit just moved the buggy code. That tag should be:
Fixes: 2c2d7329d8af ("tracing/ftrace: use preempt_enable_no_resched_notrace in ring_buffer_time_stamp()")
OK, this isn't quite fair to point all the blame on Frederic, because it did fix a bug. But the real fix for that bug was your fix here:
499d79559ffe4b ("sched/core: More notrace annotations")
-- Steve
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/trace/ring_buffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 41b6f96e5366..4ee8d8aa3d0f 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -762,7 +762,7 @@ u64 ring_buffer_time_stamp(struct ring_buffer > *buffer, int cpu) > preempt_disable_notrace(); > time = rb_time_stamp(buffer); > - preempt_enable_no_resched_notrace(); > + preempt_enable_notrace(); > > return time; > }
| |