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