From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51364) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StiOb-0002iV-3F for qemu-devel@nongnu.org; Tue, 24 Jul 2012 12:55:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StiOZ-0000RC-Qq for qemu-devel@nongnu.org; Tue, 24 Jul 2012 12:55:40 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:62407) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StiOZ-0000R5-LL for qemu-devel@nongnu.org; Tue, 24 Jul 2012 12:55:39 -0400 Received: by yenl1 with SMTP id l1so6999147yen.4 for ; Tue, 24 Jul 2012 09:55:39 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <500ED384.5030901@redhat.com> Date: Tue, 24 Jul 2012 18:55:32 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1342435377-25897-1-git-send-email-pbonzini@redhat.com> <1342435377-25897-8-git-send-email-pbonzini@redhat.com> <50040251.5000700@siemens.com> <50041510.5070105@redhat.com> <50041869.20006@siemens.com> <500418BC.8080200@redhat.com> <50041CE7.80501@siemens.com> <50041F28.3030203@redhat.com> <50042086.3050101@siemens.com> <5004232A.9010301@redhat.com> In-Reply-To: <5004232A.9010301@redhat.com> Content-Type: multipart/mixed; boundary="------------040901080606060500010300" Subject: Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "kwolf@redhat.com" , "stefanha@linux.vnet.ibm.com" , Jan Kiszka , "qemu-devel@nongnu.org" , "aliguori@linux.vnet.ibm.com" , "sw@weilnetz.de" This is a multi-part message in MIME format. --------------040901080606060500010300 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Il 16/07/2012 16:20, Paolo Bonzini ha scritto: >> > ...and that's why you check what needs to be done to handle this race >> > after grabbing the mutex. IOW, replicate the state information that the >> > Windows semaphore contains into the emulated condition variable object. > It is already there (cv->waiters), but it is accessed atomically. To do > what you suggest I would need to add a mutex. FWIW, I found a good condvar implementation in Chromium, but I really don't have the time to port it over to QEMU right now. I still would like to get the semaphore version in 1.2. Also, the attached pseudo-patch is an example of using semaphores to limit the size of the critical sections, and also decrease the number of threads created. I'm not proposing to include it now, it's just an example of things that are harder with condition variables than with semaphores. Paolo --------------040901080606060500010300 Content-Type: text/x-patch; name="thread-pool-sem.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="thread-pool-sem.patch" diff --git a/thread-pool.c b/thread-pool.c index 7895544..72be971 100644 --- a/thread-pool.c +++ b/thread-pool.c @@ -71,20 +72,16 @@ static void *worker_thread(void *unused) ThreadPoolElement *req; int ret; - qemu_mutex_lock(&lock); - idle_threads++; - qemu_mutex_unlock(&lock); - ret = qemu_sem_timedwait(&sem, 10000); - qemu_mutex_lock(&lock); - idle_threads--; + atomic_inc(&idle_threads); + do { + ret = qemu_sem_timedwait(&sem, 10000); + } while (ret == -1 && atomic_read(&QTAILQ_FIRST(&request_list)) != NULL); + atomic_dec(&idle_threads); if (ret == -1) { - if (QTAILQ_EMPTY(&request_list)) { - break; - } - qemu_mutex_unlock(&lock); - continue; + break; } + qemu_mutex_lock(&lock); req = QTAILQ_FIRST(&request_list); QTAILQ_REMOVE(&request_list, req, reqs); req->state = THREAD_ACTIVE; @@ -226,7 +223,7 @@ BlockDriverAIOCB *thread_pool_submit_aio(ThreadPoolFunc *func, void *arg, trace_thread_pool_submit(req, arg); qemu_mutex_lock(&lock); - if (idle_threads == 0 && cur_threads < max_threads) { + if (atomic_read(&idle_threads) == 0 && cur_threads < max_threads) { spawn_thread(); } QTAILQ_INSERT_TAIL(&request_list, req, reqs); --------------040901080606060500010300--