From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753550AbbC0UZa (ORCPT ); Fri, 27 Mar 2015 16:25:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40782 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752330AbbC0UZ0 (ORCPT ); Fri, 27 Mar 2015 16:25:26 -0400 Date: Fri, 27 Mar 2015 16:25:24 -0400 From: Don Zickus To: David Ahern Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Joe Mario , Jiri Olsa Subject: Re: [PATCH v2] perf tool: Fix ppid for synthesized fork events Message-ID: <20150327202524.GJ199787@redhat.com> 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> <5515B94E.6020706@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5515B94E.6020706@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 27, 2015 at 02:10:54PM -0600, David Ahern wrote: > On 3/27/15 1:49 PM, Don Zickus wrote: > >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; > > There's your answer ... the 2 lines above. > > > > >+ 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. > > Your "speed up" is based on the assumption that all synthesized > threads are their own parent which is wrong. ie., ppid != tgid of > the process. Actually, we originally intended the opposite I think. We were trying to get perf to avoid reading /proc//maps and do the slow and painful ascii to binary parsing by re-using a similar map (from a forked parent). Our speed up may have been implemented incorrectly and dumb luck got us a speed up, but we didn't mean to assume ppid != tgid, IIRC. :-) I think your's and acme's explaination makes sense. As a result, we are ok with this patch. You mentioned a v2, splitting up the patch. I will wait for that patch and ack it. Thanks! Cheers, Don > > Before ppid was getting initialized to -1. If you just make that > change to revert to the -1: > > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c > index d5efa5092ce6..ce4ca061c2e5 100644 > --- a/tools/perf/util/event.c > +++ b/tools/perf/util/event.c > @@ -150,8 +150,8 @@ static int perf_event__synthesize_fork(struct > perf_tool *tool, > { > memset(&event->fork, 0, sizeof(event->fork) + machine->id_hdr_size); > > - event->fork.ppid = tgid; > - event->fork.ptid = tgid; > + event->fork.ppid = -1; > + event->fork.ptid = -1; > event->fork.pid = tgid; > event->fork.tid = pid; > event->fork.header.type = PERF_RECORD_FORK; > > You see the "DON" messages. > > David