On Tue, Feb 15, 2022 at 12:14 PM Hanna Reitz <hre...@redhat.com> wrote: > > Ping > > (I can take it too, if you’d like, John, but you’re listed as the only > maintainer for hw/ide, so... Just say the word, though!) >
Sorry, I sent you a mail off-list at the time where I said you were free to take it whenever you like. Why'd I send it off-list? I don't know.... Please feel free to send this with your next block PR. --js > On 20.01.22 15:22, Hanna Reitz wrote: > > When we still have an AIOCB registered for DMA operations, we try to > > settle the respective operation by draining the BlockBackend associated > > with the IDE device. > > > > However, this assumes that every DMA operation is associated with an > > increment of the BlockBackend’s in-flight counter (e.g. through some > > ongoing I/O operation), so that draining the BB until its in-flight > > counter reaches 0 will settle all DMA operations. That is not the case: > > For TRIM, the guest can issue a zero-length operation that will not > > result in any I/O operation forwarded to the BlockBackend, and also not > > increment the in-flight counter in any other way. In such a case, > > blk_drain() will be a no-op if no other operations are in flight. > > > > It is clear that if blk_drain() is a no-op, the value of > > s->bus->dma->aiocb will not change between checking it in the `if` > > condition and asserting that it is NULL after blk_drain(). > > > > The particular problem is that ide_issue_trim() creates a BH > > (ide_trim_bh_cb()) to settle the TRIM request: iocb->common.cb() is > > ide_dma_cb(), which will either create a new request, or find the > > transfer to be done and call ide_set_inactive(), which clears > > s->bus->dma->aiocb. Therefore, the blk_drain() must wait for > > ide_trim_bh_cb() to run, which currently it will not always do. > > > > To fix this issue, we increment the BlockBackend's in-flight counter > > when the TRIM operation begins (in ide_issue_trim(), when the > > ide_trim_bh_cb() BH is created) and decrement it when ide_trim_bh_cb() > > is done. > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980 > > Suggested-by: Paolo Bonzini <pbonz...@redhat.com> > > Signed-off-by: Hanna Reitz <hre...@redhat.com> > > --- > > v1: > > https://lists.nongnu.org/archive/html/qemu-block/2022-01/msg00024.html > > > > v2: > > - Increment BB’s in-flight counter while the BH is active so that > > blk_drain() will poll until the BH is done, as suggested by Paolo > > > > (No git-backport-diff, because this patch was basically completely > > rewritten, so it wouldn’t be worth it.) > > --- > > hw/ide/core.c | 7 +++++++ > > 1 file changed, 7 insertions(+) >
