Re: bugs in test-lock

[Torvald Riegel]

On Fri, 2017-01-06 at 00:11 +0100, Bruno Haible wrote:
> Torvald Riegel wrote:
> > > The problem here is: it's a unit test. If it fails, I don't want to search
> > > for bugs in the condition variable subsystem, semaphore subsystem, AND the
> > > lock subsystem. I want the test to rely essentially only on the lock 
> > > subsystem.
> > 
> > locks are the wrong mechanism.  locks give you mutual exclusion.  If you
> > want a simple notification that you can query without blocking, use a
> > semaphore.
> 
> OK, thanks for this clear advice. I've pushed the change below.
> 
> The timings look OK:
> EXPLICIT_YIELD = 1 USE_SEMAPHORE = 1      1.79 sec
> EXPLICIT_YIELD = 1 USE_SEMAPHORE = 0      1.78 sec
> EXPLICIT_YIELD = 0 USE_SEMAPHORE = 1      3.3 .. 3.8 sec
> EXPLICIT_YIELD = 0 USE_SEMAPHORE = 0      3.3 .. 3.9 sec

IIRC that's for the test where some threads have unlimited work and some
have bounded work, right?  I would be careful in interpreting these
numbers, because you measure not just fairness, but also lock
contention, etc.  These can behave in ways that might be surprising if
you're not looking at stuff like this constantly.

> > > That's why I inserted the yield statements in the lock_checker_thread:
> > > 
> > >   for (;;)
> > >     {
> > >       pthread_mutex_lock (&lock);
> > >       do something;
> > >       pthread_mutex_unlock (&lock);
> > >       do very little other things;
> > >       pthread_yield ();
> > >     }
> > 
> > Why do you think this works?
> 
> I think this works because
>   - the manual page http://man7.org/linux/man-pages/man3/pthread_yield.3.html
>     says "The thread is placed at the end of the run queue". To me, this means
>     that if all participating threads call pthread_yield () once in a while,
>     all threads have a chance to be woken up and get the lock within a 
> reasonable
>     amount of time.
>   - the timings of EXPLICIT_YIELD = 1 vs. 0 (above) show that it has some
>     effect.
> 
> > We don't have uniprocessors anymore,
> > there's more that's affecting execution order than just the OS
> > scheduler.
> > ...
> > There's no guarantee that this will give fairness to calls before or
> > after it.  The OS scheduler may let other runnable processes run
> > first, but modern synchronization primitives try not to involve the OS
> > as much as possible because of the overheads this typically involves.
> 
> Are you suggesting that the pthread_yield manual page is wrong? Or that
> some warning should be added to it? I'm sure Michael Kerrisk will accept 
> inputs.

I don't think it's necessarily wrong, but if there's no actual run queue
because you have more or the same number of cores (or HW threads)
available than what the number of runnable OS threads is, then this
won't push through a FIFO run queue and thus won't order them one after
the other.

> > > For a real notification mechanism, probably a pthread_cond_t would be the 
> > > right
> > > thing.
> > 
> > But you don't want to wait for a condition, you want to query whether a
> > condition holds.  The latter is not supported by a condvar.
> 
> Oh. Really, "wait queue" would be a better name than "condition variable"
> for this thing.

Well, it's not a queue either because of spurious wake-ups being
allowed, for example.
What you can look at as hint is that there's no pthread_cond_trywait,
but there is a sem_trywait, for example.

Regarding the patch, why don't you just use the semaphore directly, but
wrap it as an atomic_int (which it is not)?  Do you have to support
platforms that offer something like a semaphore, or a latch, or an
atomic flag (with a happens-before between setter and observer seeing
the flag set)?