[lkml]   [2018]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] perf: Prevent recursion in ring buffer
On Wed, Sep 12, 2018 at 09:33:17PM +0200, Jiri Olsa wrote:

> # perf record -e 'sched:sched_switch,sched:sched_wakeup' perf bench sched messaging

> The reason for the corruptions are some of the scheduling tracepoints,
> that have __perf_task dfined and thus allow to store data to another
> cpu ring buffer:
> sched_waking
> sched_wakeup
> sched_wakeup_new
> sched_stat_wait
> sched_stat_sleep
> sched_stat_iowait
> sched_stat_blocked

> And then iterates events of the 'task' and store the sample
> for any task's event that passes tracepoint checks:
> ctx = rcu_dereference(task->perf_event_ctxp[perf_sw_context]);
> list_for_each_entry_rcu(event, &ctx->event_list, event_entry) {
> if (event->attr.type != PERF_TYPE_TRACEPOINT)
> continue;
> if (event->attr.config != entry->type)
> continue;
> perf_swevent_event(event, count, &data, regs);
> }
> Above code can race with same code running on another cpu,
> ending up with 2 cpus trying to store under the same ring
> buffer, which is not handled at the moment.

It can yes, however the only way I can see this breaking is if we use
!inherited events with a strict per-task buffer, but your record command
doesn't use that.

Now, your test-case uses inherited events, which would all share the
buffer, however IIRC inherited events require per-task-per-cpu buffers,
because there is already no guarantee the various tasks run on the same
CPU in the first place.

This means we _should_ write to the @task's local CPU buffer, and that
would work again.

Let me try and figure out where this is going wrong.

 \ /
  Last update: 2018-09-13 09:41    [W:1.233 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site