Date: Thu, 28 Apr 2016 11:32:18 +0900
From: Namhyung Kim <namhyung@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
CC: Arnaldo Carvalho de Melo <acme@kernel.org>, <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
        Hemant Kumar <hemant@linux.vnet.ibm.com>,
        Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Subject: Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to
 cache the probe definitions
Message-ID: <20160428023218.GA15597@sejong.aot.lge.com>
References: <20160427183701.23446.15293.stgit@devbox>
 <20160427183741.23446.1938.stgit@devbox>
MIME-Version: 1.0
In-Reply-To: <20160427183741.23446.1938.stgit@devbox>
User-Agent: Mutt/1.6.0 (2016-04-01)
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3572
Lines: 132

On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote:
> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> 
> Add --cache option to cache the probe definitions. This
> just saves the result of the dwarf analysis to probe cache.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  Changes in v5:
>   - Move probe_cache* definitions. (code cleanup)
> 
>  Changes in v4:
>   - Remove cache saving failure message.
> ---

[SNIP]
> +/* For the kernel probe caches, pass target = NULL */
> +static int probe_cache__open(struct probe_cache *pcache, const char *target)
> +{
> +	char cpath[PATH_MAX];
> +	char sbuildid[SBUILD_ID_SIZE];
> +	char *dir_name;
> +	bool is_kallsyms = !target;
> +	int ret, fd;
> +
> +	if (target)
> +		ret = filename__sprintf_build_id(target, sbuildid);
> +	else {
> +		target = "[kernel.kallsyms]";
> +		ret = sysfs__sprintf_build_id("/", sbuildid);
> +	}
> +	if (ret < 0) {
> +		pr_debug("Failed to get build-id from %s.\n", target ?: "kernel");
> +		return ret;
> +	}
> +
> +	/* If we have no buildid cache, make it */
> +	if (!build_id_cache__cached(sbuildid)) {
> +		ret = build_id_cache__add_s(sbuildid, target,
> +					    is_kallsyms, NULL);
> +		if (ret < 0) {
> +			pr_debug("Failed to add build-id cache: %s\n", target);
> +			return ret;
> +		}
> +	}
> +
> +	dir_name = build_id_cache__dirname_from_path(sbuildid, target,
> +						     is_kallsyms, false);
> +	if (!dir_name)
> +		return -ENOMEM;
> +
> +	snprintf(cpath, PATH_MAX, "%s/probes", dir_name);
> +	fd = open(cpath, O_CREAT | O_RDWR | O_APPEND, 0644);
> +	if (fd < 0)
> +		pr_debug("Failed to open cache(%d): %s\n", fd, cpath);
> +	free(dir_name);
> +	pcache->fd = fd;
> +
> +	return fd;
> +}

[SNIP]
> +
> +int probe_cache__commit(struct probe_cache *pcache)
> +{
> +	struct probe_cache_entry *entry;
> +	int ret = 0;
> +
> +	/* TBD: if we do not update existing entries, skip it */
> +	ret = lseek(pcache->fd, 0, SEEK_SET);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = ftruncate(pcache->fd, 0);
> +	if (ret < 0)
> +		goto out;

What is this doing?  Does it truncate old contents on write?  Opening
with O_APPEND looks strange to me..

Thanks,
Namhyung


> +
> +	list_for_each_entry(entry, &pcache->list, list) {
> +		ret = probe_cache_entry__write(entry, pcache->fd);
> +		pr_debug("Cache committed: %d\n", ret);
> +		if (ret < 0)
> +			break;
> +	}
> +out:
> +	return ret;
> +}
> diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
> index 18ac9cf..d2b8791d 100644
> --- a/tools/perf/util/probe-file.h
> +++ b/tools/perf/util/probe-file.h
> @@ -5,6 +5,19 @@
>  #include "strfilter.h"
>  #include "probe-event.h"
>  
> +/* Cache of probe definitions */
> +struct probe_cache_entry {
> +	struct list_head	list;
> +	struct perf_probe_event pev;
> +	char			*spev;
> +	struct strlist		*tevlist;
> +};
> +
> +struct probe_cache {
> +	int	fd;
> +	struct list_head list;
> +};
> +
>  #define PF_FL_UPROBE	1
>  #define PF_FL_RW	2
>  
> @@ -18,5 +31,11 @@ int probe_file__get_events(int fd, struct strfilter *filter,
>  				  struct strlist *plist);
>  int probe_file__del_strlist(int fd, struct strlist *namelist);
>  
> +struct probe_cache *probe_cache__new(const char *target);
> +int probe_cache__add_entry(struct probe_cache *pcache,
> +			   struct perf_probe_event *pev,
> +			   struct probe_trace_event *tevs, int ntevs);
> +int probe_cache__commit(struct probe_cache *pcache);
> +void probe_cache__delete(struct probe_cache *pcache);
>  
>  #endif
>