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.
>
>
