Make perf_event_exit_event() more robust, such that we can use it from other contexts. Specifically the up and coming remove_on_exec. For this to work we need to address a few issues. Remove_on_exec will not destroy the entire context, so we cannot rely on TASK_TOMBSTONE to disable event_function_call() and we thus have to use perf_remove_from_context(). When using perf_remove_from_context(), there's two races to consider. The first is against close(), where we can have concurrent tear-down of the event. The second is against child_list iteration, which should not find a half baked event. To address this, teach perf_remove_from_context() to special case !ctx->is_active and about DETACH_CHILD. [ elver@google.com: fix racing parent/child exit in sync_child_event(). ] Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20210408103605.1676875-2-elver@google.com
Unified · Split
| @@ -607,6 +607,7 @@ struct swevent_hlist { · 606 unmodified lines | ||
| 607 | 607 | #define PERF_ATTACH_TASK_DATA 0x08 |
| 608 | 608 | #define PERF_ATTACH_ITRACE 0x10 |
| 609 | 609 | #define PERF_ATTACH_SCHED_CB 0x20 |
| 610 | +#define PERF_ATTACH_CHILD 0x40 | |
| 610 | 611 | |
| 611 | 612 | struct perf_cgroup; |
| 612 | 613 | struct perf_buffer; |
| @@ -2205,6 +2205,26 @@ out: · 2204 unmodified lines | ||
| 2205 | 2205 | perf_event__header_size(leader); |
| 2206 | 2206 | } |
| 2207 | 2207 | |
| 2208 | +static void sync_child_event(struct perf_event *child_event); | |
| 2209 | + | |
| 2210 | +static void perf_child_detach(struct perf_event *event) | |
| 2211 | +{ | |
| 2212 | + struct perf_event *parent_event = event->parent; | |
| 2213 | + | |
| 2214 | + if (!(event->attach_state & PERF_ATTACH_CHILD)) | |
| 2215 | + return; | |
| 2216 | + | |
| 2217 | + event->attach_state &= ~PERF_ATTACH_CHILD; | |
| 2218 | + | |
| 2219 | + if (WARN_ON_ONCE(!parent_event)) | |
| 2220 | + return; | |
| 2221 | + | |
| 2222 | + lockdep_assert_held(&parent_event->child_mutex); | |
| 2223 | + | |
| 2224 | + sync_child_event(event); | |
| 2225 | + list_del_init(&event->child_list); | |
| 2226 | +} | |
| 2227 | + | |
| 2208 | 2228 | static bool is_orphaned_event(struct perf_event *event) |
| 2209 | 2229 | { |
| 2210 | 2230 | return event->state == PERF_EVENT_STATE_DEAD; |
| @@ -2312,6 +2332,7 @@ group_sched_out(struct perf_event *group_event, · 101 unmodified lines | ||
| 2312 | 2332 | } |
| 2313 | 2333 | |
| 2314 | 2334 | #define DETACH_GROUP 0x01UL |
| 2335 | +#define DETACH_CHILD 0x02UL | |
| 2315 | 2336 | |
| 2316 | 2337 | /* |
| 2317 | 2338 | * Cross CPU call to remove a performance event |
| @@ -2335,6 +2356,8 @@ __perf_remove_from_context(struct perf_event *event, · 17 unmodified lines | ||
| 2335 | 2356 | event_sched_out(event, cpuctx, ctx); |
| 2336 | 2357 | if (flags & DETACH_GROUP) |
| 2337 | 2358 | perf_group_detach(event); |
| 2359 | + if (flags & DETACH_CHILD) | |
| 2360 | + perf_child_detach(event); | |
| 2338 | 2361 | list_del_event(event, ctx); |
| 2339 | 2362 | |
| 2340 | 2363 | if (!ctx->nr_events && ctx->is_active) { |
| @@ -2363,25 +2386,21 @@ static void perf_remove_from_context(struct perf_event *event, unsigned long fla · 22 unmodified lines | ||
| 2363 | 2386 | |
| 2364 | 2387 | lockdep_assert_held(&ctx->mutex); |
| 2365 | 2388 | |
| 2366 | − event_function_call(event, __perf_remove_from_context, (void *)flags); | |
| 2367 | − | |
| 2368 | 2389 | /* |
| 2369 | − * The above event_function_call() can NO-OP when it hits | |
| 2370 | − * TASK_TOMBSTONE. In that case we must already have been detached | |
| 2371 | − * from the context (by perf_event_exit_event()) but the grouping | |
| 2372 | − * might still be in-tact. | |
| 2390 | + * Because of perf_event_exit_task(), perf_remove_from_context() ought | |
| 2391 | + * to work in the face of TASK_TOMBSTONE, unlike every other | |
| 2392 | + * event_function_call() user. | |
| 2373 | 2393 | */ |
| 2374 | − WARN_ON_ONCE(event->attach_state & PERF_ATTACH_CONTEXT); | |
| 2375 | − if ((flags & DETACH_GROUP) && | |
| 2376 | − (event->attach_state & PERF_ATTACH_GROUP)) { | |
| 2377 | − /* | |
| 2378 | − * Since in that case we cannot possibly be scheduled, simply | |
| 2379 | − * detach now. | |
| 2380 | − */ | |
| 2381 | − raw_spin_lock_irq(&ctx->lock); | |
| 2382 | − perf_group_detach(event); | |
| 2394 | + raw_spin_lock_irq(&ctx->lock); | |
| 2395 | + if (!ctx->is_active) { | |
| 2396 | + __perf_remove_from_context(event, __get_cpu_context(ctx), | |
| 2397 | + ctx, (void *)flags); | |
| 2383 | 2398 | raw_spin_unlock_irq(&ctx->lock); |
| 2399 | + return; | |
| 2384 | 2400 | } |
| 2401 | + raw_spin_unlock_irq(&ctx->lock); | |
| 2402 | + | |
| 2403 | + event_function_call(event, __perf_remove_from_context, (void *)flags); | |
| 2385 | 2404 | } |
| 2386 | 2405 | |
| 2387 | 2406 | /* |
| @@ -12377,14 +12396,17 @@ void perf_pmu_migrate_context(struct pmu *pmu, int src_cpu, int dst_cpu) · 9989 unmodified lines | ||
| 12377 | 12396 | } |
| 12378 | 12397 | EXPORT_SYMBOL_GPL(perf_pmu_migrate_context); |
| 12379 | 12398 | |
| 12380 | −static void sync_child_event(struct perf_event *child_event, | |
| 12381 | − struct task_struct *child) | |
| 12399 | +static void sync_child_event(struct perf_event *child_event) | |
| 12382 | 12400 | { |
| 12383 | 12401 | struct perf_event *parent_event = child_event->parent; |
| 12384 | 12402 | u64 child_val; |
| 12385 | 12403 | |
| 12386 | − if (child_event->attr.inherit_stat) | |
| 12387 | − perf_event_read_event(child_event, child); | |
| 12404 | + if (child_event->attr.inherit_stat) { | |
| 12405 | + struct task_struct *task = child_event->ctx->task; | |
| 12406 | + | |
| 12407 | + if (task && task != TASK_TOMBSTONE) | |
| 12408 | + perf_event_read_event(child_event, task); | |
| 12409 | + } | |
| 12388 | 12410 | |
| 12389 | 12411 | child_val = perf_event_count(child_event); |
| 12390 | 12412 | |
| @@ -12399,60 +12421,53 @@ static void sync_child_event(struct perf_event *child_event, · 8 unmodified lines | ||
| 12399 | 12421 | } |
| 12400 | 12422 | |
| 12401 | 12423 | static void |
| 12402 | −perf_event_exit_event(struct perf_event *child_event, | |
| 12403 | − struct perf_event_context *child_ctx, | |
| 12404 | − struct task_struct *child) | |
| 12424 | +perf_event_exit_event(struct perf_event *event, struct perf_event_context *ctx) | |
| 12405 | 12425 | { |
| 12406 | − struct perf_event *parent_event = child_event->parent; | |
| 12426 | + struct perf_event *parent_event = event->parent; | |
| 12427 | + unsigned long detach_flags = 0; | |
| 12407 | 12428 | |
| 12408 | − /* | |
| 12409 | − * Do not destroy the 'original' grouping; because of the context | |
| 12410 | − * switch optimization the original events could've ended up in a | |
| 12411 | − * random child task. | |
| 12412 | − * | |
| 12413 | − * If we were to destroy the original group, all group related | |
| 12414 | − * operations would cease to function properly after this random | |
| 12415 | − * child dies. | |
| 12416 | − * | |
| 12417 | − * Do destroy all inherited groups, we don't care about those | |
| 12418 | − * and being thorough is better. | |
| 12419 | − */ | |
| 12420 | − raw_spin_lock_irq(&child_ctx->lock); | |
| 12421 | − WARN_ON_ONCE(child_ctx->is_active); | |
| 12429 | + if (parent_event) { | |
| 12430 | + /* | |
| 12431 | + * Do not destroy the 'original' grouping; because of the | |
| 12432 | + * context switch optimization the original events could've | |
| 12433 | + * ended up in a random child task. | |
| 12434 | + * | |
| 12435 | + * If we were to destroy the original group, all group related | |
| 12436 | + * operations would cease to function properly after this | |
| 12437 | + * random child dies. | |
| 12438 | + * | |
| 12439 | + * Do destroy all inherited groups, we don't care about those | |
| 12440 | + * and being thorough is better. | |
| 12441 | + */ | |
| 12442 | + detach_flags = DETACH_GROUP | DETACH_CHILD; | |
| 12443 | + mutex_lock(&parent_event->child_mutex); | |
| 12444 | + } | |
| 12422 | 12445 | |
| 12423 | − if (parent_event) | |
| 12424 | − perf_group_detach(child_event); | |
| 12425 | − list_del_event(child_event, child_ctx); | |
| 12426 | − perf_event_set_state(child_event, PERF_EVENT_STATE_EXIT); /* is_event_hup() */ | |
| 12427 | − raw_spin_unlock_irq(&child_ctx->lock); | |
| 12446 | + perf_remove_from_context(event, detach_flags); | |
| 12447 | + | |
| 12448 | + raw_spin_lock_irq(&ctx->lock); | |
| 12449 | + if (event->state > PERF_EVENT_STATE_EXIT) | |
| 12450 | + perf_event_set_state(event, PERF_EVENT_STATE_EXIT); | |
| 12451 | + raw_spin_unlock_irq(&ctx->lock); | |
| 12428 | 12452 | |
| 12429 | 12453 | /* |
| 12430 | − * Parent events are governed by their filedesc, retain them. | |
| 12454 | + * Child events can be freed. | |
| 12431 | 12455 | */ |
| 12432 | − if (!parent_event) { | |
| 12433 | − perf_event_wakeup(child_event); | |
| 12456 | + if (parent_event) { | |
| 12457 | + mutex_unlock(&parent_event->child_mutex); | |
| 12458 | + /* | |
| 12459 | + * Kick perf_poll() for is_event_hup(); | |
| 12460 | + */ | |
| 12461 | + perf_event_wakeup(parent_event); | |
| 12462 | + free_event(event); | |
| 12463 | + put_event(parent_event); | |
| 12434 | 12464 | return; |
| 12435 | 12465 | } |
| 12436 | − /* | |
| 12437 | − * Child events can be cleaned up. | |
| 12438 | − */ | |
| 12439 | − | |
| 12440 | − sync_child_event(child_event, child); | |
| 12441 | 12466 | |
| 12442 | 12467 | /* |
| 12443 | − * Remove this event from the parent's list | |
| 12444 | − */ | |
| 12445 | − WARN_ON_ONCE(parent_event->ctx->parent_ctx); | |
| 12446 | − mutex_lock(&parent_event->child_mutex); | |
| 12447 | − list_del_init(&child_event->child_list); | |
| 12448 | − mutex_unlock(&parent_event->child_mutex); | |
| 12449 | − | |
| 12450 | − /* | |
| 12451 | − * Kick perf_poll() for is_event_hup(). | |
| 12468 | + * Parent events are governed by their filedesc, retain them. | |
| 12452 | 12469 | */ |
| 12453 | − perf_event_wakeup(parent_event); | |
| 12454 | − free_event(child_event); | |
| 12455 | − put_event(parent_event); | |
| 12470 | + perf_event_wakeup(event); | |
| 12456 | 12471 | } |
| 12457 | 12472 | |
| 12458 | 12473 | static void perf_event_exit_task_context(struct task_struct *child, int ctxn) |
| @@ -12509,7 +12524,7 @@ static void perf_event_exit_task_context(struct task_struct *child, int ctxn) · 50 unmodified lines | ||
| 12509 | 12524 | perf_event_task(child, child_ctx, 0); |
| 12510 | 12525 | |
| 12511 | 12526 | list_for_each_entry_safe(child_event, next, &child_ctx->event_list, event_entry) |
| 12512 | − perf_event_exit_event(child_event, child_ctx, child); | |
| 12527 | + perf_event_exit_event(child_event, child_ctx); | |
| 12513 | 12528 | |
| 12514 | 12529 | mutex_unlock(&child_ctx->mutex); |
| 12515 | 12530 | |
| @@ -12769,6 +12784,7 @@ inherit_event(struct perf_event *parent_event, · 253 unmodified lines | ||
| 12769 | 12784 | */ |
| 12770 | 12785 | raw_spin_lock_irqsave(&child_ctx->lock, flags); |
| 12771 | 12786 | add_event_to_ctx(child_event, child_ctx); |
| 12787 | + child_event->attach_state |= PERF_ATTACH_CHILD; | |
| 12772 | 12788 | raw_spin_unlock_irqrestore(&child_ctx->lock, flags); |
| 12773 | 12789 | |
| 12774 | 12790 | /* |