Re: perf tools: interface for improved PEBS ABI can accept wrong parameter

[Arnaldo Carvalho de Melo <acme@redhat.com>]

Em Wed, Oct 19, 2011 at 10:23:12AM +0800, Xu, Anhua escreveu:
> Thanks Peter:). The key point for this is that the detailed information about "PRECISE_IP" may not be exposed to user space.
> Error reporting may come from syscall. Anyway, expect acme's suggestions.


Well, at least we can do as you did and avoid requests completely
invalid per the ABI or look the other way, truncate and silently provide
different behaviour than requested.

Point is how to propagate back from parse_event_modifier so that we
provide a sensible error message.

One could try to reuse errno and find things like -EINVAL, ELEVEL or
define some enum.
 
> 
> -----Original Message-----
> From: Peter Zijlstra [mailto:a.p.zijlstra@chello.nl] 
> Sent: Wednesday, October 19, 2011 2:28 AM
> To: Ingo Molnar
> Cc: Xu, Anhua; Arnaldo Carvalho de Melo; linux-kernel@vger.kernel.org
> Subject: Re: perf tools: interface for improved PEBS ABI can accept wrong parameter
> 
> On Tue, 2011-10-18 at 20:19 +0200, Ingo Molnar wrote:
> > > +#define SAMPLE_IP_MAX        3 
> > >                               precise_ip     :  2, /* skid constraint       */
> > >                               mmap_data      :  1, /* non-exec mmap data    */
> > >                               sample_id_all  :  1, /* sample_type all events */
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index 928918b..a2068cc 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -775,7 +775,10 @@ parse_event_modifier(const char **strp, struct perf_event_attr *attr)
> > >       attr->exclude_user   = eu;
> > >       attr->exclude_kernel = ek;
> > >       attr->exclude_hv     = eh;
> > > -     attr->precise_ip     = precise;
> > > +     if ( precise < SAMPLE_IP_MAX )
> > > +             attr->precise_ip = precise;
> > > +     else
> > > +             return -1; 
> 
> That name is horrid, how about PRECISE_IP_MAX? Also, I suspect acme will
> want a better error return than -1, but I'll leave that up to him.