Messages in this thread | | | Date | Thu, 27 Nov 2014 13:54:16 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH] perf tools: Do not fail on processing out of order event |
| |
On Thu, Nov 27, 2014 at 11:56:03AM +0100, Ingo Molnar wrote:
SNIP
> > > - pr_oe_time(timestamp, "out of order event"); > > > + pr_oe_time(timestamp, "out of order event\n"); > > > pr_oe_time(oe->last_flush, "last flush, last_flush_type %d\n", > > > oe->last_flush_type); > > > > > > - /* We could get out of order messages after forced flush. */ > > > - if (oe->last_flush_type != OE_FLUSH__HALF) > > > - return -EINVAL; > > > + s->stats.nr_unordered_events++; > > Btw., in the forced flush case we'll get out of order events that > are 'expected'. Shouldn't we count them separately and not warn > about them, or so?
hum, we warned about them anyway, we just did not fail processing.. and the impact of both cases should be the same.. it's just at the forced flush we expected/allowed out of order events
so I think it's ok to share the same counter and warn about them the same way
> > > > + if (session->stats.nr_unordered_events != 0) { > > > + ui__warning("%u out of order events recorded.\n", > > > + session->stats.nr_unordered_events); > > > + } > > Nit: I'd suggest keeping the message printout on a single line: > > if (session->stats.nr_unordered_events != 0) { > ui__warning("%u out of order events recorded.\n", session->stats.nr_unordered_events); > > as IMHO the cure for this col80 linebreak checkpatch warning is > worse than the disease! :-)
ok ;-)
> > Barring those details: > > Acked-by: Ingo Molnar <mingo@kernel.org>
thanks, jirka
| |