Thanks, tested this on the CI system and with this patch it seems to work well.
Regards, Lukáš Tested-by: Lukáš Doktor <[email protected]> Dne 12. 12. 25 v 11:25 Hanna Czenczek napsal(a): > This reverts commit 0f142cbd919fcb6cea7aa176f7e4939925806dd9. > > Lukáš Doktor reported a simple single-threaded nvme test case hanging > and bisected it to this commit. While we are still investigating, it is > best to revert the commit for now. > > (This breaks multiqueue for nvme, but better to have single-queue > working than neither.) > > Cc: [email protected] > Reported-by: Lukáš Doktor <[email protected]> > Signed-off-by: Hanna Czenczek <[email protected]> > --- > block/nvme.c | 56 +++++++++++++++++++++++++--------------------------- > 1 file changed, 27 insertions(+), 29 deletions(-) > > diff --git a/block/nvme.c b/block/nvme.c > index 919e14cef9..c3d3b99d1f 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -1200,36 +1200,26 @@ fail: > > typedef struct { > Coroutine *co; > - bool skip_yield; > int ret; > + AioContext *ctx; > } NVMeCoData; > > +static void nvme_rw_cb_bh(void *opaque) > +{ > + NVMeCoData *data = opaque; > + qemu_coroutine_enter(data->co); > +} > + > /* Put into NVMeRequest.cb, so runs in the BDS's main AioContext */ > static void nvme_rw_cb(void *opaque, int ret) > { > NVMeCoData *data = opaque; > - > data->ret = ret; > - > - if (data->co == qemu_coroutine_self()) { > - /* > - * Fast path: We are inside of the request coroutine (through > - * nvme_submit_command, nvme_deferred_fn, nvme_process_completion). > - * We can set data->skip_yield here to keep the coroutine from > - * yielding, and then we don't need to schedule a BH to wake it. > - */ > - data->skip_yield = true; > - } else { > - /* > - * Safe to call: The case where we run in the request coroutine is > - * handled above, so we must be independent of it; and without > - * skip_yield set, the coroutine will yield. > - * No need to release NVMeQueuePair.lock (we are called without it > - * held). (Note: If we enter the coroutine here, @data will > - * probably be dangling once aio_co_wake() returns.) > - */ > - aio_co_wake(data->co); > + if (!data->co) { > + /* The rw coroutine hasn't yielded, don't try to enter. */ > + return; > } > + replay_bh_schedule_oneshot_event(data->ctx, nvme_rw_cb_bh, data); > } > > static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs, > @@ -1253,7 +1243,7 @@ static coroutine_fn int > nvme_co_prw_aligned(BlockDriverState *bs, > .cdw12 = cpu_to_le32(cdw12), > }; > NVMeCoData data = { > - .co = qemu_coroutine_self(), > + .ctx = bdrv_get_aio_context(bs), > .ret = -EINPROGRESS, > }; > > @@ -1270,7 +1260,9 @@ static coroutine_fn int > nvme_co_prw_aligned(BlockDriverState *bs, > return r; > } > nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data); > - if (!data.skip_yield) { > + > + data.co = qemu_coroutine_self(); > + while (data.ret == -EINPROGRESS) { > qemu_coroutine_yield(); > } > > @@ -1366,7 +1358,7 @@ static coroutine_fn int nvme_co_flush(BlockDriverState > *bs) > .nsid = cpu_to_le32(s->nsid), > }; > NVMeCoData data = { > - .co = qemu_coroutine_self(), > + .ctx = bdrv_get_aio_context(bs), > .ret = -EINPROGRESS, > }; > > @@ -1374,7 +1366,9 @@ static coroutine_fn int nvme_co_flush(BlockDriverState > *bs) > req = nvme_get_free_req(ioq); > assert(req); > nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data); > - if (!data.skip_yield) { > + > + data.co = qemu_coroutine_self(); > + if (data.ret == -EINPROGRESS) { > qemu_coroutine_yield(); > } > > @@ -1415,7 +1409,7 @@ static coroutine_fn int > nvme_co_pwrite_zeroes(BlockDriverState *bs, > }; > > NVMeCoData data = { > - .co = qemu_coroutine_self(), > + .ctx = bdrv_get_aio_context(bs), > .ret = -EINPROGRESS, > }; > > @@ -1435,7 +1429,9 @@ static coroutine_fn int > nvme_co_pwrite_zeroes(BlockDriverState *bs, > assert(req); > > nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data); > - if (!data.skip_yield) { > + > + data.co = qemu_coroutine_self(); > + while (data.ret == -EINPROGRESS) { > qemu_coroutine_yield(); > } > > @@ -1463,7 +1459,7 @@ static int coroutine_fn > nvme_co_pdiscard(BlockDriverState *bs, > }; > > NVMeCoData data = { > - .co = qemu_coroutine_self(), > + .ctx = bdrv_get_aio_context(bs), > .ret = -EINPROGRESS, > }; > > @@ -1508,7 +1504,9 @@ static int coroutine_fn > nvme_co_pdiscard(BlockDriverState *bs, > trace_nvme_dsm(s, offset, bytes); > > nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data); > - if (!data.skip_yield) { > + > + data.co = qemu_coroutine_self(); > + while (data.ret == -EINPROGRESS) { > qemu_coroutine_yield(); > } >
OpenPGP_0x26B362E47FCF22C1.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
