From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752349AbbETBF5 (ORCPT ); Tue, 19 May 2015 21:05:57 -0400 Received: from lgeamrelo01.lge.com ([156.147.1.125]:52299 "EHLO lgeamrelo01.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751528AbbETBF4 (ORCPT ); Tue, 19 May 2015 21:05:56 -0400 X-Original-SENDERIP: 10.177.220.203 X-Original-MAILFROM: namhyung@kernel.org Date: Wed, 20 May 2015 09:56:25 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Jiri Olsa , Ingo Molnar , Peter Zijlstra , LKML , David Ahern , Adrian Hunter , Andi Kleen , Frederic Weisbecker , Stephane Eranian Subject: Re: [PATCH 35/40] perf record: Synthesize COMM event for a command line workload Message-ID: <20150520005625.GG22713@sejong> References: <1431909055-21442-1-git-send-email-namhyung@kernel.org> <1431909055-21442-36-git-send-email-namhyung@kernel.org> <20150518124535.GD15972@kernel.org> <20150519074643.GM21663@sejong> <20150519140220.GH13946@kernel.org> <20150519151245.GC29162@danjae.kornet> <20150519191826.GA26714@krava.redhat.com> <20150519201854.GD19921@kernel.org> <20150519235658.GA22713@sejong> <20150520002210.GD26111@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150520002210.GD26111@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 Tue, May 19, 2015 at 09:22:10PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, May 20, 2015 at 08:56:58AM +0900, Namhyung Kim escreveu: > > On Tue, May 19, 2015 at 05:18:54PM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Tue, May 19, 2015 at 09:49:03PM +0200, Jiri Olsa escreveu: > > > > > > Humm, you're thinking about where you managed to reproduce the problem, > > > > > > I am thinking outside indexing, etc, i.e. by definition we either enable > > > > > > the event before we fork, so that we get the PERF_RECORD_FORK/COMM or we > > > > > > synthesize it either from /proc or directly (preferred) if we decide to > > > > > > do it after the fork/exec, right? > > > > > > But as I said before, later COMM event will override thread->comm to a > > > > > proper string as long as it can find a matching thread. So I think it > > > > > has no problem in the current code. > > > > > I can see the issue in the current script code and the patch cured it ;-) > > > > Exactly, this is my point, this is not something new :-) > > > Ah, okay. The perf script shows samples before processing comm events > > while perf report shows after processing all events. > > I.e. 'perf script' behaves like 'perf trace' and 'perf top'. 'perf > report' is the odd one out, and I think it should be not, i.e. you > should try to think more about the non 'report' use cases when thinking > about how to improve report :-) I'll keep it in mind. > > But I digress, lets get back to the question at hand... > > > But to move it under generic place like perf_evlist__{prepare,start}_ > > workload(), it seems we need to pass an additional callback and data. > > Only if you want to do it with perf_event__synthesize_comm(). I > suggested writing a new synthesize routine that doesn't parses /proc, as > we have all that we need, no? Agreed. > > I think that just doing something like: > > thread = machine__findnew_thread(evlist->workload.pid, evlist->workload.pid); > if (thread) > thread__set_comm(thread, argv[0], timestamp); > > Should be enough, no? I.e. no need for setting up a PERF_RECORD_FORK and > a PERF_RECORD_COMM, read /proc, etc, just do it directly with the info > we used to do the fork in perf_evlist__prepare_workload(), etc. For non-record use case it'd be enough. But for record, it needs to synthesize/write an event to data file so that perf report can recognize it later. That's why I think it needs callback. Thanks, Namhyung