Re: [PATCH V2 15/15] perf tools: fix ppid in thread__fork()

[Adrian Hunter]

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@xxxxxxxxx>
>> ---
>>   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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/