Am 13.03.2012 11:42, schrieb David Gibson:
> 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).
I'm pretty sure that it's older than these patches.
> 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.
Yes, I think it is. Once bdrv_aio_cancel() returns, the block layer
doesn't do anything with the request any more. The caller (e.g. IDE)
can't use the memory areas either because it doesn't even know them.
Kevin
