Re: [PATCH] perf tools: Do not fail on processing out of order event

[Jiri Olsa <jolsa@redhat.com>]

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