Skip to content

Commit f3c0eba

Browse files
author
Peter Zijlstra
committed
perf: Add a few assertions
While auditing 6b959ba ("perf/core: Fix reentry problem in perf_output_read_group()") a few spots were found that wanted assertions. Notable for_each_sibling_event() relies on exclusion from modification. This would normally be holding either ctx->lock or ctx->mutex, however due to how things are constructed disabling IRQs is a valid and sufficient substitute for ctx->lock. Another possible site to add assertions would be the various pmu::{add,del,read,..}() methods, but that's not trivially expressable in C -- the best option is wrappers, but those are easy enough to forget. Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
1 parent 88081cf commit f3c0eba

File tree

2 files changed

+19
-0
lines changed

2 files changed

+19
-0
lines changed

include/linux/perf_event.h

+17
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct perf_guest_info_callbacks {
6161
#include <linux/refcount.h>
6262
#include <linux/security.h>
6363
#include <linux/static_call.h>
64+
#include <linux/lockdep.h>
6465
#include <asm/local.h>
6566

6667
struct perf_callchain_entry {
@@ -634,7 +635,23 @@ struct pmu_event_list {
634635
struct list_head list;
635636
};
636637

638+
/*
639+
* event->sibling_list is modified whole holding both ctx->lock and ctx->mutex
640+
* as such iteration must hold either lock. However, since ctx->lock is an IRQ
641+
* safe lock, and is only held by the CPU doing the modification, having IRQs
642+
* disabled is sufficient since it will hold-off the IPIs.
643+
*/
644+
#ifdef CONFIG_PROVE_LOCKING
645+
#define lockdep_assert_event_ctx(event) \
646+
WARN_ON_ONCE(__lockdep_enabled && \
647+
(this_cpu_read(hardirqs_enabled) || \
648+
lockdep_is_held(&(event)->ctx->mutex) != LOCK_STATE_HELD))
649+
#else
650+
#define lockdep_assert_event_ctx(event)
651+
#endif
652+
637653
#define for_each_sibling_event(sibling, event) \
654+
lockdep_assert_event_ctx(event); \
638655
if ((event)->group_leader == (event)) \
639656
list_for_each_entry((sibling), &(event)->sibling_list, sibling_list)
640657

kernel/events/core.c

+2
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,8 @@ static void __update_context_time(struct perf_event_context *ctx, bool adv)
14681468
{
14691469
u64 now = perf_clock();
14701470

1471+
lockdep_assert_held(&ctx->lock);
1472+
14711473
if (adv)
14721474
ctx->time += now - ctx->timestamp;
14731475
ctx->timestamp = now;

0 commit comments

Comments
 (0)