Messages in this thread | | | From | (Eric W. Biederman) | Date | Thu, 11 Mar 2021 10:30:08 -0600 | Subject | Re: [PATCH v3] do_wait: make PIDTYPE_PID case O(1) instead of O(n) |
| |
Jim Newsome <jnewsome@torproject.org> writes:
> On 3/10/21 16:40, Eric W. Biederman wrote: >>> +// Optimization for waiting on PIDTYPE_PID. No need to iterate > through child >>> +// and tracee lists to find the target task. >> >> Minor nit: C++ style comments look very out of place in this file >> which uses old school C /* */ comment delimiters for >> all of it's block comments. > > Will do > >>> +static int do_wait_pid(struct wait_opts *wo) >>> +{ >>> + struct task_struct *target = pid_task(wo->wo_pid, PIDTYPE_PID); >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> This is subtle change in behavior. >> >> Today on the task->children list we only place thread group leaders. > > Shouldn't we allow waiting on clone children if __WALL or __WCLONE is set? > > This is already checked later in `eligible_child`, called from > `wait_consider_task`, so I *think* the current form should already do > the right thing. Now I'm confused though how the general path (through > `do_wait_thread`) works if clone children aren't on the task->children > list...? > > (In any case it seems this will need another version with at least an > explanatory comment here)
What I am worried about are not clone children. AKA ordinary children that have a different exit signal but CLONE_THREAD children that are never put on the children list so are naturally excluded from today's do_wait (except in the case of ptrace). These are also known as threads.
Maybe I am missing it but I don't see anything in wait_consider_task or in the way that you are calling it that would exclude CLONE_THREAD children for the non-ptrace case.
Eric
| |