Date: Wed, 29 Mar 2006 11:10:27 +0200 From: des@des.no (Dag-Erling =?iso-8859-1?Q?Sm=F8rgrav?=) To: David Xu <davidxu@freebsd.org> Cc: threads@freebsd.org, jeff@freebsd.org Subject: Re: libthr cleanup Message-ID: <861wwlehvg.fsf@xps.des.no> In-Reply-To: <442A466A.9040506@freebsd.org> (David Xu's message of "Wed, 29 Mar 2006 16:33:46 %2B0800") References: <86slp1u4qb.fsf@xps.des.no> <442A466A.9040506@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
David Xu <davidxu@freebsd.org> writes: > When libthr was redesigned, things were not clear, I have figured out > all semantics needed to implement libthr, the above functions are the > interfaces current internally implemented by libthr, and functions in > umtx.h is not used because it can generate bloat code than the > versions in libthr, current libthr shared library only has 64K size. > I really dont need struct umtx at all, an integer is enough, basic idea > is using atomic operations to test in userland and wait in kernel. > the above functions should be changed to: > int _umtx_lock_timeout(umtx_t *, > const struct timespec * __restrict timeout); > int _umtx_unlock(umtx_t *mtx); > int _umtx_wait_timeout(umtx_t *, umtx_t expect, > const struct timespec * __restrict timeout); > int _umtx_wake(umtx_t *i, int number); > > the umtx_t could be an integer type, but to maintain binary > compatibility, it has to be a long integer type. To maintain compatibility and ensure type safety, it should be a struct umtx. In effect, the wait / wake operations implement a condition variable. You should not use the same struct (or type) to describe the condition variable as the one you use to describe a mutex. Condition variables are always used in conjunction with a mutex. The mutex must be passed to the wait function so it can be unlocked while the waiting thread waits. It must be held by the thread calling the wake method. Neither the existing interface nor the one you propose do this. > Well, I am not sure you know the history of umtx, the orignal work > only did a lock and unlock semantic, there is no general sleep and > wait semantic, orignal umtx code has obscure race, I think in early > days, valgrind suffered from this bug. I am not sure you understand > how a userland synchronization works, these two operations are used > to implement compare and wait for a integer to be changed, You cannot "wait for an integer to be changed" (at least not without using hardware debugging facilities), and that is not what UMTX_OP_WAIT does. It is a botched condition variable. It waits for another thread to perform an UMTX_OP_WAKE operation with the correct arguments; the fact that an integer has changed is incidental, and the test for that change could be implemented in userland: look up condition variables in any good CS textbook and you will see an example of this, likely in the guise of a sample message queue (or mailbox) implementation. I can understand wanting to move the check into the kernel to avoid spurious context switches, but it has to be done right. > you > should check source code before talking a lot, That is precisely what I have done. > saying that you doubt > the UMTX_OP_WAIT and UMTX_OP_WAKE's correctness is not professional, > there are users using libthr in daily work and stress testing had > been done. > I have put lots of work and time on libthr, I know the problems, > though the _umtx interface is a bit ulgly because it was unclear > when it was being designed, but I don't think it really hurt you or > other people, it can be fixed in few days, I just was hesitating to > add the new interfaces. The amount of work you have put into libthr and its importance to your self-esteem do not guarantee its correctness. What is unprofessional here is 1) the quality of your code and 2) your refusal to consider that I might have a point. DES --=20 Dag-Erling Sm=F8rgrav - des@des.no
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?861wwlehvg.fsf>