Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752647AbcD1XGY (ORCPT ); Thu, 28 Apr 2016 19:06:24 -0400 Received: from mail.kernel.org ([198.145.29.136]:37105 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752432AbcD1XGX (ORCPT ); Thu, 28 Apr 2016 19:06:23 -0400 Date: Fri, 29 Apr 2016 08:06:18 +0900 From: Masami Hiramatsu To: Namhyung Kim Cc: Arnaldo Carvalho de Melo , , Peter Zijlstra , Ingo Molnar , Hemant Kumar , Ananth N Mavinakayanahalli Subject: Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to cache the probe definitions Message-Id: <20160429080618.47b205a74458a8ae04905bdb@kernel.org> In-Reply-To: <20160428023218.GA15597@sejong.aot.lge.com> References: <20160427183701.23446.15293.stgit@devbox> <20160427183741.23446.1938.stgit@devbox> <20160428023218.GA15597@sejong.aot.lge.com> X-Mailer: Sylpheed 3.4.3 (GTK+ 2.24.28; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2667 Lines: 92 On Thu, 28 Apr 2016 11:32:18 +0900 Namhyung Kim wrote: > On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote: > > From: Masami Hiramatsu > > > > 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 > > Signed-off-by: Masami Hiramatsu > > --- > > 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.. Ah, right. I forget the reason why :(, but I guess it maybe because just changing the design while coding. I'll remove O_APPEND flag then. Thank you! -- Masami Hiramatsu