Hast locking up under 9.2

[David Xu]

On 2013/12/05 05:37, Mikolaj Golub wrote:
> 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?
>

No problem. ;-)