[RFC 1/2] perf: update shadow timestamp before add event

[RFC 1/2] perf: update shadow timestamp before add event

[Shaohua Li]

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

[RFC 2/2] perf: update userspace page info for software event

[Shaohua Li]

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

Re: [RFC 2/2] perf: update userspace page info for software event

[Peter Zijlstra]

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.

Re: [RFC 2/2] perf: update userspace page info for software event

[Shaohua Li]

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

Re: [RFC 2/2] perf: update userspace page info for software event

[Shaohua Li]

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