lkml.org 
[lkml]   [2017]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v6 2/3]: perf/core: use context tstamp_data for skipped events on mux interrupt
From
Date
On 04.08.2017 15:35, Peter Zijlstra wrote:
> On Thu, Aug 03, 2017 at 09:47:56PM +0300, Alexey Budankov wrote:
>> On 03.08.2017 18:00, Peter Zijlstra wrote:
>
>>> Are the magic spots, right? And I'm not convinced its right.
>>>
>>> Suppose I have two events in my context, and I created them 1 minute
>>> apart. Then their respective tstamp_enabled are 1 minute apart as well.
>>> But the above doesn't seem to preserve that difference.
>>>
>>> A similar argument can be made for running I think. That is a per event
>>> value and cannot be passed along to the ctx and back.
>>
>> Aww, I see your point and it challenges my initial assumptions.
>> Let me think thru the case more. There must be some solution. Thanks!
>
> So the sensible thing is probably to rewrite the entire time tracking to
> make more sense. OTOH that's also the riskiest.
>
> Something like:
>
> __update_state_and_time(event, new_state)
> {
> u64 delta, now = perf_event_time(event);
> int old_state = event->state;
>
> event->tstamp = now;
> event->state = new_state;
>
> delta = now - event->tstamp;
> switch (state) {
> case STATE_ACTIVE:
> WARN_ON_ONCE(old_state != STATE_INACTIVE);
> event->total_time_enabled += delta;
> break;
>
> case STATE_INACTIVE:
> switch (old_state) {
> case STATE_OFF:
> /* ignore the OFF -> INACTIVE period */
> break;
>
> case STATE_ACTIVE:
> event->total_time_enabled += delta;
> event->total_time_running += delta;
> break;
>
> default:
> WARN_ONCE();
> }
> break;
>
> case STATE_OFF:
> WARN_ON_ONCE(old_state != STATE_INACTIVE)
> event->total_time_enabled += delta;
> break;
> }
> }
>
> __read_curent_times(event, u64 *enabled, u64 *running)
> {
> u64 delta, now = perf_event_time(event);
>
> delta = now - event->tstamp;
>
> *enabled = event->total_time_enabled;
> if (event->state >= STATE_INACTIVE)
> *enabled += delta;
> *running = event->total_time_running
> if (event->state == STATE_ACTIVE)
> *running += delta;
> }
>
> perhaps? That instantly solves the problem I think, because now we don't
> need to update inactive events. But maybe I missed some, could you
> verify?

Thanks for the input. I will check it.

>
>

\
 
 \ /
  Last update: 2017-08-04 16:24    [W:0.095 / U:6.980 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site