Messages in this thread | | | Date | Mon, 20 Mar 2006 21:30:47 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH] simplify/fix first_tid() |
| |
"Eric W. Biederman" wrote: > > Oleg Nesterov <oleg@tv-sign.ru> writes: > > > first_tid: > > > > /* If nr exceeds the number of threads there is nothing todo */ > > if (nr) { > > if (nr >= get_nr_threads(leader)) > > goto done; > > } > > > > This is not reliable: sub-threads can exit after this check, so the > > 'for' loop below can overlap and proc_task_readdir() can return an > > already filldir'ed dirents. > > > > for (; pos && pid_alive(pos); pos = next_thread(pos)) { > > if (--nr > 0) > > continue; > > > > Off-by-one error, will return 'leader' when nr == 1. > > > > This patch tries to fix these problems and simplify the code. > > This is better however if I read this code correctly. It modifies > the code so the last time user space goes trough this loop > with nr > nr_threads. Then we will walk the entire threads > list to achieve nothing.
This can happen only if the thread we stopped at has exited, and some other threads have exited too, so that nr >= ->signal->count.
I think it's not worth optimizing this rare and anyway slow path. However, you are the code author, I'll send a trivial patch which restores this optimization if you don't change you mind.
> So we really still need the nr_threads test in there so we don't > traverse the list twice everytime through readdir.
How so? We don't do it twice?
Oleg. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |