perf: perf interface for uprobes
in [Kernel]
|
Prev: [tip:x86/urgent] x86, Calgary: Limit the max PHB number to 256
Next: Rif: Re: [PATCH] spi: davinci: Added support for chip select using gpio
From: Masami Hiramatsu on 29 Jul 2010 08:10 Hi Srikar, Srikar Dronamraju wrote: > Enhances perf probe to accept pid and user vaddr. > Provides very basic support for uprobes. [...] > @@ -162,6 +190,14 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev, > bool need_dwarf = perf_probe_event_need_dwarf(pev); > int fd, ntevs; > > + if (pev->upid) { > + if (need_dwarf) { > + pr_warning("Debuginfo-analysis is not supported.\n"); This should be "Debuginfo-analysis is not supported with -p/--pid option.\n" > + return -ENOSYS; > + } > + return convert_name_to_addr(pev); > + } > + And I found a small bug. # ./perf probe -vf -p 3199 "setenv %ax" probe-definition(0): setenv %ax symbol:setenv file:(null) line:0 offset:0 return:0 lazy:(null) parsing arg: %ax into %ax 1 arguments Opening /debug/tracing/uprobe_events write=1 Add new event: Writing event: p:probe_3199/setenv 3199:0x47e680 %ax Failed to write event: Invalid argument Error: Failed to add events. (-1) # ./perf probe -l probe_3199:setenv (on 3199:0x000000000047e680) When writing an event, there is a "\n" right after probe point (3199:0x47e680\n) > @@ -1066,10 +1213,22 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev) > if (buf == NULL) > return NULL; > > - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s+%lu", > - tp->retprobe ? 'r' : 'p', > - tev->group, tev->event, > - tp->symbol, tp->offset); > + if (tev->upid) > + len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %d:%s\n", Here is a "\n". Thank you, -- Masami HIRAMATSU 2nd Research Dept. Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt(a)hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on 30 Jul 2010 15:20 Em Tue, Jul 27, 2010 at 04:41:05PM +0530, Srikar Dronamraju escreveu: > > Enhances perf probe to accept pid and user vaddr. > Provides very basic support for uprobes. > @@ -188,6 +190,8 @@ static const struct option options[] = { > OPT__DRY_RUN(&probe_event_dry_run), > OPT_INTEGER('\0', "max-probes", ¶ms.max_probe_points, > "Set how many probe points can be found for a probe."), > + OPT_INTEGER('p', "pid", ¶ms.upid, > + "specify a pid for a uprobes based probe"), "ubrobes based probe" could be made clear as: "Specify pid of process where the probe will be added" ? The following three hunks could be moved to a separate patch that I'd apply on my perf/core branch, so to reduce this patchset size, like I did with the s/kprobe/probe/g one that is already there: http://git.kernel.org/?p=linux/kernel/git/acme/linux-2.6.git;a=commitdiff;h=0e60836bbd392300198c5c2d918c18845428a1fe > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 1e8e92e..b513e40 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -1082,26 +1082,6 @@ static void event__process_sample(const event_t *self, > } > } > > -static int event__process(event_t *event, struct perf_session *session) > -{ > - switch (event->header.type) { > - case PERF_RECORD_COMM: > - event__process_comm(event, session); > - break; > - case PERF_RECORD_MMAP: > - event__process_mmap(event, session); > - break; > - case PERF_RECORD_FORK: > - case PERF_RECORD_EXIT: > - event__process_task(event, session); > - break; > - default: > - break; > - } > - > - return 0; > -} > - > struct mmap_data { > int counter; > void *base; > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c > index d7f21d7..d93e0bb 100644 > --- a/tools/perf/util/event.c > +++ b/tools/perf/util/event.c > @@ -552,6 +552,26 @@ int event__process_task(event_t *self, struct perf_session *session) > return 0; > } > > +int event__process(event_t *event, struct perf_session *session) > +{ > + switch (event->header.type) { > + case PERF_RECORD_COMM: > + event__process_comm(event, session); > + break; > + case PERF_RECORD_MMAP: > + event__process_mmap(event, session); > + break; > + case PERF_RECORD_FORK: > + case PERF_RECORD_EXIT: > + event__process_task(event, session); > + break; > + default: > + break; > + } > + > + return 0; > +} > + > void thread__find_addr_map(struct thread *self, > struct perf_session *session, u8 cpumode, > enum map_type type, pid_t pid, u64 addr, > diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h > index 887ee63..8e790da 100644 > --- a/tools/perf/util/event.h > +++ b/tools/perf/util/event.h > @@ -154,6 +154,7 @@ int event__process_comm(event_t *self, struct perf_session *session); > int event__process_lost(event_t *self, struct perf_session *session); > int event__process_mmap(event_t *self, struct perf_session *session); > int event__process_task(event_t *self, struct perf_session *session); > +int event__process(event_t *event, struct perf_session *session); > > struct addr_location; > int event__preprocess_sample(const event_t *self, struct perf_session *session, [...] > group = pev->group; > - else > + else if (!pev->upid) > group = PERFPROBE_GROUP; > + else { > + /* > + * For uprobes based probes create a group > + * probe_<pid>. > + */ > + snprintf(buf, 64, "%s_%d", PERFPROBE_GROUP, pev->upid); > + group = buf; > + } > + > + tev->group = strdup(group); Here you don't check as you do on the next strdups > + if (pev->event) > + event = pev->event; > + else if (pev->point.function) > + event = pev->point.function; > + else > + event = tev->point.symbol; > > /* Get an unused new event name */ > ret = get_new_event_name(buf, 64, event, > @@ -1471,9 +1681,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > if (ret < 0) > break; > event = buf; > - > tev->event = strdup(event); > - tev->group = strdup(group); > + here > if (tev->event == NULL || tev->group == NULL) { > ret = -ENOMEM; > break; > @@ -1571,6 +1780,11 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev, > } > } > > + if (pev->upid) { > + tev->upid = pev->upid; > + return 1; > + } > + > /* Currently just checking function name from symbol map */ > sym = map__find_symbol_by_name(machine.vmlinux_maps[MAP__FUNCTION], > tev->point.symbol, NULL); > @@ -1598,15 +1812,19 @@ struct __event_package { > int add_perf_probe_events(struct perf_probe_event *pevs, int npevs, > bool force_add, int max_tevs) > { > - int i, j, ret; > + int i, j, ret = 0; > struct __event_package *pkgs; > > pkgs = zalloc(sizeof(struct __event_package) * npevs); > if (pkgs == NULL) > return -ENOMEM; > > - /* Init vmlinux path */ > - ret = init_vmlinux(); > + if (!pevs->upid) > + /* Init vmlinux path */ > + ret = init_vmlinux(); > + else > + ret = init_perf_uprobes(); > + > if (ret < 0) pkgs leaks here. > return ret; > > @@ -1666,23 +1884,15 @@ error: > return ret; > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on 31 Jul 2010 15:40 Em Sat, Jul 31, 2010 at 08:27:48AM +0530, Srikar Dronamraju escreveu: > > > @@ -1598,15 +1812,19 @@ struct __event_package { > > > int add_perf_probe_events(struct perf_probe_event *pevs, int npevs, > > > bool force_add, int max_tevs) > > > { > > > - int i, j, ret; > > > + int i, j, ret = 0; > > > struct __event_package *pkgs; > > > > > > pkgs = zalloc(sizeof(struct __event_package) * npevs); > > > if (pkgs == NULL) > > > return -ENOMEM; > > > > > > - /* Init vmlinux path */ > > > - ret = init_vmlinux(); > > > + if (!pevs->upid) > > > + /* Init vmlinux path */ > > > + ret = init_vmlinux(); > > > + else > > > + ret = init_perf_uprobes(); > > > + > > > if (ret < 0) > > > > pkgs leaks here. > > Right, but I dont think this leak was introduced by my patch(s). I > guess its better fixed by a different patch. Sorry, will fix that then on a separate patch. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Masami Hiramatsu on 1 Aug 2010 22:00 Arnaldo Carvalho de Melo wrote: > Em Sat, Jul 31, 2010 at 08:27:48AM +0530, Srikar Dronamraju escreveu: >>>> @@ -1598,15 +1812,19 @@ struct __event_package { >>>> int add_perf_probe_events(struct perf_probe_event *pevs, int npevs, >>>> bool force_add, int max_tevs) >>>> { >>>> - int i, j, ret; >>>> + int i, j, ret = 0; >>>> struct __event_package *pkgs; >>>> >>>> pkgs = zalloc(sizeof(struct __event_package) * npevs); >>>> if (pkgs == NULL) >>>> return -ENOMEM; >>>> >>>> - /* Init vmlinux path */ >>>> - ret = init_vmlinux(); >>>> + if (!pevs->upid) >>>> + /* Init vmlinux path */ >>>> + ret = init_vmlinux(); >>>> + else >>>> + ret = init_perf_uprobes(); >>>> + >>>> if (ret < 0) >>> pkgs leaks here. >> Right, but I dont think this leak was introduced by my patch(s). I >> guess its better fixed by a different patch. > > Sorry, will fix that then on a separate patch. Yes, sorry, that was introduced by me... Thanks for pointing it out. -- Masami HIRAMATSU 2nd Research Dept. Hitachi, Ltd., Systems Development Laboratory E-mail: masami.hiramatsu.pt(a)hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on 2 Aug 2010 11:00
Em Mon, Aug 02, 2010 at 05:57:14PM +0530, Srikar Dronamraju escreveu: > Hi Arnaldo, > > > > > > The following three hunks could be moved to a separate patch that I'd > > apply on my perf/core branch, so to reduce this patchset size, like I > > did with the s/kprobe/probe/g one that is already there: > > > > http://git.kernel.org/?p=linux/kernel/git/acme/linux-2.6.git;a=commitdiff;h=0e60836bbd392300198c5c2d918c18845428a1fe > > I will soon 2 patches instead of this patch splitting the 3 hunks into > a new patch and the resuting patch after removing the previous 3 hunks. Thanks, I'm just trying to erode the patchset by merging the completely uncontentious hunks. :) - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |