Re: New pthreads library

[Ulrich Drepper <drepper@cygnus.com>]

Hi,

I've taken a brief look at the code.  The model would currently not be
usable for Linux due to missing functionality in the kernel.  We need
this strange manager thread.  But this hopefully will change.

The code itself looks quite reasonable though some things are really
problematic: too many locks.  E.g., you use a lock to increment global
variables.  This should not be necessary.  Most processors define such
an instruction or a number of instructions.  I have added some time
back the header atomicity.h in glibc and the functions contained
should be used.  There are certainly some missing (e.g.,
atomic_increment and atomic_decrement, which are less generic than
atomic_add).

In this context I should complain a bit about the lack of comments.
For code as complicated as the thread library I would like to see lots
of comments.  I know that it seems early to do this but because you
are not going to develop this code alone by yourself it is necessary
to communicate the decisions.  E.g., from the brief look I was not
able to see why you need the PTHREAD_COUNTED flag.

The next problem is the handling of thread descriptor.  We used to use
the pointer to the data structures as the pthread_t element but
stopped this.  I think the main reason was that it caused problems
recognizing when a thread desriptor was illegal.  I personally don't
think this is much of a problem.

What is a problem is that you dereference the pointer directly.  Maybe
you should have taken a look at the LinuxThreads code.  Here I've
added some time last year some changes which move the access in a
macro.  E.g., instead of

	thread->exited

I would write

	THREAD_GETMEM (thread, exited)

The benefit of this is that on modern architectures with dedicated
thread pointer registers (see SPARC) one could define THREAD_GETMEM as
this:

	register pthread_t *__thread_self __asm__ ("%g6");

	#define THREAD_GETMEM(descr, member) __thread_self->member

It should be obvious that this is much better.  Even for x86 it is
possible to user this trick and the only reason I have not activated
this so far in LinuxThreads is that there iss a kernel problem.  But
for the Hurd you should be able to make it work.


A last point is no really criticism on your code.  I planned to revamp
the entire thread library anyway.  One of the things I want to
implement is the separation of kernel and user threads.  User level
thread implementations have big advantages in some situations and I
would like to have a combination.  I have not yet any concrete
planning but I would have waited making with coming up with a new
design until I have an indea how to tackle this problem.  At least the
implementation should be general enough to allow this modified behaviour.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------