From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752453AbbFYOjp (ORCPT ); Thu, 25 Jun 2015 10:39:45 -0400 Received: from mail.kernel.org ([198.145.29.136]:60951 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752293AbbFYOjc (ORCPT ); Thu, 25 Jun 2015 10:39:32 -0400 Date: Thu, 25 Jun 2015 11:39:27 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: Jiri Olsa , lkml , Adrian Hunter , Andi Kleen , David Ahern , Ingo Molnar , Namhyung Kim , Peter Zijlstra , Stephane Eranian Subject: Re: [PATCH 02/27] perf tools: Add comm string into struct thread_map Message-ID: <20150625143927.GD3253@kernel.org> References: <1435012588-9007-1-git-send-email-jolsa@kernel.org> <1435012588-9007-3-git-send-email-jolsa@kernel.org> <20150623210158.GA11231@kernel.org> <20150623215617.GB24969@krava.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150623215617.GB24969@krava.brq.redhat.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Jun 23, 2015 at 11:56:18PM +0200, Jiri Olsa escreveu: > On Tue, Jun 23, 2015 at 06:01:58PM -0300, Arnaldo Carvalho de Melo wrote: > > Em Tue, Jun 23, 2015 at 12:36:03AM +0200, Jiri Olsa escreveu: > > > Adding support to hold comm name together with pids in > > > 'struct thread_map'. It will be useful for --per-thread > > > option to display task pid together with task name. > > > Getting the task name from /proc/$pid/comm. > > Do we really need to read this all the time? Or just for tools that need > > this? > > Can't we have a thread_map__read_comms(map); > > That we can make this specific --per-thread option in 'stat' call? > ook Thanks, this made me think: thread_map is just an array that enumerates the pids ultimately for use with sys_perf_event_open(), bloating it with extra details used by just one oddball subfeature of just one tool seems overkill. Probably it would be better to rename it to pid_map, that is what is useful for all tools using it to create the event file descriptors and have a comm_map that would return another enumeration to obtain the comms, something like: struct thread_map *tids = thread__map_by_foo(parameters); struct comm_map *comms = comm_map__new(tids); That way the various ways of populating the thread_map->map[] (the _by_foo() variants of thread_map we have) would do that leg work and the comm_map__new(tids) would just read the comms. The thread_map users would not have to be touched at all, it would be kept as simple as it was and just tools that need those comms would go to the trouble of allocating the needed space for the array and to do the comm reading. But nah, I'll probably merge what you did and later, if I find the time, do it myself, better to have that new feature than to be picky, right? Thanks, - Arnaldo