On 29/07/19 23:46, John Snow wrote:
>> @@ -111,15 +112,12 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
>> {
>> trace_dma_complete(dbs, ret, dbs->common.cb);
>>
>> + assert(!dbs->acb && !dbs->bh);
>> dma_blk_unmap(dbs);
>> if (dbs->common.cb) {
>> dbs->common.cb(dbs->common.opaque, ret);
>> }
>> qemu_iovec_destroy(&dbs->iov);
>> - if (dbs->bh) {
>> - qemu_bh_delete(dbs->bh);
>> - dbs->bh = NULL;
>> - }
>
> Now presumably handled by dma_aio_cancel,
No, it simply could never happen. dma_complete is called here in dma_blk_cb:
dbs->acb = NULL;
dbs->offset += dbs->iov.size;
if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
dma_complete(dbs, ret);
return;
}
and the only way to reach that when dbs->bh becomes non-NULL is through
reschedule_dma, which clears dbs->bh before invoking dma_blk_cb.
>> if (dbs->acb) {
>> + /* This will invoke dma_blk_cb. */
>
> uhh, does it?
Yes:
/* Async version of aio cancel. The caller is not blocked if the acb implements
* cancel_async, otherwise we do nothing and let the request normally complete.
* In either case the completion callback must be called. */
> this is maybe where I got lost reading this code.
> Isn't dbs->acb going to be what was returned from e.g.
> dma_blk_read_io_func, which ultimately uses blk_aio_em_aiocb_info, that
> has no cancel callback?
Right therefore the I/O will complete and the callback will be invoked.
> Well, here at least I am now on terra-firma that we're going to call the
> original callback with ECANCELED, which is a step towards code that
> isn't surprising my sensibilities.
Good. :)
Paolo
