From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20620 invoked by alias); 22 Jul 2014 11:54:16 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 20607 invoked by uid 89); 22 Jul 2014 11:54:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e28smtp09.in.ibm.com Received: from e28smtp09.in.ibm.com (HELO e28smtp09.in.ibm.com) (122.248.162.9) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 22 Jul 2014 11:54:15 +0000 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 22 Jul 2014 17:24:09 +0530 Received: from d28dlp03.in.ibm.com (9.184.220.128) by e28smtp09.in.ibm.com (192.168.1.139) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 22 Jul 2014 17:24:08 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 18BAC1258048 for ; Tue, 22 Jul 2014 17:23:57 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s6MBtgY117170486 for ; Tue, 22 Jul 2014 17:25:43 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s6MBrpwo014322 for ; Tue, 22 Jul 2014 17:23:52 +0530 Received: from localhost.localdomain ([9.124.35.114]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s6MBrpE9014315; Tue, 22 Jul 2014 17:23:51 +0530 Message-ID: <53CE50CF.3080200@linux.vnet.ibm.com> Date: Tue, 22 Jul 2014 11:54:00 -0000 From: Hemant Kumar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Andi Kleen CC: linux-kernel@vger.kernel.org, srikar@linux.vnet.ibm.com, peterz@infradead.org, oleg@redhat.com, hegdevasant@linux.vnet.ibm.com, mingo@redhat.com, anton@redhat.com, systemtap@sourceware.org, namhyung@kernel.org, masami.hiramatsu.pt@hitachi.com, aravinda@linux.vnet.ibm.com, penberg@iki.fi Subject: Re: [PATCH v2 1/3] perf/sdt : Listing of SDT markers by perf References: <20140717054826.19995.61782.stgit@hemant-fedora> <20140717055341.19995.97042.stgit@hemant-fedora> <87d2d24kui.fsf@tassilo.jf.intel.com> In-Reply-To: <87d2d24kui.fsf@tassilo.jf.intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14072211-2674-0000-0000-0000010A5491 X-SW-Source: 2014-q3/txt/msg00068.txt.bz2 Hi Andi, On 07/18/2014 11:20 PM, Andi Kleen wrote: > Hemant Kumar writes: > > First I should say supporting these probes is very useful. Thanks for > working on this. Thanks a lot for the appreciation. > >> + >> +#define SDT_CACHE_DIR "/var/cache/perf/" > This requires running perf as root, right? Yes! > > It would be better to use the $HOME cache dir, like the recent JSON patches. Hmm, seems like a good idea! We can use ~/.debug as Masami suggested. > >> +#define SDT_CACHE "perf-sdt.cache" >> +#define SDT_CACHE_TMP "perf-sdt.cache.tmp" >> + >> +#define DELIM ':' >> + >> +struct path_list { >> + char path[PATH_MAX]; >> + struct list_head list; >> +} execs; >> + >> +/* Write operation for cache */ >> +static void write_cache(FILE *cache, char *buffer) >> +{ >> + fprintf(cache, "%s", buffer); >> +} >> + > The function seems redundant. > Yeah, right. Will remove it. >> +/* >> + * get_sdt_note_info() is the function actually responsible for >> + * flushing the SDT notes info into the "cache" file or to the >> + * stdout if "cache" points to NULL. Also, this function finds out >> + * the build-id of an ELF to be written into "cache". >> + */ >> +static void get_sdt_note_info(struct list_head *start, const char *target, >> + FILE *cache) >> +{ >> + struct sdt_note *pos; >> + u8 build_id[BUILD_ID_SIZE]; >> + char sbuild_id[BUILD_ID_SIZE * 2 + 1]; >> + char buffer[2 * PATH_MAX]; >> + >> + if (list_empty(start)) >> + return; >> + >> + /* Read the build id of the file */ >> + if (filename__read_build_id(target, &build_id, >> + sizeof(build_id)) < 0) { >> + pr_debug("Couldn't read build-id in %s\n", target); > pr_info ? Ok, pr_info will be better. >> + >> +/* >> + * Finds out the libraries present in a system as shown by the command >> + * "ldconfig --print-cache". Uses "=>" and '/' to find out the start of a >> + * dso path. >> + */ > This seems like a hack. How would that handle chroot, containers etc. ? Right now, it doesn't handle chroot, containers and other path related stuff. We will have to figure that out! >> +static inline void append_path(char *path, struct list_head *list) >> +{ >> + char *res_path = NULL; >> + struct path_list *tmp = NULL; >> + >> + res_path = (char *)malloc(sizeof(char) * PATH_MAX); >> + >> + if (!res_path) >> + return; >> + >> + memset(res_path, '\0', PATH_MAX); >> + >> + if (realpath(path, res_path) && !is_present_in_list(list, res_path)) { > > O^2 algorithm ? :) Will improve that by using a hash table. >> +/* >> + * Obtain the list of paths from the PATH env variable >> + */ > Same as above. This probably needs to be more configurable to handle > more ways to find binaries. > We can make it more configurable, but by default, it should go for binaries in these directories. What would you suggest? Thanks a lot for reviewing the patch. :) -- Thanks, Hemant Kumar