[Qemu-devel] Re: [PATCH 01/13] Add aiocb_mutex and aiocb_completion.

[Arun R Bharadwaj <arun@linux.vnet.ibm.com>]

* Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> [2011-01-05 19:53:44]:

> On Tue, Jan 04, 2011 at 10:57:08AM +0530, Arun R Bharadwaj wrote:
> > @@ -545,13 +555,19 @@ static void paio_cancel(BlockDriverAIOCB *blockacb)
> >      }
> >      mutex_unlock(&lock);
> >  
> > -    if (active) {
> > -        /* fail safe: if the aio could not be canceled, we wait for
> > -           it */
> > -        while (qemu_paio_error(acb) == EINPROGRESS)
> > -            ;
> > +    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);
> > +        }
> >      }
> > -
> > +    qemu_mutex_unlock(&aiocb_mutex);
> >      paio_remove(acb);
> >  }
> 
> acb->ret and acb->active have been moved under aiocb_mutex.  They are still
> accessed under lock here and this needs to be fixed:
> 
> mutex_lock(&lock);
> if (!acb->active) {
>     QTAILQ_REMOVE(&request_list, acb, node);
>     acb->ret = -ECANCELED;
> } else if (acb->ret == -EINPROGRESS) {
>     active = 1;
> }
> mutex_unlock(&lock);
> 

You are right. This needs to go under aiocb_mutex too.

-arun

> Stefan