Messages in this thread | | | Date | Sat, 31 Jul 2010 08:27:48 +0530 | From | Srikar Dronamraju <> | Subject | Re: [PATCHv10 2.6.35-rc6-tip 11/14] perf: perf interface for uprobes |
| |
> > "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"
Okay . will do.
> > ? > > 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 > [snipped]
Okay, I can make a separate patch of those three hunks and send it out.
> > > 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
Actually tev->group gets checked where tev->event gets checked. However I had moved assigning tev->group to where group gets assigned. This probably is causing the confusion. I will move the assignment to where tev->event gets assigned.
> > break; > > event = buf; > > - > > tev->event = strdup(event); > > - tev->group = strdup(group); > > + > > here
This strdup(group) was moved ahead by few lines. I can move it back here it makes things more clear.
> > > 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.
Right, but I dont think this leak was introduced by my patch(s). I guess its better fixed by a different patch.
>
-- Thanks and Regards Srikar
| |