Messages in this thread | | | From | Namhyung Kim <> | Date | Sun, 5 Dec 2021 12:56:14 -0800 | Subject | Re: [PATCH v2] perf/core: Set event shadow time for inactive events too |
| |
Hi Peter,
On Sun, Dec 5, 2021 at 6:16 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Tue, Nov 30, 2021 at 08:58:07PM -0800, Namhyung Kim wrote: > > From: Namhyung Kim <namhyung@google.com> > > > > While f79256532682 ("perf/core: fix userpage->time_enabled of inactive > > events") fixed this problem for user rdpmc usage, bperf (perf stat > > with BPF) still has the same problem that accessing inactive perf > > events from BPF using bpf_perf_event_read_value(). > > > > +static inline void group_update_event_time(struct perf_event *group_event) > > { > > + struct perf_event *event; > > + struct perf_event_context *ctx = group_event->ctx; > > :-( surely you're aware of the reverse xmas tree thing by now?
Will change the order.
> > > > > + perf_event_update_time(group_event); > > + perf_set_shadow_time(group_event, ctx); > > > > + for_each_sibling_event(event, group_event) { > > + perf_event_update_time(event); > > + perf_set_shadow_time(event, ctx); > > + } > > > > + if (likely(!atomic_read(&group_event->mmap_count))) > > return; > > > > + perf_event_update_userpage(group_event); > > + > > for_each_sibling_event(event, group_event) > > + perf_event_update_userpage(event); > > How does it make sense to chase those pointers twice?
OK, then I think I can use the existing code to update event time before checking the mmap count.
Thanks, Namhyung
> > > }
| |