Messages in this thread Patch in this message | | | Date | Tue, 21 Dec 2021 13:39:28 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3] perf/core: Set event shadow time for inactive events too |
| |
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;
| |