From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54923 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pf3LJ-0007XL-FA for qemu-devel@nongnu.org; Mon, 17 Jan 2011 23:38:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pf3LI-0006tr-Ac for qemu-devel@nongnu.org; Mon, 17 Jan 2011 23:38:53 -0500 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:51569) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pf3LH-0006tl-Pe for qemu-devel@nongnu.org; Mon, 17 Jan 2011 23:38:52 -0500 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [202.81.31.247]) by e23smtp04.au.ibm.com (8.14.4/8.13.1) with ESMTP id p0I4XXD7012631 for ; Tue, 18 Jan 2011 15:33:33 +1100 Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0I4coid2256938 for ; Tue, 18 Jan 2011 15:38:50 +1100 Received: from d23av02.au.ibm.com (loopback [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0I4cniu032557 for ; Tue, 18 Jan 2011 15:38:50 +1100 Date: Tue, 18 Jan 2011 10:08:45 +0530 From: Arun R Bharadwaj Subject: Re: [Qemu-devel] [PATCH 00/12] Threadlets Infrastructure. Message-ID: <20110118043845.GB21383@linux.vnet.ibm.com> References: <20110113120837.4487.95784.stgit@localhost6.localdomain6> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Reply-To: arun@linux.vnet.ibm.com List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, aliguori@linux.vnet.ibm.com, jvrao@linux.vnet.ibm.com, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com * Stefan Hajnoczi [2011-01-17 10:32:24]: > This series needs a new pair of eyes for review. I'm probably missing > things here after seeing it many times. > > posix-aio-compat.c:handle_work() doesn't need to take aiocb_mutex for > container_of(), which just calculates an address but doesn't actually > access the aiocb. > > dequeue_work_on_queue() is incorrect because a threadlet_worker() may > remove the work item from the request_list just before we remove it > again. That double-remove is wrong and by not detecting this case we > return success even though the work function may be running > concurrently (and it's not safe to do anything with the work item at > this time!). Dequeue needs to lock the request_list, test if the item > is still > on the list, remove it if necessary, and return whether or not it was removed. > > Stefan I'll make the above changes with respect to handle_work() and dequeue_work_on_queue(). Hopefully the patch series gets more review now that I have broken down the patchset so that review becomes easier. -arun