Re: [PATCH] Robust and Real-time Mutexes in NPTL

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Liu, Bing Wei wrote:

> This is a ---prototype implementation--- that extends NPTL with
> real-time and robust mutex features.

I looked over the code now and can make a few comments.


I like the fact that no new syscall is used.  And that, as it seems,
enabling robustness for mutexes can be implemented for ordinary mutexes.
   We already have a switch in the mutex function entry points, adding
more differentiations is not problem.  The actual implementation of the
.rtflags field in the pthread_mutexattr_t type is fatally flawed,
though.  The size of the type is changed which is completely out of
question.  Instead the additional attributes should be folded into the
.mutexkind field.  This also makes sense since the actual code of the
mutex functions must be different.  For robust mutexes we have no
fastpath at userlevel and therefore the code looks different.

This functionality clearly is useful outside the RT arena.  Therefore
the kernel side should not be mixed with the rtfutex code.  You
definitely should split this functionality out, make the robustness
patch a first, smaller patch.


As for the RT futex stuff.  It looks awfully complicated.  Not that the
topic isn't complicated but I would be surprised if you would get the
code past Linus.  The integration of the RT handling in nptl itself is
not acceptable for the official version.  You can have a separate
libpthread.so binary with the RT behavior but that's it.  The question
is how will this work with inter-process sync objects where one side
uses RT, the other non-RT.  It is possible to declare this a user
problem but it can easily happen.  For mutexes it might be possible to
integrate the changes in libpthread itself.  Not the way you've done it,
but instead by using yet another kind of mutex.  This way regular users
are not disturbed and punished by the RT code.  For rwlocks I don't see
this happening.  Maybe defining a completely separate
pthread_rt_rwlock_t type and set of functions?  Or not handling RT for
rwlocks at all.  POSIX doesn't say anything about such beasts anyway.

condvars are a special case.  Only the pthread_cond_signal code would
need handling.  Maybe some more kernel support is needed, too.  What
should happen is that for FUTEX_RT_REQUEUE the kernel should look for
the highest-priority waiting threads.  If it is possible to avoid
special handling in the cond_wait code I could see RT condvars being
supported by default.


And of course Intel's pet, sched_yield.  The implementation in the patch
is plainly wrong.  sched_yield yields the processor, for the _entire_
process, not just the current thread.  Get over it and fix your code.

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/C8Q52ijCOnn/RHQRAlpZAJ9jBG+vHepGE2hoTJmzVMCLYRq5WwCgx/QU
lLayAGGdhkQ+T/FTtzW3Qog=
=PGnP
-----END PGP SIGNATURE-----