* [RFC 1/2] perf: update shadow timestamp before add event @ 2015-01-22 21:09 Shaohua Li 2015-01-22 21:09 ` [RFC 2/2] perf: update userspace page info for software event Shaohua Li 0 siblings, 1 reply; 5+ messages in thread From: Shaohua Li @ 2015-01-22 21:09 UTC (permalink / raw) To: linux-kernel; +Cc: Kernel-team, Peter Zijlstra, Andy Lutomirski, Ingo Molnar Update the shadow timestamp before start event. .add might use the timestamp. Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Ingo Molnar <mingo@kernel.org> Signed-off-by: Shaohua Li <shli@fb.com> --- kernel/events/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index 882f835..4edde3e 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1769,6 +1769,10 @@ event_sched_in(struct perf_event *event, perf_pmu_disable(event->pmu); + event->tstamp_running += tstamp - event->tstamp_stopped; + + perf_set_shadow_time(event, ctx, tstamp); + if (event->pmu->add(event, PERF_EF_START)) { event->state = PERF_EVENT_STATE_INACTIVE; event->oncpu = -1; @@ -1776,10 +1780,6 @@ event_sched_in(struct perf_event *event, goto out; } - event->tstamp_running += tstamp - event->tstamp_stopped; - - perf_set_shadow_time(event, ctx, tstamp); - if (!is_software_event(event)) cpuctx->active_oncpu++; ctx->nr_active++; -- 1.8.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC 2/2] perf: update userspace page info for software event 2015-01-22 21:09 [RFC 1/2] perf: update shadow timestamp before add event Shaohua Li @ 2015-01-22 21:09 ` Shaohua Li 2015-01-23 8:44 ` Peter Zijlstra 0 siblings, 1 reply; 5+ messages in thread From: Shaohua Li @ 2015-01-22 21:09 UTC (permalink / raw) To: linux-kernel; +Cc: Kernel-team, Peter Zijlstra, Andy Lutomirski, Ingo Molnar For hardware event, the userspace page of the event gets updated in context switch, so if we read time in the page, we get updated info. For software event, this is missed currently. This patch makes the behavior consistency. With this patch, we can implement clock_gettime(THREAD_CPUTIME) with PERF_COUNT_SW_DUMMY in userspace as suggested by Andy and Peter. Code likes this: if (pc->cap_user_time) { do { seq = pc->lock; barrier(); running = pc->time_running; cyc = rdtsc(); time_mult = pc->time_mult; time_shift = pc->time_shift; time_offset = pc->time_offset; barrier(); } while (pc->lock != seq); quot = (cyc >> time_shift); rem = cyc & ((1 << time_shift) - 1); delta = time_offset + quot * time_mult + ((rem * time_mult) >> time_shift); running += delta; return running; } I tried in a busy system, the userspace page updating hasn't noticeable overhead. Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Ingo Molnar <mingo@kernel.org> Signed-off-by: Shaohua Li <shli@fb.com> --- kernel/events/core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index 4edde3e..4221240 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5950,6 +5950,7 @@ static int perf_swevent_add(struct perf_event *event, int flags) } hlist_add_head_rcu(&event->hlist_entry, head); + perf_event_update_userpage(event); return 0; } @@ -6419,6 +6420,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags) { if (flags & PERF_EF_START) cpu_clock_event_start(event, flags); + perf_event_update_userpage(event); return 0; } @@ -6493,6 +6495,7 @@ static int task_clock_event_add(struct perf_event *event, int flags) { if (flags & PERF_EF_START) task_clock_event_start(event, flags); + perf_event_update_userpage(event); return 0; } -- 1.8.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] perf: update userspace page info for software event 2015-01-22 21:09 ` [RFC 2/2] perf: update userspace page info for software event Shaohua Li @ 2015-01-23 8:44 ` Peter Zijlstra 2015-01-23 15:57 ` Shaohua Li 0 siblings, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2015-01-23 8:44 UTC (permalink / raw) To: Shaohua Li; +Cc: linux-kernel, Kernel-team, Andy Lutomirski, Ingo Molnar On Thu, Jan 22, 2015 at 01:09:02PM -0800, Shaohua Li wrote: > --- > kernel/events/core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 4edde3e..4221240 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -5950,6 +5950,7 @@ static int perf_swevent_add(struct perf_event *event, int flags) > } > > hlist_add_head_rcu(&event->hlist_entry, head); > + perf_event_update_userpage(event); > > return 0; > } > @@ -6419,6 +6420,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags) > { > if (flags & PERF_EF_START) > cpu_clock_event_start(event, flags); > + perf_event_update_userpage(event); > > return 0; > } > @@ -6493,6 +6495,7 @@ static int task_clock_event_add(struct perf_event *event, int flags) > { > if (flags & PERF_EF_START) > task_clock_event_start(event, flags); > + perf_event_update_userpage(event); > > return 0; > } How about the one I sent; which adds it to {start,stop} instead of add? {start,stop} is the right place to add them, although this add might be sufficient for your use case. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] perf: update userspace page info for software event 2015-01-23 8:44 ` Peter Zijlstra @ 2015-01-23 15:57 ` Shaohua Li 2015-01-29 7:00 ` Shaohua Li 0 siblings, 1 reply; 5+ messages in thread From: Shaohua Li @ 2015-01-23 15:57 UTC (permalink / raw) To: Peter Zijlstra; +Cc: linux-kernel, Kernel-team, Andy Lutomirski, Ingo Molnar On Fri, Jan 23, 2015 at 09:44:51AM +0100, Peter Zijlstra wrote: > On Thu, Jan 22, 2015 at 01:09:02PM -0800, Shaohua Li wrote: > > --- > > kernel/events/core.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index 4edde3e..4221240 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -5950,6 +5950,7 @@ static int perf_swevent_add(struct perf_event *event, int flags) > > } > > > > hlist_add_head_rcu(&event->hlist_entry, head); > > + perf_event_update_userpage(event); > > > > return 0; > > } > > @@ -6419,6 +6420,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags) > > { > > if (flags & PERF_EF_START) > > cpu_clock_event_start(event, flags); > > + perf_event_update_userpage(event); > > > > return 0; > > } > > @@ -6493,6 +6495,7 @@ static int task_clock_event_add(struct perf_event *event, int flags) > > { > > if (flags & PERF_EF_START) > > task_clock_event_start(event, flags); > > + perf_event_update_userpage(event); > > > > return 0; > > } > > How about the one I sent; which adds it to {start,stop} instead of add? > {start,stop} is the right place to add them, although this add might be > sufficient for your use case. Hi Peter, I tried {start, stop}, it doesn't work (doesn't get called in context switch) and I still get a CLOCK_MONOTONIC. So I added it to .add, which is called in context switch and I got correct thread time. Am I missing anything? Thanks, Shaohua ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 2/2] perf: update userspace page info for software event 2015-01-23 15:57 ` Shaohua Li @ 2015-01-29 7:00 ` Shaohua Li 0 siblings, 0 replies; 5+ messages in thread From: Shaohua Li @ 2015-01-29 7:00 UTC (permalink / raw) To: Peter Zijlstra; +Cc: linux-kernel, Kernel-team, Andy Lutomirski, Ingo Molnar Ping! On Fri, Jan 23, 2015 at 07:57:24AM -0800, Shaohua Li wrote: > On Fri, Jan 23, 2015 at 09:44:51AM +0100, Peter Zijlstra wrote: > > On Thu, Jan 22, 2015 at 01:09:02PM -0800, Shaohua Li wrote: > > > --- > > > kernel/events/core.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > > index 4edde3e..4221240 100644 > > > --- a/kernel/events/core.c > > > +++ b/kernel/events/core.c > > > @@ -5950,6 +5950,7 @@ static int perf_swevent_add(struct perf_event *event, int flags) > > > } > > > > > > hlist_add_head_rcu(&event->hlist_entry, head); > > > + perf_event_update_userpage(event); > > > > > > return 0; > > > } > > > @@ -6419,6 +6420,7 @@ static int cpu_clock_event_add(struct perf_event *event, int flags) > > > { > > > if (flags & PERF_EF_START) > > > cpu_clock_event_start(event, flags); > > > + perf_event_update_userpage(event); > > > > > > return 0; > > > } > > > @@ -6493,6 +6495,7 @@ static int task_clock_event_add(struct perf_event *event, int flags) > > > { > > > if (flags & PERF_EF_START) > > > task_clock_event_start(event, flags); > > > + perf_event_update_userpage(event); > > > > > > return 0; > > > } > > > > How about the one I sent; which adds it to {start,stop} instead of add? > > {start,stop} is the right place to add them, although this add might be > > sufficient for your use case. > Hi Peter, > I tried {start, stop}, it doesn't work (doesn't get called in context > switch) and I still get a CLOCK_MONOTONIC. So I added it to .add, which > is called in context switch and I got correct thread time. Am I missing > anything? > > Thanks, > Shaohua ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-29 7:01 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-22 21:09 [RFC 1/2] perf: update shadow timestamp before add event Shaohua Li 2015-01-22 21:09 ` [RFC 2/2] perf: update userspace page info for software event Shaohua Li 2015-01-23 8:44 ` Peter Zijlstra 2015-01-23 15:57 ` Shaohua Li 2015-01-29 7:00 ` Shaohua Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).