From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752865AbbC0UJY (ORCPT ); Fri, 27 Mar 2015 16:09:24 -0400 Received: from mail.kernel.org ([198.145.29.136]:53754 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751471AbbC0UJV (ORCPT ); Fri, 27 Mar 2015 16:09:21 -0400 Date: Fri, 27 Mar 2015 17:09:22 -0300 From: Arnaldo Carvalho de Melo To: Don Zickus Cc: David Ahern , linux-kernel@vger.kernel.org, Joe Mario , Jiri Olsa Subject: Re: [PATCH v2] perf tool: Fix ppid for synthesized fork events Message-ID: <20150327200922.GL21510@kernel.org> References: <1427302270-10178-1-git-send-email-dsahern@gmail.com> <20150325191526.GX162412@redhat.com> <551312C0.4060706@gmail.com> <20150326211146.GZ162412@redhat.com> <55147C19.5090302@gmail.com> <20150327131005.GA162412@redhat.com> <55156330.9080607@gmail.com> <20150327142036.GI21510@kernel.org> <20150327194941.GG162412@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150327194941.GG162412@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 Fri, Mar 27, 2015 at 03:49:41PM -0400, Don Zickus escreveu: > On Fri, Mar 27, 2015 at 11:20:36AM -0300, Arnaldo Carvalho de Melo wrote: > > ... which is what David is suggesting here: > > > > > Try this: > > > perf record -o unpatched.data -g -- perf.unpatched mem record -a -e > > > cpu/mem-loads,ldlat=50/pp -e cpu/mem-stores/pp sleep 10 > > > > > > perf record -o patched.data -g -- perf.patched mem record -a -e > > > cpu/mem-loads,ldlat=50/pp -e cpu/mem-stores/pp sleep 10 > > > > > > And then compare the reports for each. > > > > Cache effects, i.e. OS FS caches for the files accessed when doing the > > build id table could be responsible for part of the difference at some > > point, but further investigation by using 'perf record' > > patched/unpatched will give us more clues. > > Alright, Joe and I poked some more and as I thought, David's patch does > something subtle which may have inadvertently undid my original patch. > Though the threading model isn't clear in my head right now. > > Here is the patch I added to test a theory: > > diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c > index 1c8fbc9..7ee3823 100644 > --- a/tools/perf/util/thread.c > +++ b/tools/perf/util/thread.c > @@ -187,6 +187,7 @@ static int thread__clone_map_groups(struct thread *thread, > if (thread->pid_ == parent->pid_) > return 0; > > + printf("DON:\n"); > /* But this one is new process, copy maps. */ > for (i = 0; i < MAP__NR_TYPES; ++i) > if (map_groups__clone(thread->mg, parent->mg, i) < 0) > > before David's patch, we do _not_ see any DON markers. After David's patch > we see a 1:1 match of DON markers to the number of threads currently running > in the system. > > As a result the perf record -g command David recommended showed a spike in > rb_next and map_groups__clone which is the result of the above discovery. > > > So the next question is, is this correct? On the surface I would say no > because it doesn't seem like we are not being smart any more and taking > advantage of the existing thread maps created. But I guess the idea behind > cloning is that we are. > > I can't think right now what is the correct behaviour, thoughts? ok, so if in perf_event__synthesize_fork we correctly set up event->fork.ppid, when we call process() there it will end up calling: perf_event__process_fork() machine__process_fork_event() And that will call: struct thread *thread = machine__find_thread(machine, event->fork.pid, event->fork.tid); struct thread *parent = machine__findnew_thread(machine, event->fork.ppid, event->fork.ptid); Without David's patch the second will pass -1 to both ppid and ptid, right? That will find a fake thread with ppid == -1 and ptid == -1, that has no mmaps. Next thing perf_event__synthesize_fork() will do is to call: thread__fork(thread, parent, sample->time) And that is what will end up calling: thread__clone_map_groups(thread, parent) map_groups__clone(thread->mg, parent->mg, i) So, assuming the parent was synthesized first and got its mmaps, then when the child is processed it will find and will clone its mmaps. - Arnaldo