Am 06.04.2017 um 16:25 hat Fam Zheng geschrieben: > During block job completion, nothing is preventing > block_job_defer_to_main_loop_bh from being called in a nested > aio_poll(), which is a trouble, such as in this code path: > > qmp_block_commit > commit_active_start > bdrv_reopen > bdrv_reopen_multiple > bdrv_reopen_prepare > bdrv_flush > aio_poll > aio_bh_poll > aio_bh_call > block_job_defer_to_main_loop_bh > stream_complete > bdrv_reopen > > block_job_defer_to_main_loop_bh is the last step of the stream job, > which should have been "paused" by the bdrv_drained_begin/end in > bdrv_reopen_multiple, but it is not done because it's in the form of a > main loop BH. > > Similar to why block jobs should be paused between drained_begin and > drained_end, BHs they schedule must be excluded as well. To achieve > this, this patch forces draining the BH before leaving bdrv_drained_begin(). > > Signed-off-by: Fam Zheng <f...@redhat.com>
We used to poll BHs in earlier times. Commit 99723548 ('block: add BDS field to count in-flight requests') changed this, without an explanation in the commit message. Paolo, is this simply a bug in that commit, or do you rely on it somewhere? I remember that you wanted to get rid of some aio_poll() calls a while ago. > block/io.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/block/io.c b/block/io.c > index 2709a70..b9cfd18 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -228,7 +228,12 @@ void bdrv_drained_begin(BlockDriverState *bs) > bdrv_parent_drained_begin(bs); > } > > - bdrv_drain_recurse(bs); > + while (true) { > + if (!bdrv_drain_recurse(bs) && > + !aio_poll(bdrv_get_aio_context(bs), false)) { > + break; > + } The indentation is off here. > + } > } The old code had this in what is now the BDRV_POLL_WHILE() call in bdrv_drain_recurse(). I think it makes more sense there, saves you a loop here and fixes bdrv_drain_all_begin() at the same time. Kevin