Re: [PATCH] ftrace: do_each_pid_task() needs rcu lock

[ebiederm@xmission.com (Eric W. Biederman)]

Oleg Nesterov <oleg@redhat.com> writes:

> On 02/03, Eric W. Biederman wrote:
>>
>> Oleg Nesterov <oleg@redhat.com> writes:
>>
>> > On 02/03, Oleg Nesterov wrote:
>> >>
>> >> "ftrace: use struct pid" commit 978f3a45d9499c7a447ca7615455cefb63d44165
>> >> converted ftrace_pid_trace to "struct pid*". But we can't use
>> >> do_each_pid_task() without rcu_read_lock() even if we know the pid
>> >> itself can't go away (it was pinned in ftrace_pid_write). The exiting
>> >> task can detach itself from this pid at any moment.
>> >
>> > Q: why do we use do_each_pid_task(PIDTYPE_PID) ? We can never have more
>> > than 1 task in the loop. Perhaps,
>>
>> That is a bug in do_each_pid_task(PIDTYPE_PID).
>> For ftrace we really want to grab all tasks with a given pid even
>> in the crazy exec case.
>
> Yes, I thought about de_thread() too. But we can't "fix" do_each_pid_task()
> to avoid the race?

For the case of exec there is that.  There is also the case that
ftrace unlike everything else wants to trace be able to trace all of
the idle threads with pid 0.  I think that is a special case
currently, but for that case the only correct version I can think
of do_each_task_pid(), and current do_each_task_pid is wrong because
it does not allow that.

Eric