On 2/11/22 16:38, Kevin Wolf wrote:
The callback of an I/O function isn't I/O, though. It is code_after_
the I/O has completed. If this doesn't work any more, it feels like this
is a bug.
The issue is that the I/O has *not* completed yet. blk_aio_preadv(...,
cb, opaque) is not equivalent to
ret = blk_co_preadv(...);
cb(ret, opaque);
but rather to
blk_inc_in_flight(blk);
ret = blk_co_preadv(...);
cb(ret, opaque);
blk_dec_in_flight(blk);
Your own commit message (yeah I know we've all been through that :))
explains why, and notes that it is now invalid to drain in a callback:
commit 46aaf2a566e364a62315219255099cbf1c9b990d
Author: Kevin Wolf <kw...@redhat.com>
Date: Thu Sep 6 17:47:22 2018 +0200
block-backend: Decrease in_flight only after callback
Request callbacks can do pretty much anything, including operations
that will yield from the coroutine (such as draining the backend).
In that case, a decreased in_flight would be visible to other code
and could lead to a drain completing while the callback hasn't
actually completed yet.
Note that reordering these operations forbids calling drain directly
inside an AIO callback.
So the questions are:
1) is the above commit wrong though well-intentioned?
2) is it unexpected that bdrv_replace_child_noperm() drains (thus
becoming invalid from the callback, albeit valid through a bottom half)?
My answer is respectively 1) it's correct, many coroutines do
inc_in_flight before creation and dec_in_flight at the end, we're just
saying that it's _always_ the case for callback-based operations; 2) no,
it's not unexpected and therefore the test is the incorrect one.
Paolo
