qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/3] thread-pool: replace semaphore with condition variabl


From: Paolo Bonzini
Subject: Re: [PATCH v2 2/3] thread-pool: replace semaphore with condition variable
Date: Fri, 13 May 2022 14:38:21 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0

On 5/13/22 13:56, Nicolas Saenz Julienne wrote:
              pool->idle_threads++;
-            qemu_mutex_unlock(&pool->lock);
-            ret = qemu_sem_timedwait(&pool->sem, 10000);
-            qemu_mutex_lock(&pool->lock);
+            ret = qemu_cond_timedwait(&pool->request_cond, &pool-
lock, 10000);
              pool->idle_threads--;
-        } while (back_to_sleep(pool, ret));
-        if (ret == -1 || pool->stopping ||

I think, you need to check for 'pool->stopping' upon exiting wait_cond().
Otherwise it'll blindly try to dequeue a request from a list that is otherwise
empty.

Good point, thanks.

-    if (elem->state == THREAD_QUEUED &&
-        /* No thread has yet started working on elem. we can try to
"steal"
-         * the item from the worker if we can get a signal from the
-         * semaphore.  Because this is non-blocking, we can do it
with
-         * the lock taken and ensure that elem will remain
THREAD_QUEUED.
-         */
-        qemu_sem_timedwait(&pool->sem, 0) == 0) {
+    if (elem->state == THREAD_QUEUED) {
          QTAILQ_REMOVE(&pool->request_list, elem, reqs);
          qemu_bh_schedule(pool->completion_bh);

The 'thread-pool cancel' unit test fails.

I think it's because there is an assumption in worker_thread() that if you get
woken up, you'll have a pending request. And you're now 'stealing' work
requests, without 'stealing' a wakeup (what qemu_sem_timedwait(sem, 0) achieved
in the past).

You don't need to steal a wakeup because cond_wait does not "count", but yeah it's essentially the same issue that you mentioned above.

Paolo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]