Am 12.02.2025 um 10:32 hat Hanna Czenczek geschrieben:
> RBD schedules the request completion code (qemu_rbd_finish_bh()) to run
> in the BDS's AioContext. The intent seems to be to run it in the same
> context that the original request coroutine ran in, i.e. the thread on
> whose stack the RBDTask object exists (see qemu_rbd_start_co()).
>
> However, with multiqueue, that thread is not necessarily the same as the
> BDS's AioContext. Instead, we need to remember the actual AioContext
> and schedule the completion BH there.
>
> Buglink: https://issues.redhat.com/browse/RHEL-67115
Please add a short summary of what actually happens to the commit
message. I had to check the link to remember what the symptoms are.
> Reported-by: Junyao Zhao <junz...@redhat.com>
> Signed-off-by: Hanna Czenczek <hre...@redhat.com>
> ---
> I think I could also drop RBDTask.ctx and just use
> `qemu_coroutine_get_aio_context(RBDTask.co)` instead, but this is the
> version of the patch that was tested and confirmed to fix the issue (I
> don't have a local reproducer), so I thought I'll post this first.
Did you figure out why it even makes a difference in which thread
qemu_rbd_finish_bh() runs? For context:
static void qemu_rbd_finish_bh(void *opaque)
{
RBDTask *task = opaque;
task->complete = true;
aio_co_wake(task->co);
}
This looks as if it should be working in any thread, except maybe for a
missing barrier after updating task->complete - but I think the failure
mode for that would be a hang in qemu_rbd_start_co().
> block/rbd.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index af984fb7db..9d4e0817e0 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -102,7 +102,7 @@ typedef struct BDRVRBDState {
> } BDRVRBDState;
>
> typedef struct RBDTask {
> - BlockDriverState *bs;
> + AioContext *ctx;
> Coroutine *co;
> bool complete;
> int64_t ret;
> @@ -1269,8 +1269,7 @@ static void qemu_rbd_completion_cb(rbd_completion_t c,
> RBDTask *task)
> {
> task->ret = rbd_aio_get_return_value(c);
> rbd_aio_release(c);
> - aio_bh_schedule_oneshot(bdrv_get_aio_context(task->bs),
> - qemu_rbd_finish_bh, task);
> + aio_bh_schedule_oneshot(task->ctx, qemu_rbd_finish_bh, task);
> }
>
> static int coroutine_fn qemu_rbd_start_co(BlockDriverState *bs,
> @@ -1281,7 +1280,10 @@ static int coroutine_fn
> qemu_rbd_start_co(BlockDriverState *bs,
> RBDAIOCmd cmd)
> {
> BDRVRBDState *s = bs->opaque;
> - RBDTask task = { .bs = bs, .co = qemu_coroutine_self() };
> + RBDTask task = {
> + .ctx = qemu_get_current_aio_context(),
> + .co = qemu_coroutine_self(),
> + };
> rbd_completion_t c;
> int r;
Nothing wrong I can see about the change, but I don't understand why it
fixes the problem.
Kevin
