On 29/07/19 21:45, John Snow wrote: > Next, we'll unschedule the BH if there is one. I think the only case > where there is one is the reschedule_dma case of dma_blk_cb. (I'm not > too familiar with these DMA helpers: in what cases do we expect the iov > to be empty?)
When there is another I/O that is using the DMA bounce buffer (the one case that comes to mind in which you do DMA from MMIO areas is loading/saving VGA RAM). > So it looks like this cancellation will produce one of two effects, > depending on when it's invoked: > > 1) We'll stall the DMA permanently by deleting that BH, because > dma_complete will never get invoked and therefore nobody will ever call > ide_dma_cb with any return value of any kind. The IDE state machine > likely just hangs waiting for the DMA to finish until the guest OS > decides to reset the errant controller. > > 2) The DMA will continue blissfully unaware it was canceled, because the > lower AIOCB has no cancel method, and so will finish, call back to > dma_blk_cb, and continue the transfer loop unaware. > > > ... Does your reading align with mine? > > > If it does -- if there are indeed no places in the code today that > artificially inject -ECANCELED -- I need to remove these special stanzas > from the IDE code and allow the IDE state machine to handle these errors > as true errors. The bug is that there is no place to inject -ECANCELED in the dbs->bh case. I've sent an obviously^W untested patch. Paolo > I'm just not confident enough in my unwinding of the DMA callback > spaghetti, though. > > --js >
