Messages in this thread Patch in this message | | | From | "Xu, Anhua" <> | Date | Mon, 24 Oct 2011 01:07:02 +0800 | Subject | RE: perf tools: interface for improved PEBS ABI can accept wrong parameter |
| |
I have update my patch. Hope it is not so horrid :)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index c816075..837f416 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -238,6 +238,17 @@ struct perf_event_attr { }; }; +enum perf_sample_ip_constraints { + PERF_SAMPLE_IP_ARBITRARY_SKID, + PERF_SAMPLE_IP_CONSTANT_SKID, + PERF_SAMPLE_IP_REQUEST_NOSKID, + + /* + * The PEBS implementation now supports up to 2. + */ + PERF_SMAPLE_IP_CONSTRAINT_MAX, +}; + /* * Ioctls that can be done on a perf event fd: */ diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 928918b..55567ec 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 < PERF_SMAPLE_IP_CONSTRAINT_MAX ) + attr->precise_ip = precise; + else + return -EOPNOTSUPP; return 0; } -----Original Message----- From: Arnaldo Carvalho de Melo [mailto:acme@redhat.com] Sent: Wednesday, October 19, 2011 10:15 PM To: Xu, Anhua Cc: Peter Zijlstra; Ingo Molnar; linux-kernel@vger.kernel.org Subject: Re: perf tools: interface for improved PEBS ABI can accept wrong parameter 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.
| |