From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753123AbbC3PXa (ORCPT ); Mon, 30 Mar 2015 11:23:30 -0400 Received: from mail-ie0-f172.google.com ([209.85.223.172]:34761 "EHLO mail-ie0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752853AbbC3PX2 (ORCPT ); Mon, 30 Mar 2015 11:23:28 -0400 Message-ID: <55196A69.4030107@gmail.com> Date: Mon, 30 Mar 2015 09:23:21 -0600 From: David Ahern User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Jiri Olsa CC: acme@kernel.org, linux-kernel@vger.kernel.org, Don Zickus , Joe Mario Subject: Re: [PATCH 2/2] perf tool: Fix ppid for synthesized fork events References: <1427668202-15422-1-git-send-email-dsahern@gmail.com> <1427668202-15422-2-git-send-email-dsahern@gmail.com> <20150330080429.GA8624@krava.brq.redhat.com> In-Reply-To: <20150330080429.GA8624@krava.brq.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 3/30/15 2:04 AM, Jiri Olsa wrote: > On Sun, Mar 29, 2015 at 04:30:02PM -0600, David Ahern wrote: > > SNIP > >> -static pid_t perf_event__synthesize_comm(struct perf_tool *tool, >> - union perf_event *event, pid_t pid, >> - perf_event__handler_t process, >> - struct machine *machine) >> +static int perf_event__synthesize_comm(struct perf_tool *tool, >> + union perf_event *event, pid_t pid, >> + perf_event__handler_t process, >> + struct machine *machine, >> + pid_t *tgid, pid_t *ppid) >> { >> - pid_t tgid = perf_event__prepare_comm(event, pid, machine); >> - >> - if (tgid == -1) >> - goto out; >> + if (perf_event__prepare_comm(event, pid, machine, tgid, ppid) != 0) >> + return -1; > > why dont we set ppid also for single comm event? seems to me > like following assignments: > > event->fork.ppid = ppid; > event->fork.ptid = ppid; > event->fork.pid = tgid; > event->fork.tid = pid; > event->fork.header.type = PERF_RECORD_FORK; > > should be part of perf_event__prepare_comm function..? comm events versus fork events. Only fork has the ppid. That loop that generates COMM and FORK events needs some work. e.g., FORK should come before COMM. But that is not the point of this patch which is to fix ppid. > > SNIP > >> @@ -365,14 +387,12 @@ static int __event__synthesize_thread(union perf_event *comm_event, >> char filename[PATH_MAX]; >> DIR *tasks; >> struct dirent dirent, *next; >> - pid_t tgid; >> + pid_t tgid, ppid; >> >> /* special case: only send one comm event using passed in pid */ >> if (!full) { >> - tgid = perf_event__synthesize_comm(tool, comm_event, pid, >> - process, machine); >> - >> - if (tgid == -1) >> + if (perf_event__synthesize_comm(tool, comm_event, pid, >> + process, machine, &tgid, &ppid) != 0) > > why did you add *ppid arg into perf_event__synthesize_comm? > I see no use for it in the rest of the caller.. Because of the twisted state that code is in. Yes, I can remove that one. David