[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] blk: postpone request execution on a context pr
Re: [Qemu-devel] [PATCH] blk: postpone request execution on a context protected with "drained section"
Wed, 12 Dec 2018 13:24:57 +0100
Am 11.12.2018 um 17:55 hat Denis Plotnikov geschrieben:
> > Why involve the AioContext at all? This could all be kept at the
> > BlockBackend level without extending the layering violation that
> > aio_disable_external() is.
> > BlockBackends get notified when their root node is drained, so hooking
> > things up there should be as easy, if not even easier than in
> > AioContext.
> Just want to make sure that I understood correctly what you meant by
> "BlockBackends get notified". Did you mean that bdrv_drain_end calls
> child's role callback blk_root_drained_end by calling
Yes, blk_root_drained_begin/end calls are all you need. Specifically,
their adjustments to blk->quiesce_counter that are already there, and in
the 'if (--blk->quiesce_counter == 0)' block of blk_root_drained_end()
we can resume the queued requests.
> In case if it's so, it won't work if resume postponed requests in
> blk_root_drained_end since we can't know if external is disabled for the
> context because the counter showing that is decreased only after roles'
> drained callbacks are finished at bdrv_do_drained_end.
> Please correct me if I'm wrong.
You don't need to know about the AioContext state, this is the whole
point. blk->quiesce_counter is what tells you whether to postpone
> Looking at the patch again, I think that it might be useful to keep the
> requests in the structure that limits their execution and also protects
> the access (context acquire/release) although it's indeed the layering
> violation but at least we can store the parts related at the same place
> and later on move somewhere else alongside the request restrictor.
You can keep everything you need in BlockBackend (and that's also where
your code is that really postpones request).