Re: [PATCH v3] perf/core: Set event shadow time for inactive events too

[Peter Zijlstra]

On Mon, Dec 20, 2021 at 10:30:34AM +0100, Peter Zijlstra wrote:

> Ohh.. argh. I think I see why, it looses the context time enable edge,
> and because this is all strictly per-event in the uapi (there is no ctx
> representation) it can't be cured by improving ctx time handling :/
> 
> Bah, I so hate this.
> 

I wonder... could we get away with something like this...


---
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2722,13 +2722,17 @@ void arch_perf_update_userpage(struct pe
 	struct cyc2ns_data data;
 	u64 offset;
 
-	userpg->cap_user_time = 0;
-	userpg->cap_user_time_zero = 0;
-	userpg->cap_user_rdpmc =
-		!!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED);
+	userpg->cap_user_rdpmc = !!(event->hw.flags & PERF_X86_EVENT_RDPMC_ALLOWED);
 	userpg->pmc_width = x86_pmu.cntval_bits;
 
-	if (!using_native_sched_clock() || !sched_clock_stable())
+	if (unlikely(!using_native_sched_clock() || !sched_clock_stable())) {
+		userpg->cap_user_time = 0;
+		userpg->cap_user_time_zero = 0;
+		return;
+	}
+
+	/* already set the time fields before */
+	if (likely(userpf->cap_user_time))
 		return;
 
 	cyc2ns_read_begin(&data);
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -612,6 +612,7 @@ struct swevent_hlist {
 #define PERF_ATTACH_ITRACE	0x10
 #define PERF_ATTACH_SCHED_CB	0x20
 #define PERF_ATTACH_CHILD	0x40
+#define PERF_ATTACH_SELF	0x80
 
 struct perf_cgroup;
 struct perf_buffer;
@@ -812,6 +813,7 @@ struct perf_event_context {
 
 	int				nr_events;
 	int				nr_active;
+	int				nr_self;
 	int				is_active;
 	int				nr_stat;
 	int				nr_freq;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1877,6 +1877,8 @@ list_add_event(struct perf_event *event,
 	ctx->nr_events++;
 	if (event->attr.inherit_stat)
 		ctx->nr_stat++;
+	if (event->attach_state & PERF_ATTACH_SELF)
+		ctx->nr_self++;
 
 	if (event->state > PERF_EVENT_STATE_OFF)
 		perf_cgroup_event_enable(event, ctx);
@@ -2068,6 +2070,8 @@ list_del_event(struct perf_event *event,
 	ctx->nr_events--;
 	if (event->attr.inherit_stat)
 		ctx->nr_stat--;
+	if (event->attach_state & PERF_ATTACH_SELF)
+		ctx->nr_self--;
 
 	list_del_rcu(&event->event_entry);
 
@@ -3755,7 +3759,8 @@ static noinline int visit_groups_merge(s
  */
 static inline bool event_update_userpage(struct perf_event *event)
 {
-	if (likely(!atomic_read(&event->mmap_count)))
+	if (likely(!atomic_read(&event->mmap_count) ||
+		   !(event->attach_state & PERF_ATTACH_SELF)))
 		return false;
 
 	perf_event_update_time(event);
@@ -3800,7 +3805,8 @@ static int merge_sched_in(struct perf_ev
 		} else {
 			ctx->rotate_necessary = 1;
 			perf_mux_hrtimer_restart(cpuctx);
-			group_update_userpage(event);
+			if (ctx->nr_self)
+				group_update_userpage(event);
 		}
 	}
 
@@ -5900,6 +5906,9 @@ void perf_event_update_userpage(struct p
 	if (!rb)
 		goto unlock;
 
+	if (!(event->attach_state & PERF_ATTACH_SELF))
+		goto unlock;
+
 	/*
 	 * compute total_time_enabled, total_time_running
 	 * based on snapshot values taken when the event
@@ -11613,6 +11622,8 @@ perf_event_alloc(struct perf_event_attr
 		 * pmu before we get a ctx.
 		 */
 		event->hw.target = get_task_struct(task);
+		if (event->hw.target == current && !attr->inherit)
+			event->attach_state |= PERF_ATTACH_SELF;
 	}
 
 	event->clock = &local_clock;