linux-kernel - Re: [PATCH V2 15/15] perf tools: fix ppid in thread_

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:	Fri, 28 Jun 2013 09:47:35 +0300
From:	Adrian Hunter <adrian.hunter@...el.com>
To:	David Ahern <dsahern@...il.com>
CC:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	linux-kernel@...r.kernel.org,
	Frederic Weisbecker <fweisbec@...il.com>,
	Jiri Olsa <jolsa@...hat.com>, Mike Galbraith <efault@....de>,
	Namhyung Kim <namhyung@...il.com>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH V2 15/15] perf tools: fix ppid in thread__fork()

On 27/06/13 19:57, David Ahern wrote:
> On 6/27/13 1:55 AM, Adrian Hunter wrote:
>> ppid should be assigned to the parents pid.  Note
>> 'thread__fork()'s only caller 'machine__process_fork_event()'
>> ensures that the parents pid is set.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
>> ---
>>   tools/perf/util/thread.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
>> index e3d4a55..93f3eab 100644
>> --- a/tools/perf/util/thread.c
>> +++ b/tools/perf/util/thread.c
>> @@ -85,7 +85,7 @@ int thread__fork(struct thread *self, struct thread
>> *parent)
>>           if (map_groups__clone(&self->mg, &parent->mg, i) < 0)
>>               return -ENOMEM;
>>
>> -    self->ppid = parent->tid;
>> +    self->ppid = parent->pid_;
>>
> 
> knowledge of the specific thread within the parent that created the process
> can be valuable -- that's what we get now. This change drops that
> information. I'd prefer this stays at thread id -- or save the parent thread
> id in addition to the pid.

That means renaming ppid to ptid.  Do you want to do that?

Isn't it possible that the parent could exit and the pid or tid be re-used
for another process?  In that case, to reliable identify the parent a
pointer to its struct thread would be needed.  i.e.

diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -14,6 +14,7 @@ struct thread {
        struct map_groups       mg;
        pid_t                   pid_; /* Not all tools update this */
        pid_t                   tid;
-       pid_t                   ppid;
+       struct thread           *parent;
        char                    shortname[3];
        bool                    comm_set;

However that means tracking the lifetime of 'parent' to ensure that it is
not left dangling.

Do you want to do that?
--
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/