From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54561 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PaZSg-0000ez-AX for qemu-devel@nongnu.org; Wed, 05 Jan 2011 14:55:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PaZSe-0000HG-VL for qemu-devel@nongnu.org; Wed, 05 Jan 2011 14:55:58 -0500 Received: from mtagate7.uk.ibm.com ([194.196.100.167]:51448) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PaZSe-0000H3-MZ for qemu-devel@nongnu.org; Wed, 05 Jan 2011 14:55:56 -0500 Received: from d06nrmr1307.portsmouth.uk.ibm.com (d06nrmr1307.portsmouth.uk.ibm.com [9.149.38.129]) by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p05Jtt55004804 for ; Wed, 5 Jan 2011 19:55:55 GMT Received: from d06av04.portsmouth.uk.ibm.com (d06av04.portsmouth.uk.ibm.com [9.149.37.216]) by d06nrmr1307.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p05JtvPq3203246 for ; Wed, 5 Jan 2011 19:55:57 GMT Received: from d06av04.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p05JttHk018689 for ; Wed, 5 Jan 2011 12:55:55 -0700 Date: Wed, 5 Jan 2011 19:55:46 +0000 From: Stefan Hajnoczi Message-ID: <20110105195545.GD9821@stefanha-thinkpad.localdomain> References: <20110104052627.15887.43436.stgit@localhost6.localdomain6> <20110104052739.15887.60037.stgit@localhost6.localdomain6> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110104052739.15887.60037.stgit@localhost6.localdomain6> Subject: [Qemu-devel] Re: [PATCH 06/13] Threadlet: Add dequeue_work threadlet API List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Arun R Bharadwaj Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com On Tue, Jan 04, 2011 at 10:57:39AM +0530, Arun R Bharadwaj wrote: > @@ -574,33 +574,39 @@ static void paio_remove(struct qemu_paiocb *acb) > } > } > > -static void paio_cancel(BlockDriverAIOCB *blockacb) > +/** > + * dequeue_work: Cancel a task queued on the global queue. > + * @work: Contains the information of the task that needs to be cancelled. > + * > + * Returns: 0 if the task is successfully cancelled. > + * 1 otherwise. > + */ > +static int dequeue_work(ThreadletWork *work) > { > - struct qemu_paiocb *acb = (struct qemu_paiocb *)blockacb; > - int active = 0; > + int ret = 1; > > qemu_mutex_lock(&globalqueue.lock); > - if (!acb->active) { > - QTAILQ_REMOVE(&globalqueue.request_list, &acb->work, node); > - acb->ret = -ECANCELED; > - } else if (acb->ret == -EINPROGRESS) { > - active = 1; > - } > + QTAILQ_REMOVE(&globalqueue.request_list, work, node); > + ret = 0; > qemu_mutex_unlock(&globalqueue.lock); > > - qemu_mutex_lock(&aiocb_mutex); > - if (!active) { > - acb->ret = -ECANCELED; > - } else { > - while (acb->ret == -EINPROGRESS) { > - /* > - * fail safe: if the aio could not be canceled, > - * we wait for it > - */ > - qemu_cond_wait(&aiocb_completion, &aiocb_mutex); > + return ret; It always succeeds? Why bother with the ret local variable? > +} > + > +static void paio_cancel(BlockDriverAIOCB *blockacb) > +{ > + struct qemu_paiocb *acb = (struct qemu_paiocb *)blockacb; > + if (!acb->active) { > + if (dequeue_work(&acb->work) != 0) { > + /* Wait for running work item to complete */ > + qemu_mutex_lock(&aiocb_mutex); > + while (acb->ret == -EINPROGRESS) { > + qemu_cond_wait(&aiocb_completion, &aiocb_mutex); > + } > + qemu_mutex_unlock(&aiocb_mutex); > } > } > - qemu_mutex_unlock(&aiocb_mutex); > + > paio_remove(acb); I'm not convinced this function works. If the request is active in a worker thread and paio_cancel() is called then we invoke paio_remove(). Stefan