linux-kernel - Re: [PATCH 2/2] perf tool: Fix ppid for synthesized fork events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150330152946.GE32560@kernel.org>
Date:	Mon, 30 Mar 2015 12:29:46 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	David Ahern <dsahern@...il.com>
Cc:	Jiri Olsa <jolsa@...hat.com>, linux-kernel@...r.kernel.org,
	Don Zickus <dzickus@...hat.com>, Joe Mario <jmario@...hat.com>
Subject: Re: [PATCH 2/2] perf tool: Fix ppid for synthesized fork events

Em Mon, Mar 30, 2015 at 09:23:21AM -0600, David Ahern escreveu:
> 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.

Yes, and COMM nowadays creates the thread (uses findnew_thread) where it
should really use just find_thread(), i.e. the fork event is the only
one that should create it, I guess it was like that because it made the
code a bit more robust, i.e. would handle COMMs without FORKs.

This one:

commit 4aa5f4f7bb8bc41cba15bcd0d80c4fb085027d6b
Author: Arnaldo Carvalho de Melo <acme@...hat.com>
Date:   Fri Feb 27 19:52:10 2015 -0300

    perf tools: Fix FORK after COMM when synthesizing records for pre-existing threads

Should have fixed a bit of this...

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/