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

[Masami Hiramatsu]

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

> On Thu, Apr 28, 2016 at 03:37:52AM +0900, Masami Hiramatsu wrote:
> > From: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > ---
> 
> [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@xxxxxxxxxx>