Re: [PATCH perf/core v5 05/15] perf probe: Use cache entry if possible

Masami Hiramatsu
2016-04-28T23:52:36Z

On Thu, 28 Apr 2016 11:47:53 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> On Thu, Apr 28, 2016 at 03:37:52AM +0900, Masami Hiramatsu wrote:
> > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >
> > Before analyzing debuginfo, try to find a corresponding entry
> > from probe cache always. This does not depend on --cache,
> > the --cache enables to store/update cache, but looking up
> > the cache is always enabled.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
>
> [SNIP]
> > +static int find_probe_trace_events_from_cache(struct perf_probe_event *pev,
> > + struct probe_trace_event **tevs)
> > +{
> > + struct probe_cache *cache;
> > + struct probe_cache_entry *entry;
> > + struct probe_trace_event *tev;
> > + struct str_node *node;
> > + int ret, i;
> > +
> > + cache = probe_cache__new(pev->target);
> > + if (!cache)
> > + return 0;
> > +
> > + entry = probe_cache__find(cache, pev);
> > +
> > + if (!entry && !pev->event &&
> > + (!pev->point.file && pev->point.function && !pev->point.retprobe &&
> > + !pev->point.line && !pev->point.offset && !pev->point.lazy_line)) {
> > + entry = probe_cache__find_by_name(cache, pev->group,
> > + pev->point.function);
> > + }
>
> What about arguments? It seems that it only checks group and event
> names of the given pev and use the cached entry's tev if found. What
> if they have different argument spec?

Indeed. I think this is for reusing cached event by using event name.
however, with 11/15 ("perf probe: Accept %sdt and %cached event name"),
we don't need this anymore.

So, I'll remove this fallback code.

Thank you!

>
> Thanks,
> Namhyung
>
>
> > + if (!entry) {
> > + ret = 0;
> > + goto out;
> > + }
> > +
> > + ret = strlist__nr_entries(entry->tevlist);
> > + if (ret > probe_conf.max_probes) {
> > + pr_debug("Too many entries matched in the cache of %s\n",
> > + pev->target ? : "kernel");
> > + ret = -E2BIG;
> > + goto out;
> > + }
> > +
> > + *tevs = zalloc(ret * sizeof(*tev));
> > + if (!*tevs) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + i = 0;
> > + strlist__for_each(node, entry->tevlist) {
> > + tev = &(*tevs)[i++];
> > + ret = parse_probe_trace_command(node->s, tev);
> > + if (ret < 0)
> > + goto out;
> > + /* Set the uprobes attribute as same as original */
> > + tev->uprobes = pev->uprobes;
> > + }
> > + ret = i;
> > +
> > +out:
> > + probe_cache__delete(cache);
> > + return ret;
> > +}

--
Masami Hiramatsu <mhiramat@kernel.org>