Messages in this thread | | | Subject | Re: [PATCH v6 2/3]: perf/core: use context tstamp_data for skipped events on mux interrupt | From | Alexey Budankov <> | Date | Fri, 4 Aug 2017 17:25:55 +0300 |
| |
On 04.08.2017 15:51, Peter Zijlstra wrote: > On Fri, Aug 04, 2017 at 02:35:34PM +0200, Peter Zijlstra wrote: >> 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; > Obv should go above the tstamp assignment > >> 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; >> } >> } > > So that's a straight fwd state machine that deals with: > > OFF <-> INACTIVE <-> ACTIVE > > but I think something like: > > __update_state_and_time(event, new_state) > { > u64 delta, new = perf_event_time(event); > int old_state = event->state; > > delta = now - event->tstamp; > event->tstamp = now; > event->state = new_state; > > if (old_state == STATE_OFF) > return; > > event->total_time_enabled += delta; > > if (old_state == STATE_ACTIVE) > event->total_time_running += delta; > } > > is equivalent and generates smaller code.. but again, double check (also > it doesn't validate the state transitions).
Accepted.
>
| |