From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752558AbbFYQBp (ORCPT ); Thu, 25 Jun 2015 12:01:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35242 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751862AbbFYQBg (ORCPT ); Thu, 25 Jun 2015 12:01:36 -0400 Date: Thu, 25 Jun 2015 18:01:25 +0200 From: Jiri Olsa To: Arnaldo Carvalho de Melo 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: <20150625160125.GA17260@krava.redhat.com> 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> <20150625143927.GD3253@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150625143927.GD3253@kernel.org> 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 On Thu, Jun 25, 2015 at 11:39:27AM -0300, Arnaldo Carvalho de Melo wrote: > 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? right ;-) also the 'comm' string is small update after long time of no change in thread_map and I guess it will take probably another long time period before we will want to change it again.. thanks, jirka