| Message ID | de6c0cd8236de1cabc179e456514c2a399cbc7b5.1418146300.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Michael Ellerman |
| Headers | show |
On Tue, 2014-12-09 at 23:04 +0530, Naveen N. Rao wrote: > Currently, perf probe considers patterns including a '.' to be a file. > However, this causes problems on powerpc ABIv1 where all functions have > a leading '.': > > $ perf probe -F | grep schedule_timeout_interruptible > .schedule_timeout_interruptible > $ perf probe .schedule_timeout_interruptible > Semantic error :File always requires line number or lazy pattern. > Error: Command Parse Error. > > Fix this by checking the probe pattern in more detail. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > tools/perf/util/probe-event.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index c150ca4..c7e01ef 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > arg = tmp; > } > > + /* > + * Check arg is function or file name and copy it. > + * > + * We consider arg to be a file spec if and only if it satisfies > + * all of the below criteria:: > + * - it does not include any of "+@%", > + * - it includes one of ":;", and > + * - it has a period '.' in the name. I don't think we need to be this elaborate. AFAIK there are no source files in the kernel that start with '.' So if the arg starts with '.' it must be a function? cheers
On 2014/12/10 09:00PM, Michael Ellerman wrote: > On Tue, 2014-12-09 at 23:04 +0530, Naveen N. Rao wrote: > > Currently, perf probe considers patterns including a '.' to be a file. > > However, this causes problems on powerpc ABIv1 where all functions have > > a leading '.': > > > > $ perf probe -F | grep schedule_timeout_interruptible > > .schedule_timeout_interruptible > > $ perf probe .schedule_timeout_interruptible > > Semantic error :File always requires line number or lazy pattern. > > Error: Command Parse Error. > > > > Fix this by checking the probe pattern in more detail. > > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > --- > > tools/perf/util/probe-event.c | 23 ++++++++++++++++++++--- > > 1 file changed, 20 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > > index c150ca4..c7e01ef 100644 > > --- a/tools/perf/util/probe-event.c > > +++ b/tools/perf/util/probe-event.c > > @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > > arg = tmp; > > } > > > > + /* > > + * Check arg is function or file name and copy it. > > + * > > + * We consider arg to be a file spec if and only if it satisfies > > + * all of the below criteria:: > > + * - it does not include any of "+@%", > > + * - it includes one of ":;", and > > + * - it has a period '.' in the name. > > I don't think we need to be this elaborate. > > AFAIK there are no source files in the kernel that start with '.' > > So if the arg starts with '.' it must be a function? Indeed, but this is also used for parsing uprobes. So, I coded this based on the spec for the probe pattern. - Naveen
On Wed, 2014-12-10 at 16:29 +0530, Naveen N. Rao wrote: > On 2014/12/10 09:00PM, Michael Ellerman wrote: > > On Tue, 2014-12-09 at 23:04 +0530, Naveen N. Rao wrote: > > > Currently, perf probe considers patterns including a '.' to be a file. > > > However, this causes problems on powerpc ABIv1 where all functions have > > > a leading '.': > > > > > > $ perf probe -F | grep schedule_timeout_interruptible > > > .schedule_timeout_interruptible > > > $ perf probe .schedule_timeout_interruptible > > > Semantic error :File always requires line number or lazy pattern. > > > Error: Command Parse Error. > > > > > > Fix this by checking the probe pattern in more detail. > > > > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > > --- > > > tools/perf/util/probe-event.c | 23 ++++++++++++++++++++--- > > > 1 file changed, 20 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > > > index c150ca4..c7e01ef 100644 > > > --- a/tools/perf/util/probe-event.c > > > +++ b/tools/perf/util/probe-event.c > > > @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > > > arg = tmp; > > > } > > > > > > + /* > > > + * Check arg is function or file name and copy it. > > > + * > > > + * We consider arg to be a file spec if and only if it satisfies > > > + * all of the below criteria:: > > > + * - it does not include any of "+@%", > > > + * - it includes one of ":;", and > > > + * - it has a period '.' in the name. > > > > I don't think we need to be this elaborate. > > > > AFAIK there are no source files in the kernel that start with '.' > > > > So if the arg starts with '.' it must be a function? > > Indeed, but this is also used for parsing uprobes. So, I coded this > based on the spec for the probe pattern. OK. It also seems unlikely you'll want a uprobe on a file that starts with a . ? I'll leave it up to the perf guys to decide if they're happy with it. cheers
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index c150ca4..c7e01ef 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) arg = tmp; } + /* + * Check arg is function or file name and copy it. + * + * We consider arg to be a file spec if and only if it satisfies + * all of the below criteria:: + * - it does not include any of "+@%", + * - it includes one of ":;", and + * - it has a period '.' in the name. + * + * Otherwise, we consider arg to be a function specification. + */ + c = 0; + if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) { + /* This is a file spec if it includes a '.' before ; or : */ + if (memchr(arg, '.', ptr-arg)) + c = 1; + } + ptr = strpbrk(arg, ";:+@%"); if (ptr) { nc = *ptr; @@ -1009,10 +1027,9 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) if (tmp == NULL) return -ENOMEM; - /* Check arg is function or file and copy it */ - if (strchr(tmp, '.')) /* File */ + if (c == 1) pp->file = tmp; - else /* Function */ + else pp->function = tmp; /* Parse other options */
Currently, perf probe considers patterns including a '.' to be a file. However, this causes problems on powerpc ABIv1 where all functions have a leading '.': $ perf probe -F | grep schedule_timeout_interruptible .schedule_timeout_interruptible $ perf probe .schedule_timeout_interruptible Semantic error :File always requires line number or lazy pattern. Error: Command Parse Error. Fix this by checking the probe pattern in more detail. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- tools/perf/util/probe-event.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)