On Fri, Mar 09, 2012 at 10:43:35AM +0100, Kevin Wolf wrote:
> Am 09.03.2012 06:01, schrieb David Gibson:
[snip]
> > @@ -104,10 +104,20 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
> > }
> > }
> >
> > +static void dma_bdrv_cancel(void *opaque)
> > +{
> > + DMAAIOCB *dbs = opaque;
> > +
> > + bdrv_aio_cancel(dbs->acb);
> > + dma_bdrv_unmap(dbs);
> > + qemu_iovec_destroy(&dbs->iov);
> > + qemu_aio_release(dbs);
> > +}
>
> I'm lacking the context to know when this is actually called, but it
> looks suspicious. Did you consider that bdrv_aio_cancel() can actually
> invoke the completion callback?
>
> What's the difference between the existing dma_aio_cancel() and the
> function that you need here?
So, first thing to note is that as I said in another sub-thread, there
are several approaches we could take for handling invalidation of
IOMMU mappings while they're in use by drivers, and I'm not sure which
is best yet.
Second is that this piece of code comes from Eduard - Gabriel's
original and I haven't actually understood it as well as I should have
:).
So, examining in more detail it looks like dma_aio_cancel() is the
right thing to do (I don't know if it existed when Eduard - Gabriel
wrote the initial version).
The semantics of the callback are that after it's complete, there
should be no further access to the dma_memory_map()ed memory areas. I
haven't yet understood the bdrv stuff sufficiently well to be
completely sure that's true.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
