On Thu, Sep 13, 2018 at 09:07:40AM +0200, Peter Zijlstra wrote:
> On Wed, Sep 12, 2018 at 09:33:17PM +0200, Jiri Olsa wrote:
> > Some of the scheduling tracepoints allow the perf_tp_event
> > code to write to ring buffer under different cpu than the
> > code is running on.
>
> ARGH.. that is indeed borken.
>
> > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > index 4a9937076331..0c976ac414c5 100644
> > --- a/kernel/events/ring_buffer.c
> > +++ b/kernel/events/ring_buffer.c
> > @@ -101,6 +101,7 @@ static void perf_output_put_handle(struct
> > perf_output_handle *handle)
> >
> > out:
> > preempt_enable();
> > + atomic_set(&rb->recursion, 0);
> > }
> >
> > static __always_inline bool
> > @@ -145,6 +146,12 @@ __perf_output_begin(struct perf_output_handle *handle,
> > goto out;
> > }
> >
> > + if (atomic_cmpxchg(&rb->recursion, 0, 1) != 0) {
> > + if (rb->nr_pages)
> > + local_inc(&rb->lost);
> > + goto out;
> > + }
> > +
> > handle->rb = rb;
> > handle->event = event;
> >
> > @@ -286,6 +293,7 @@ ring_buffer_init(struct ring_buffer *rb, long
> > watermark, int flags)
> > rb->overwrite = 1;
> >
> > atomic_set(&rb->refcount, 1);
> > + atomic_set(&rb->recursion, 0);
> >
> > INIT_LIST_HEAD(&rb->event_list);
> > spin_lock_init(&rb->event_lock);
>
> That's not a recursion count, that's a test-and-set spinlock, and you
> got the memory ordering wrong for that.
>
> Also, we tried very hard to avoid atomic ops in the ring-buffer and you
> just wrecked that. Worse, you wrecked previously working interrupt
> nesting output.
ah.. the interrupt will also bail out now.. right :-\
>
> Let me have a look at this.
>
