Hast locking up under 9.2

[Mikolaj Golub]

On Tue, Dec 03, 2013 at 10:23:49AM +0800, David Xu wrote:
> Also I found the following code in hastd:
> 
> #define QUEUE_INSERT1(hio, name, ncomp) do {                            \
>          bool _wakeup;                                                   \
>                                                                          \
>          mtx_lock(&hio_##name##_list_lock[(ncomp)]);                     \
>          _wakeup = TAILQ_EMPTY(&hio_##name##_list[(ncomp)]);             \
>          TAILQ_INSERT_TAIL(&hio_##name##_list[(ncomp)], (hio),           \
>              hio_next[(ncomp)]);                                         \
>          hio_##name##_list_size[(ncomp)]++;                              \
>          mtx_unlock(&hio_##name##_list_lock[ncomp]);                     \
>          if (_wakeup)                                                    \
>                  cv_broadcast(&hio_##name##_list_cond[(ncomp)]);
> 
> Our thread library does optimize the condition variable's wait/signal 
> lock contention, we had implemented wait queue morphying, so it is not
> needed to unlock mutex first, then call cv_broadcast, such code really
> increases spurious wakeups, and can be worse in some cases:
> for example, before cv_broadcast is called, the producer thread is
> preempted, and a consumer thread removes elements in the queue and
> sleeps again, and the producer thread is scheduled again and it blindly
> calls cv_broadcast, and a consumer thread then find nothing in the
> queue, and sleeps again.
> 
> I think following code is enough for our thread library, and works
> better.
> 
> #define QUEUE_INSERT1(hio, name, ncomp) do {                            \
>          mtx_lock(&hio_##name##_list_lock[(ncomp)]);                     \
>          if (TAILQ_EMPTY(&hio_##name##_list[(ncomp)]))                   \
>                  cv_broadcast(&hio_##name##_list_cond[(ncomp)]);         \
>          TAILQ_INSERT_TAIL(&hio_##name##_list[(ncomp)], (hio),           \
>              hio_next[(ncomp)]);                                         \
>          hio_##name##_list_size[(ncomp)]++;                              \
>          mtx_unlock(&hio_##name##_list_lock[ncomp]);                     \
> } while (0)

Ok. I have updated all three macros we have accordingly:

http://people.freebsd.org/~trociny/patches/hast.queue_insert_wakeup.1.patch

Pawel, what do you think?

-- 
Mikolaj Golub