From 0f0d2c876c96d4908a9ef40959a44bec21bdd6cf Mon Sep 17 00:00:00 2001 From: Minwoo Im Date: Thu, 5 Nov 2020 23:28:47 +0900 Subject: nvme: free sq/cq dbbuf pointers when dbbuf set fails If Doorbell Buffer Config command fails even 'dev->dbbuf_dbs != NULL' which means OACS indicates that NVME_CTRL_OACS_DBBUF_SUPP is set, nvme_dbbuf_update_and_check_event() will check event even it's not been successfully set. This patch fixes mismatch among dbbuf for sq/cqs in case that dbbuf command fails. Signed-off-by: Minwoo Im Signed-off-by: Christoph Hellwig --- drivers/nvme/host/pci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'drivers') diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 0578ff253c47..3be352403839 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -292,9 +292,21 @@ static void nvme_dbbuf_init(struct nvme_dev *dev, nvmeq->dbbuf_cq_ei = &dev->dbbuf_eis[cq_idx(qid, dev->db_stride)]; } +static void nvme_dbbuf_free(struct nvme_queue *nvmeq) +{ + if (!nvmeq->qid) + return; + + nvmeq->dbbuf_sq_db = NULL; + nvmeq->dbbuf_cq_db = NULL; + nvmeq->dbbuf_sq_ei = NULL; + nvmeq->dbbuf_cq_ei = NULL; +} + static void nvme_dbbuf_set(struct nvme_dev *dev) { struct nvme_command c; + unsigned int i; if (!dev->dbbuf_dbs) return; @@ -308,6 +320,9 @@ static void nvme_dbbuf_set(struct nvme_dev *dev) dev_warn(dev->ctrl.device, "unable to set dbbuf\n"); /* Free memory and continue on */ nvme_dbbuf_dma_free(dev); + + for (i = 1; i <= dev->online_queues; i++) + nvme_dbbuf_free(&dev->queues[i]); } } -- cgit v1.2.3 From f6224b8681326856937420e1db18564a934bf32b Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Fri, 13 Nov 2020 10:28:30 -0800 Subject: nvme: directly cache command effects log Remove the struct used for tracking known command effects logs in a list. This is now saved in an xarray that doesn't use these elements. Instead, store the log directly instead of the wrapper struct. Signed-off-by: Keith Busch Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 9 ++++----- drivers/nvme/host/nvme.h | 6 ------ 2 files changed, 4 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 9b01afcb7777..2f96675cc0cf 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2929,7 +2929,7 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi, static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, struct nvme_effects_log **log) { - struct nvme_cel *cel = xa_load(&ctrl->cels, csi); + struct nvme_effects_log *cel = xa_load(&ctrl->cels, csi); int ret; if (cel) @@ -2940,16 +2940,15 @@ static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, return -ENOMEM; ret = nvme_get_log(ctrl, 0x00, NVME_LOG_CMD_EFFECTS, 0, csi, - &cel->log, sizeof(cel->log), 0); + cel, sizeof(*cel), 0); if (ret) { kfree(cel); return ret; } - cel->csi = csi; - xa_store(&ctrl->cels, cel->csi, cel, GFP_KERNEL); + xa_store(&ctrl->cels, csi, cel, GFP_KERNEL); out: - *log = &cel->log; + *log = cel; return 0; } diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index bc330bf0d3bd..567f7ad18a91 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -226,12 +226,6 @@ struct nvme_fault_inject { #endif }; -struct nvme_cel { - struct list_head entry; - struct nvme_effects_log log; - u8 csi; -}; - struct nvme_ctrl { bool comp_seen; enum nvme_ctrl_state state; -- cgit v1.2.3 From 8168d23fbcee4f9f6c5a1ce8650417f09aef70eb Mon Sep 17 00:00:00 2001 From: Keith Busch Date: Fri, 13 Nov 2020 10:45:45 -0800 Subject: nvme: fix memory leak freeing command effects xa_destroy() frees only internal data. The caller is responsible for freeing the exteranl objects referenced by an xarray. Fixes: 1cf7a12e09aa4 ("nvme: use an xarray to lookup the Commands Supported and Effects log") Signed-off-by: Keith Busch Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 2f96675cc0cf..9a270e49df17 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4373,6 +4373,19 @@ void nvme_uninit_ctrl(struct nvme_ctrl *ctrl) } EXPORT_SYMBOL_GPL(nvme_uninit_ctrl); +static void nvme_free_cels(struct nvme_ctrl *ctrl) +{ + struct nvme_effects_log *cel; + unsigned long i; + + xa_for_each (&ctrl->cels, i, cel) { + xa_erase(&ctrl->cels, i); + kfree(cel); + } + + xa_destroy(&ctrl->cels); +} + static void nvme_free_ctrl(struct device *dev) { struct nvme_ctrl *ctrl = @@ -4382,8 +4395,7 @@ static void nvme_free_ctrl(struct device *dev) if (!subsys || ctrl->instance != subsys->instance) ida_simple_remove(&nvme_instance_ida, ctrl->instance); - xa_destroy(&ctrl->cels); - + nvme_free_cels(ctrl); nvme_mpath_uninit(ctrl); __free_page(ctrl->discard_page); -- cgit v1.2.3 From 6f117cb854a44a79898d844e6ae3fd23bd94e786 Mon Sep 17 00:00:00 2001 From: Stefan Haberland Date: Mon, 16 Nov 2020 16:23:47 +0100 Subject: s390/dasd: fix null pointer dereference for ERP requests When requeueing all requests on the device request queue to the blocklayer we might get to an ERP (error recovery) request that is a copy of an original CQR. Those requests do not have blocklayer request information or a pointer to the dasd_queue set. When trying to access those data it will lead to a null pointer dereference in dasd_requeue_all_requests(). Fix by checking if the request is an ERP request that can simply be ignored. The blocklayer request will be requeued by the original CQR that is on the device queue right behind the ERP request. Fixes: 9487cfd3430d ("s390/dasd: fix handling of internal requests") Cc: #4.16 Signed-off-by: Stefan Haberland Reviewed-by: Jan Hoeppner Signed-off-by: Jens Axboe --- drivers/s390/block/dasd.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers') diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index eb17fea8075c..217a7b84abdf 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -2980,6 +2980,12 @@ static int _dasd_requeue_request(struct dasd_ccw_req *cqr) if (!block) return -EINVAL; + /* + * If the request is an ERP request there is nothing to requeue. + * This will be done with the remaining original request. + */ + if (cqr->refers) + return 0; spin_lock_irq(&cqr->dq->lock); req = (struct request *) cqr->callback_data; blk_mq_requeue_request(req, false); -- cgit v1.2.3