From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54282 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P2yfo-0008Vt-83 for qemu-devel@nongnu.org; Mon, 04 Oct 2010 23:58:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P2yfm-0006Ab-Su for qemu-devel@nongnu.org; Mon, 04 Oct 2010 23:58:40 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:54723) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P2yfm-00069t-Kq for qemu-devel@nongnu.org; Mon, 04 Oct 2010 23:58:38 -0400 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by e31.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id o953k41I030328 for ; Mon, 4 Oct 2010 21:46:04 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o953wXEY125938 for ; Mon, 4 Oct 2010 21:58:33 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o953wW3G024861 for ; Mon, 4 Oct 2010 21:58:33 -0600 Message-ID: <4CAAA267.5040506@linux.vnet.ibm.com> Date: Mon, 04 Oct 2010 20:58:31 -0700 From: "Venkateswararao Jujjuri (JV)" MIME-Version: 1.0 Subject: Re: [Qemu-devel] qemu_cond_signal() taking a long time to complete. References: <20101004104030.GA22130@linux.vnet.ibm.com> <4CA9F4AA.7020007@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: mohan@in.ibm.com, qemu-devel@nongnu.org, sripathik@in.ibm.com, aneesh.kumar@linux.vnet.ibm.com, arun@linux.vnet.ibm.com On 10/4/2010 8:49 AM, Stefan Hajnoczi wrote: > On Mon, Oct 4, 2010 at 4:37 PM, Anthony Liguori wrote: >> On 10/04/2010 05:40 AM, Arun R Bharadwaj wrote: >>> >>> Hi, >>> >>> I am working on introducing threading model into Qemu. This introduces >>> the Threadlets infrastructure which allows subsystems to offload possibly >>> blocking work to a queue to be processed by a pool of threads >>> asynchrnonously. >>> Threadlets are useful when there are operations that can be performed >>> outside the context of the VCPU/IO threads inorder to free these latter >>> to service any other guest requests. As of now we have converted a few >>> v9fs calls like v9fs_read, v9fs_write etc to this model to test the >>> working nature of this model. >>> >>> I observed that performance is degrading in the threading model for the >>> following reason: >>> >>> Taking the example of v9fs_read call: We submit the blocking work in >>> v9fs_read to a queue and do a qemu_cond_signal(). the work will be picked >>> up by a worker thread which is waiting on the condition variable to go >>> true. I measured the time taken to execute the v9fs_read call; in the >>> case without the threading model, it takes around 15microseconds to >>> complete, while in the threading model, it takes around 30microsends >>> to complete. Most of this extra time (around 22microsends) is spent in >>> completing the qemu_cond_signal() call. I suspect this is the reason why >>> I am seeing performance hit with the threading model, because this >>> time is much more than the time needed to complete the entire >>> v9fs_read call in the non threading model case. >>> >>> I need advice on how to proceed from this situation. Pasting relevant >>> code snippets below. >>> >>> thanks >>> arun. >>> --- >>> >>> /* Code to sumbit work to the queue */ >>> void submit_threadlet_to_queue(ThreadletQueue *queue, ThreadletWork >>> *work) >>> { >>> qemu_mutex_lock(&(queue->lock)); >>> if (queue->idle_threads == 0&& queue->cur_threads< >>> queue->max_threads) { >>> spawn_threadlet(queue); >>> } >>> QTAILQ_INSERT_TAIL(&(queue->request_list), work, node); >>> qemu_cond_signal(&(queue->cond)); >>> qemu_mutex_unlock(&(queue->lock)); >>> } >>> >> >> Try moving qemu_cond_signal past qemu_mutex_unlock(). > > There are race conditions here: > > 1. When a new threadlet is started because there are no idle threads, > qemu_cond_signal() may fire a blank because the threadlet isn't inside > qemu_cond_timedwait() yet. The result, the work item is deadlocked > until another thread grabs more work off the queue. If I'm reading Since it is timedwait, it is either the time expiry or more work is added to the queue. - JV > the code correctly this bug is currently present! > 2. Moving qemu_cond_signal() outside queue->lock is dangerous for the > same reason: you need to be careful not to qemu_cond_signal() when the > thread isn't inside qemu_cond_timedwait(). > > Stefan > > Stefan