On 10/24, Peter Zijlstra wrote: > > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -1257,7 +1257,14 @@ static u32 perf_event_pid(struct perf_event *event, > struct task_struct *p) > if (event->parent) > event = event->parent; > > - return task_tgid_nr_ns(p, event->ns); > + /* > + * It is possible the task already got unhashed, in which case we > + * cannot determine the current->group_leader/real_parent. > + * > + * Also, report -1 to indicate unhashed, so as not to confused with > + * 0 for the idle task. > + */ > + return pid_alive(p) ? task_tgid_nr_ns(p, event->ns) : ~0; > }
Yes, but this _looks_ racy unless p == current. I mean, pid_alive() makes task_tgid_nr_ns() safe, but task_tgid_nr_ns() still can return zero _if_ it can race with the exiting task. > static u32 perf_event_tid(struct perf_event *event, struct task_struct *p) > @@ -1268,7 +1275,7 @@ static u32 perf_event_tid(struct perf_event *event, > struct task_struct *p) > if (event->parent) > event = event->parent; > > - return task_pid_nr_ns(p, event->ns); > + return pid_alive(p) ? task_pid_nr_ns(p, event->ns) : ~0; The same. However. At first glance the only case when p != current is copy_process(), right? And in this case the new child can't go away. So I think this patch is fine. Oleg.
