Re: cvsup broken on amd64?

On Mon, Oct 03, 2011 at 06:15:41PM +0200, Maxime Henrion wrote:
> Knowing all that, what's happening seems quite clear. If
> fixups_close() is called while there was still fixup requests pending,
> those should be processed by the detailer thread before it returns.
> Subsequent fixups_get() call should continue returning pending items,
> until there are no more entries in the queue _and_ the queue is
> closed.

> So, line 144 in fixups.c (in fixups_get()):

>         if (f->closed) {

> should actually be:

>         if (f->closed && f->size == 0) {

That looks sensible.

> The fact that this could even happen smells a bit weird to me though;
> it means the detailer thread didn't get a chance to run between some
> item was added to the queue (fixups_put() signals the detailer thread
> via pthread_cond_signal()), and that it only runs now that the updater
> queue has been closed. I wouldn't go as far as saying this is a bug,
> but is it even valid for the pthread library to "coalesce" two
> pthread_cond_signal() calls into one when the thread didn't get to run
> since the first call was made? If so, then the above fix is perfectly
> valid. If not, there is a deeper bug in the threading library, or in
> the CVS mode code which I didn't write (but that seems far-fetched).

The pthread library is free to "coalesce" pthread_cond_signal() calls
like that. This is because a thread awakened by pthread_cond_signal()
(or any other event) is not guaranteed to start running immediately and
pthread_cond_signal() does nothing if there is no thread to wake up.

If there is no second CPU core available to run the detailer thread, it
is more efficient to have the updater thread do some more work before
incurring a context switch.

-- 
Jilles Tjoelker