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]

Date:	Mon, 30 Mar 2015 09:23:21 -0600
From:	David Ahern <dsahern@...il.com>
To:	Jiri Olsa <jolsa@...hat.com>
CC:	acme@...nel.org, 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

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
--
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/