Prev: xstat: Add a pair of system calls to make extended file stats available [ver #4]
Next: [PATCH] Fixed division by zero bug in kernel/padata.c
From: Srikar Dronamraju on 30 Jul 2010 23:10 > > "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 -- 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: Srikar Dronamraju on 2 Aug 2010 08:30
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. -Srikar -- 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/ |