aboutsummaryrefslogtreecommitdiff
path: root/hw/nvme
diff options
context:
space:
mode:
authorKlaus Jensen <k.jensen@samsung.com>2022-11-10 07:59:40 +0100
committerKlaus Jensen <k.jensen@samsung.com>2022-12-01 08:44:16 +0100
commit433c71e494ec66a7455b8ef2e6b2b42118426e50 (patch)
treee64647d336ce7cbad6c3b33d09d4bce2ea8bb56e /hw/nvme
parentc4ffd91aba1c3d878e99a3e7ba8aad4826728ece (diff)
downloadqemu-433c71e494ec66a7455b8ef2e6b2b42118426e50.zip
qemu-433c71e494ec66a7455b8ef2e6b2b42118426e50.tar.gz
qemu-433c71e494ec66a7455b8ef2e6b2b42118426e50.tar.bz2
hw/nvme: fix aio cancel in format
There are several bugs in the async cancel code for the Format command. Firstly, cancelling a format operation neglects to set iocb->ret as well as clearing the iocb->aiocb after cancelling the underlying aiocb which causes the aio callback to ignore the cancellation. Trivial fix. Secondly, and worse, because the request is queued up for posting to the CQ in a bottom half, if the cancellation is due to the submission queue being deleted (which calls blk_aio_cancel), the req structure is deallocated in nvme_del_sq prior to the bottom half being schedulued. Fix this by simply removing the bottom half, there is no reason to defer it anyway. Fixes: 3bcf26d3d619 ("hw/nvme: reimplement format nvm to allow cancellation") Reported-by: Jonathan Derrick <jonathan.derrick@linux.dev> Reviewed-by: Keith Busch <kbusch@kernel.org> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Diffstat (limited to 'hw/nvme')
-rw-r--r--hw/nvme/ctrl.c28
1 files changed, 12 insertions, 16 deletions
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index ac3885c..9bc5607 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5741,7 +5741,6 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
typedef struct NvmeFormatAIOCB {
BlockAIOCB common;
BlockAIOCB *aiocb;
- QEMUBH *bh;
NvmeRequest *req;
int ret;
@@ -5756,14 +5755,15 @@ typedef struct NvmeFormatAIOCB {
uint8_t pil;
} NvmeFormatAIOCB;
-static void nvme_format_bh(void *opaque);
-
static void nvme_format_cancel(BlockAIOCB *aiocb)
{
NvmeFormatAIOCB *iocb = container_of(aiocb, NvmeFormatAIOCB, common);
+ iocb->ret = -ECANCELED;
+
if (iocb->aiocb) {
blk_aio_cancel_async(iocb->aiocb);
+ iocb->aiocb = NULL;
}
}
@@ -5787,13 +5787,17 @@ static void nvme_format_set(NvmeNamespace *ns, uint8_t lbaf, uint8_t mset,
nvme_ns_init_format(ns);
}
+static void nvme_do_format(NvmeFormatAIOCB *iocb);
+
static void nvme_format_ns_cb(void *opaque, int ret)
{
NvmeFormatAIOCB *iocb = opaque;
NvmeNamespace *ns = iocb->ns;
int bytes;
- if (ret < 0) {
+ if (iocb->ret < 0) {
+ goto done;
+ } else if (ret < 0) {
iocb->ret = ret;
goto done;
}
@@ -5817,8 +5821,7 @@ static void nvme_format_ns_cb(void *opaque, int ret)
iocb->offset = 0;
done:
- iocb->aiocb = NULL;
- qemu_bh_schedule(iocb->bh);
+ nvme_do_format(iocb);
}
static uint16_t nvme_format_check(NvmeNamespace *ns, uint8_t lbaf, uint8_t pi)
@@ -5842,9 +5845,8 @@ static uint16_t nvme_format_check(NvmeNamespace *ns, uint8_t lbaf, uint8_t pi)
return NVME_SUCCESS;
}
-static void nvme_format_bh(void *opaque)
+static void nvme_do_format(NvmeFormatAIOCB *iocb)
{
- NvmeFormatAIOCB *iocb = opaque;
NvmeRequest *req = iocb->req;
NvmeCtrl *n = nvme_ctrl(req);
uint32_t dw10 = le32_to_cpu(req->cmd.cdw10);
@@ -5882,11 +5884,7 @@ static void nvme_format_bh(void *opaque)
return;
done:
- qemu_bh_delete(iocb->bh);
- iocb->bh = NULL;
-
iocb->common.cb(iocb->common.opaque, iocb->ret);
-
qemu_aio_unref(iocb);
}
@@ -5905,7 +5903,6 @@ static uint16_t nvme_format(NvmeCtrl *n, NvmeRequest *req)
iocb = qemu_aio_get(&nvme_format_aiocb_info, NULL, nvme_misc_cb, req);
iocb->req = req;
- iocb->bh = qemu_bh_new(nvme_format_bh, iocb);
iocb->ret = 0;
iocb->ns = NULL;
iocb->nsid = 0;
@@ -5934,14 +5931,13 @@ static uint16_t nvme_format(NvmeCtrl *n, NvmeRequest *req)
}
req->aiocb = &iocb->common;
- qemu_bh_schedule(iocb->bh);
+ nvme_do_format(iocb);
return NVME_NO_COMPLETE;
out:
- qemu_bh_delete(iocb->bh);
- iocb->bh = NULL;
qemu_aio_unref(iocb);
+
return status;
}