From 52abfcbd57eefdd54737fc8c2dc79d8f46d4a3e5 Mon Sep 17 00:00:00 2001
From: Gabriel Krisman Bertazi <krisman@collabora.com>
Date: Thu, 22 Oct 2020 16:58:41 -0400
Subject: [PATCH 01/17] blk-cgroup: Fix memleak on error path
If new_blkg allocation raced with blk_policy change and
blkg_lookup_check fails, new_blkg is leaked.
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/blk-cgroup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index f9b55614d67d..f9389b7cf823 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -663,6 +663,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
blkg = blkg_lookup_check(pos, pol, q);
if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
+ blkg_free(new_blkg);
goto fail_unlock;
}
From f255c19b3ab46d3cad3b1b2e1036f4c926cb1d0c Mon Sep 17 00:00:00 2001
From: Gabriel Krisman Bertazi <krisman@collabora.com>
Date: Thu, 22 Oct 2020 16:58:42 -0400
Subject: [PATCH 02/17] blk-cgroup: Pre-allocate tree node on blkg_conf_prep
Similarly to commit 457e490f2b741 ("blkcg: allocate struct blkcg_gq
outside request queue spinlock"), blkg_create can also trigger
occasional -ENOMEM failures at the radix insertion because any
allocation inside blkg_create has to be non-blocking, making it more
likely to fail. This causes trouble for userspace tools trying to
configure io weights who need to deal with this condition.
This patch reduces the occurrence of -ENOMEMs on this path by preloading
the radix tree element on a GFP_KERNEL context, such that we guarantee
the later non-blocking insertion won't fail.
A similar solution exists in blkcg_init_queue for the same situation.
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/blk-cgroup.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index f9389b7cf823..c68bdf58c9a6 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -657,6 +657,12 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
goto fail;
}
+ if (radix_tree_preload(GFP_KERNEL)) {
+ blkg_free(new_blkg);
+ ret = -ENOMEM;
+ goto fail;
+ }
+
rcu_read_lock();
spin_lock_irq(&q->queue_lock);
@@ -664,7 +670,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
blkg_free(new_blkg);
- goto fail_unlock;
+ goto fail_preloaded;
}
if (blkg) {
@@ -673,10 +679,12 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
blkg = blkg_create(pos, q, new_blkg);
if (IS_ERR(blkg)) {
ret = PTR_ERR(blkg);
- goto fail_unlock;
+ goto fail_preloaded;
}
}
+ radix_tree_preload_end();
+
if (pos == blkcg)
goto success;
}
@@ -686,6 +694,8 @@ success:
ctx->body = input;
return 0;
+fail_preloaded:
+ radix_tree_preload_end();
fail_unlock:
spin_unlock_irq(&q->queue_lock);
rcu_read_unlock();
From 8685699c28d7452ff06d62b4692df985eb7301f0 Mon Sep 17 00:00:00 2001
From: Keith Busch <kbusch@kernel.org>
Date: Fri, 23 Oct 2020 12:16:28 -0700
Subject: [PATCH 03/17] nvme: ignore zone validate errors on subsequent scans
Revalidating nvme zoned namespaces requires IO commands, and there are
controller states that prevent IO. For example, a sanitize in progress
is required to fail all IO, but we don't want to remove a namespace
we've previously added just because the controller is in such a state.
Suppress the error in this case.
Reported-by: Michael Nguyen <michael.nguyen@wdc.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 95ef4943d8bd..376096bfc54a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2125,7 +2125,7 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
if (blk_queue_is_zoned(ns->queue)) {
ret = nvme_revalidate_zones(ns);
- if (ret)
+ if (ret && !nvme_first_scan(ns->disk))
return ret;
}
From 25c1ca6ecaba3b751d3f7ff92d5cddff3b05f8d0 Mon Sep 17 00:00:00 2001
From: zhenwei pi <pizhenwei@bytedance.com>
Date: Sun, 25 Oct 2020 19:51:24 +0800
Subject: [PATCH 04/17] nvme-rdma: handle unexpected nvme completion data
length
Receiving a zero length message leads to the following warnings because
the CQE is processed twice:
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 0 at lib/refcount.c:28
RIP: 0010:refcount_warn_saturate+0xd9/0xe0
Call Trace:
<IRQ>
nvme_rdma_recv_done+0xf3/0x280 [nvme_rdma]
__ib_process_cq+0x76/0x150 [ib_core]
...
Sanity check the received data length, to avoids this.
Thanks to Chao Leng & Sagi for suggestions.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/rdma.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index aad829a2b50d..40a0a3b6476c 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1768,6 +1768,14 @@ static void nvme_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc)
return;
}
+ /* sanity checking for received data length */
+ if (unlikely(wc->byte_len < len)) {
+ dev_err(queue->ctrl->ctrl.device,
+ "Unexpected nvme completion length(%d)\n", wc->byte_len);
+ nvme_rdma_error_recovery(queue->ctrl);
+ return;
+ }
+
ib_dma_sync_single_for_cpu(ibdev, qe->dma, len, DMA_FROM_DEVICE);
/*
* AEN requests are special as they don't time out and can
From caf1cbe3677576ef860f7bbba9074d0e6a0bcabb Mon Sep 17 00:00:00 2001
From: James Smart <james.smart@broadcom.com>
Date: Fri, 23 Oct 2020 15:26:04 -0700
Subject: [PATCH 05/17] nvme-fc: track error_recovery while connecting
Whenever there are errors during CONNECTING, the driver recovers by
aborting all outstanding ios and counts on the io completion to fail them
and thus the connection/association they are on. However, the connection
failure depends on a failure state from the core routines. Not all
commands that are issued by the core routine are guaranteed to cause a
failure of the core routine. They may be treated as a failure status and
the status is then ignored.
As such, whenever the transport enters error_recovery while CONNECTING,
it will set a new flag indicating an association failed. The
create_association routine which creates and initializes the controller,
will monitor the state of the flag as well as the core routine error
status and ensure the association fails if there was an error.
Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/fc.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 3c002bdcace3..cfb6ef74c742 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -146,7 +146,8 @@ struct nvme_fc_rport {
/* fc_ctrl flags values - specified as bit positions */
#define ASSOC_ACTIVE 0
-#define FCCTRL_TERMIO 1
+#define ASSOC_FAILED 1
+#define FCCTRL_TERMIO 2
struct nvme_fc_ctrl {
spinlock_t lock;
@@ -2988,6 +2989,8 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
ctrl->cnum, ctrl->lport->localport.port_name,
ctrl->rport->remoteport.port_name, ctrl->ctrl.opts->subsysnqn);
+ clear_bit(ASSOC_FAILED, &ctrl->flags);
+
/*
* Create the admin queue
*/
@@ -3016,7 +3019,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
*/
ret = nvme_enable_ctrl(&ctrl->ctrl);
- if (ret)
+ if (ret || test_bit(ASSOC_FAILED, &ctrl->flags))
goto out_disconnect_admin_queue;
ctrl->ctrl.max_segments = ctrl->lport->ops->max_sgl_segments;
@@ -3026,7 +3029,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
ret = nvme_init_identify(&ctrl->ctrl);
- if (ret)
+ if (ret || test_bit(ASSOC_FAILED, &ctrl->flags))
goto out_disconnect_admin_queue;
/* sanity checks */
@@ -3071,9 +3074,9 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
ret = nvme_fc_create_io_queues(ctrl);
else
ret = nvme_fc_recreate_io_queues(ctrl);
- if (ret)
- goto out_term_aen_ops;
}
+ if (ret || test_bit(ASSOC_FAILED, &ctrl->flags))
+ goto out_term_aen_ops;
changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
@@ -3301,6 +3304,7 @@ __nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl)
*/
if (ctrl->ctrl.state == NVME_CTRL_CONNECTING) {
__nvme_fc_abort_outstanding_ios(ctrl, true);
+ set_bit(ASSOC_FAILED, &ctrl->flags);
return;
}
From 9c2bb2577d81b1a09f7e342e947986e55cad18e3 Mon Sep 17 00:00:00 2001
From: James Smart <james.smart@broadcom.com>
Date: Fri, 23 Oct 2020 15:27:50 -0700
Subject: [PATCH 06/17] nvme-fc: remove err_work work item
err_work was created to handle errors (mainly I/O timeouts) while in
CONNECTING state. The flag for err_work_active is also unneeded.
Remove err_work_active and err_work. The actions to abort I/Os are moved
inline to nvme_error_recovery().
Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/fc.c | 40 ++++++++++------------------------------
1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index cfb6ef74c742..9cdb116b545b 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -158,7 +158,6 @@ struct nvme_fc_ctrl {
u32 cnum;
bool ioq_live;
- atomic_t err_work_active;
u64 association_id;
struct nvmefc_ls_rcv_op *rcv_disconn;
@@ -168,7 +167,6 @@ struct nvme_fc_ctrl {
struct blk_mq_tag_set tag_set;
struct delayed_work connect_work;
- struct work_struct err_work;
struct kref ref;
unsigned long flags;
@@ -2415,11 +2413,11 @@ nvme_fc_nvme_ctrl_freed(struct nvme_ctrl *nctrl)
nvme_fc_ctrl_put(ctrl);
}
+static void __nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl);
+
static void
nvme_fc_error_recovery(struct nvme_fc_ctrl *ctrl, char *errmsg)
{
- int active;
-
/*
* if an error (io timeout, etc) while (re)connecting,
* it's an error on creating the new association.
@@ -2428,11 +2426,14 @@ nvme_fc_error_recovery(struct nvme_fc_ctrl *ctrl, char *errmsg)
* ios hitting this path before things are cleaned up.
*/
if (ctrl->ctrl.state == NVME_CTRL_CONNECTING) {
- active = atomic_xchg(&ctrl->err_work_active, 1);
- if (!active && !queue_work(nvme_fc_wq, &ctrl->err_work)) {
- atomic_set(&ctrl->err_work_active, 0);
- WARN_ON(1);
- }
+ __nvme_fc_terminate_io(ctrl);
+
+ /*
+ * Rescheduling the connection after recovering
+ * from the io error is left to the reconnect work
+ * item, which is what should have stalled waiting on
+ * the io that had the error that scheduled this work.
+ */
return;
}
@@ -3240,7 +3241,6 @@ nvme_fc_delete_ctrl(struct nvme_ctrl *nctrl)
{
struct nvme_fc_ctrl *ctrl = to_fc_ctrl(nctrl);
- cancel_work_sync(&ctrl->err_work);
cancel_delayed_work_sync(&ctrl->connect_work);
/*
* kill the association on the link side. this will block
@@ -3351,23 +3351,6 @@ nvme_fc_reset_ctrl_work(struct work_struct *work)
ctrl->cnum);
}
-static void
-nvme_fc_connect_err_work(struct work_struct *work)
-{
- struct nvme_fc_ctrl *ctrl =
- container_of(work, struct nvme_fc_ctrl, err_work);
-
- __nvme_fc_terminate_io(ctrl);
-
- atomic_set(&ctrl->err_work_active, 0);
-
- /*
- * Rescheduling the connection after recovering
- * from the io error is left to the reconnect work
- * item, which is what should have stalled waiting on
- * the io that had the error that scheduled this work.
- */
-}
static const struct nvme_ctrl_ops nvme_fc_ctrl_ops = {
.name = "fc",
@@ -3495,7 +3478,6 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
ctrl->dev = lport->dev;
ctrl->cnum = idx;
ctrl->ioq_live = false;
- atomic_set(&ctrl->err_work_active, 0);
init_waitqueue_head(&ctrl->ioabort_wait);
get_device(ctrl->dev);
@@ -3503,7 +3485,6 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
INIT_WORK(&ctrl->ctrl.reset_work, nvme_fc_reset_ctrl_work);
INIT_DELAYED_WORK(&ctrl->connect_work, nvme_fc_connect_ctrl_work);
- INIT_WORK(&ctrl->err_work, nvme_fc_connect_err_work);
spin_lock_init(&ctrl->lock);
/* io queue count */
@@ -3596,7 +3577,6 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
fail_ctrl:
nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_DELETING);
cancel_work_sync(&ctrl->ctrl.reset_work);
- cancel_work_sync(&ctrl->err_work);
cancel_delayed_work_sync(&ctrl->connect_work);
ctrl->ctrl.opts = NULL;
From 95ced8a2c72dd48167adc4981c3c465c491f2f02 Mon Sep 17 00:00:00 2001
From: James Smart <james.smart@broadcom.com>
Date: Fri, 23 Oct 2020 15:27:51 -0700
Subject: [PATCH 07/17] nvme-fc: eliminate terminate_io use by
nvme_fc_error_recovery
nvme_fc_error_recovery() special cases handling when in CONNECTING state
and calls __nvme_fc_terminate_io(). __nvme_fc_terminate_io() itself
special cases CONNECTING state and calls the routine to abort outstanding
ios.
Simplify the sequence by putting the call to abort outstanding I/Os
directly in nvme_fc_error_recovery.
Move the location of __nvme_fc_abort_outstanding_ios(), and
nvme_fc_terminate_exchange() which is called by it, to avoid adding
function prototypes for nvme_fc_error_recovery().
Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/fc.c | 187 ++++++++++++++++++-----------------------
1 file changed, 84 insertions(+), 103 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 9cdb116b545b..ffbfb0533cac 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2413,27 +2413,97 @@ nvme_fc_nvme_ctrl_freed(struct nvme_ctrl *nctrl)
nvme_fc_ctrl_put(ctrl);
}
-static void __nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl);
+/*
+ * This routine is used by the transport when it needs to find active
+ * io on a queue that is to be terminated. The transport uses
+ * blk_mq_tagset_busy_itr() to find the busy requests, which then invoke
+ * this routine to kill them on a 1 by 1 basis.
+ *
+ * As FC allocates FC exchange for each io, the transport must contact
+ * the LLDD to terminate the exchange, thus releasing the FC exchange.
+ * After terminating the exchange the LLDD will call the transport's
+ * normal io done path for the request, but it will have an aborted
+ * status. The done path will return the io request back to the block
+ * layer with an error status.
+ */
+static bool
+nvme_fc_terminate_exchange(struct request *req, void *data, bool reserved)
+{
+ struct nvme_ctrl *nctrl = data;
+ struct nvme_fc_ctrl *ctrl = to_fc_ctrl(nctrl);
+ struct nvme_fc_fcp_op *op = blk_mq_rq_to_pdu(req);
+
+ __nvme_fc_abort_op(ctrl, op);
+ return true;
+}
+
+/*
+ * This routine runs through all outstanding commands on the association
+ * and aborts them. This routine is typically be called by the
+ * delete_association routine. It is also called due to an error during
+ * reconnect. In that scenario, it is most likely a command that initializes
+ * the controller, including fabric Connect commands on io queues, that
+ * may have timed out or failed thus the io must be killed for the connect
+ * thread to see the error.
+ */
+static void
+__nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
+{
+ /*
+ * If io queues are present, stop them and terminate all outstanding
+ * ios on them. As FC allocates FC exchange for each io, the
+ * transport must contact the LLDD to terminate the exchange,
+ * thus releasing the FC exchange. We use blk_mq_tagset_busy_itr()
+ * to tell us what io's are busy and invoke a transport routine
+ * to kill them with the LLDD. After terminating the exchange
+ * the LLDD will call the transport's normal io done path, but it
+ * will have an aborted status. The done path will return the
+ * io requests back to the block layer as part of normal completions
+ * (but with error status).
+ */
+ if (ctrl->ctrl.queue_count > 1) {
+ nvme_stop_queues(&ctrl->ctrl);
+ blk_mq_tagset_busy_iter(&ctrl->tag_set,
+ nvme_fc_terminate_exchange, &ctrl->ctrl);
+ blk_mq_tagset_wait_completed_request(&ctrl->tag_set);
+ if (start_queues)
+ nvme_start_queues(&ctrl->ctrl);
+ }
+
+ /*
+ * Other transports, which don't have link-level contexts bound
+ * to sqe's, would try to gracefully shutdown the controller by
+ * writing the registers for shutdown and polling (call
+ * nvme_shutdown_ctrl()). Given a bunch of i/o was potentially
+ * just aborted and we will wait on those contexts, and given
+ * there was no indication of how live the controlelr is on the
+ * link, don't send more io to create more contexts for the
+ * shutdown. Let the controller fail via keepalive failure if
+ * its still present.
+ */
+
+ /*
+ * clean up the admin queue. Same thing as above.
+ */
+ blk_mq_quiesce_queue(ctrl->ctrl.admin_q);
+ blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
+ nvme_fc_terminate_exchange, &ctrl->ctrl);
+ blk_mq_tagset_wait_completed_request(&ctrl->admin_tag_set);
+}
static void
nvme_fc_error_recovery(struct nvme_fc_ctrl *ctrl, char *errmsg)
{
/*
- * if an error (io timeout, etc) while (re)connecting,
- * it's an error on creating the new association.
- * Start the error recovery thread if it hasn't already
- * been started. It is expected there could be multiple
- * ios hitting this path before things are cleaned up.
+ * if an error (io timeout, etc) while (re)connecting, the remote
+ * port requested terminating of the association (disconnect_ls)
+ * or an error (timeout or abort) occurred on an io while creating
+ * the controller. Abort any ios on the association and let the
+ * create_association error path resolve things.
*/
if (ctrl->ctrl.state == NVME_CTRL_CONNECTING) {
- __nvme_fc_terminate_io(ctrl);
-
- /*
- * Rescheduling the connection after recovering
- * from the io error is left to the reconnect work
- * item, which is what should have stalled waiting on
- * the io that had the error that scheduled this work.
- */
+ __nvme_fc_abort_outstanding_ios(ctrl, true);
+ set_bit(ASSOC_FAILED, &ctrl->flags);
return;
}
@@ -2747,30 +2817,6 @@ nvme_fc_complete_rq(struct request *rq)
nvme_fc_ctrl_put(ctrl);
}
-/*
- * This routine is used by the transport when it needs to find active
- * io on a queue that is to be terminated. The transport uses
- * blk_mq_tagset_busy_itr() to find the busy requests, which then invoke
- * this routine to kill them on a 1 by 1 basis.
- *
- * As FC allocates FC exchange for each io, the transport must contact
- * the LLDD to terminate the exchange, thus releasing the FC exchange.
- * After terminating the exchange the LLDD will call the transport's
- * normal io done path for the request, but it will have an aborted
- * status. The done path will return the io request back to the block
- * layer with an error status.
- */
-static bool
-nvme_fc_terminate_exchange(struct request *req, void *data, bool reserved)
-{
- struct nvme_ctrl *nctrl = data;
- struct nvme_fc_ctrl *ctrl = to_fc_ctrl(nctrl);
- struct nvme_fc_fcp_op *op = blk_mq_rq_to_pdu(req);
-
- __nvme_fc_abort_op(ctrl, op);
- return true;
-}
-
static const struct blk_mq_ops nvme_fc_mq_ops = {
.queue_rq = nvme_fc_queue_rq,
@@ -3111,60 +3157,6 @@ out_free_queue:
}
-/*
- * This routine runs through all outstanding commands on the association
- * and aborts them. This routine is typically be called by the
- * delete_association routine. It is also called due to an error during
- * reconnect. In that scenario, it is most likely a command that initializes
- * the controller, including fabric Connect commands on io queues, that
- * may have timed out or failed thus the io must be killed for the connect
- * thread to see the error.
- */
-static void
-__nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
-{
- /*
- * If io queues are present, stop them and terminate all outstanding
- * ios on them. As FC allocates FC exchange for each io, the
- * transport must contact the LLDD to terminate the exchange,
- * thus releasing the FC exchange. We use blk_mq_tagset_busy_itr()
- * to tell us what io's are busy and invoke a transport routine
- * to kill them with the LLDD. After terminating the exchange
- * the LLDD will call the transport's normal io done path, but it
- * will have an aborted status. The done path will return the
- * io requests back to the block layer as part of normal completions
- * (but with error status).
- */
- if (ctrl->ctrl.queue_count > 1) {
- nvme_stop_queues(&ctrl->ctrl);
- blk_mq_tagset_busy_iter(&ctrl->tag_set,
- nvme_fc_terminate_exchange, &ctrl->ctrl);
- blk_mq_tagset_wait_completed_request(&ctrl->tag_set);
- if (start_queues)
- nvme_start_queues(&ctrl->ctrl);
- }
-
- /*
- * Other transports, which don't have link-level contexts bound
- * to sqe's, would try to gracefully shutdown the controller by
- * writing the registers for shutdown and polling (call
- * nvme_shutdown_ctrl()). Given a bunch of i/o was potentially
- * just aborted and we will wait on those contexts, and given
- * there was no indication of how live the controlelr is on the
- * link, don't send more io to create more contexts for the
- * shutdown. Let the controller fail via keepalive failure if
- * its still present.
- */
-
- /*
- * clean up the admin queue. Same thing as above.
- */
- blk_mq_quiesce_queue(ctrl->ctrl.admin_q);
- blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
- nvme_fc_terminate_exchange, &ctrl->ctrl);
- blk_mq_tagset_wait_completed_request(&ctrl->admin_tag_set);
-}
-
/*
* This routine stops operation of the controller on the host side.
* On the host os stack side: Admin and IO queues are stopped,
@@ -3297,17 +3289,6 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status)
static void
__nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl)
{
- /*
- * if state is CONNECTING - the error occurred as part of a
- * reconnect attempt. Abort any ios on the association and
- * let the create_association error paths resolve things.
- */
- if (ctrl->ctrl.state == NVME_CTRL_CONNECTING) {
- __nvme_fc_abort_outstanding_ios(ctrl, true);
- set_bit(ASSOC_FAILED, &ctrl->flags);
- return;
- }
-
/*
* For any other state, kill the association. As this routine
* is a common io abort routine for resetting and such, after
From ac9b820e713bdc8f340e7d65ca6a19b10b790f2a Mon Sep 17 00:00:00 2001
From: James Smart <james.smart@broadcom.com>
Date: Fri, 23 Oct 2020 15:27:52 -0700
Subject: [PATCH 08/17] nvme-fc: remove nvme_fc_terminate_io()
__nvme_fc_terminate_io() is now called by only 1 place, in reset_work.
Consoldate and move the functionality of terminate_io into reset_work.
In reset_work, rather than calling the create_association directly,
schedule the connect work element to do its thing. After scheduling,
flush the connect work element to continue with semantic of not
returning until connect has been attempted at least once.
Signed-off-by: James Smart <james.smart@broadcom.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/fc.c | 53 ++++++++++++++----------------------------
1 file changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index ffbfb0533cac..f4c246462658 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3286,50 +3286,33 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status)
}
}
-static void
-__nvme_fc_terminate_io(struct nvme_fc_ctrl *ctrl)
-{
- /*
- * For any other state, kill the association. As this routine
- * is a common io abort routine for resetting and such, after
- * the association is terminated, ensure that the state is set
- * to CONNECTING.
- */
-
- nvme_stop_keep_alive(&ctrl->ctrl);
-
- /* will block will waiting for io to terminate */
- nvme_fc_delete_association(ctrl);
-
- if (ctrl->ctrl.state != NVME_CTRL_CONNECTING &&
- !nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING))
- dev_err(ctrl->ctrl.device,
- "NVME-FC{%d}: error_recovery: Couldn't change state "
- "to CONNECTING\n", ctrl->cnum);
-}
-
static void
nvme_fc_reset_ctrl_work(struct work_struct *work)
{
struct nvme_fc_ctrl *ctrl =
container_of(work, struct nvme_fc_ctrl, ctrl.reset_work);
- int ret;
-
- __nvme_fc_terminate_io(ctrl);
nvme_stop_ctrl(&ctrl->ctrl);
- if (ctrl->rport->remoteport.port_state == FC_OBJSTATE_ONLINE)
- ret = nvme_fc_create_association(ctrl);
- else
- ret = -ENOTCONN;
+ /* will block will waiting for io to terminate */
+ nvme_fc_delete_association(ctrl);
- if (ret)
- nvme_fc_reconnect_or_delete(ctrl, ret);
- else
- dev_info(ctrl->ctrl.device,
- "NVME-FC{%d}: controller reset complete\n",
- ctrl->cnum);
+ if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING))
+ dev_err(ctrl->ctrl.device,
+ "NVME-FC{%d}: error_recovery: Couldn't change state "
+ "to CONNECTING\n", ctrl->cnum);
+
+ if (ctrl->rport->remoteport.port_state == FC_OBJSTATE_ONLINE) {
+ if (!queue_delayed_work(nvme_wq, &ctrl->connect_work, 0)) {
+ dev_err(ctrl->ctrl.device,
+ "NVME-FC{%d}: failed to schedule connect "
+ "after reset\n", ctrl->cnum);
+ } else {
+ flush_delayed_work(&ctrl->connect_work);
+ }
+ } else {
+ nvme_fc_reconnect_or_delete(ctrl, -ENOTCONN);
+ }
}
From 3c3751f2daf6675f6b5bee83b792354c272f5bd2 Mon Sep 17 00:00:00 2001
From: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Date: Thu, 22 Oct 2020 16:58:21 -0700
Subject: [PATCH 09/17] nvmet: fix a NULL pointer dereference when tracing the
flush command
When target side trace in turned on and flush command is issued from the
host it results in the following Oops.
[ 856.789724] BUG: kernel NULL pointer dereference, address: 0000000000000068
[ 856.790686] #PF: supervisor read access in kernel mode
[ 856.791262] #PF: error_code(0x0000) - not-present page
[ 856.791863] PGD 6d7110067 P4D 6d7110067 PUD 66f0ad067 PMD 0
[ 856.792527] Oops: 0000 [#1] SMP NOPTI
[ 856.792950] CPU: 15 PID: 7034 Comm: nvme Tainted: G OE 5.9.0nvme-5.9+ #71
[ 856.793790] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-59-gc9ba5276e3214
[ 856.794956] RIP: 0010:trace_event_raw_event_nvmet_req_init+0x13e/0x170 [nvmet]
[ 856.795734] Code: 41 5c 41 5d c3 31 d2 31 f6 e8 4e 9b b8 e0 e9 0e ff ff ff 49 8b 55 00 48 8b 38 8b 0
[ 856.797740] RSP: 0018:ffffc90001be3a60 EFLAGS: 00010246
[ 856.798375] RAX: 0000000000000000 RBX: ffff8887e7d2c01c RCX: 0000000000000000
[ 856.799234] RDX: 0000000000000020 RSI: 0000000057e70ea2 RDI: ffff8887e7d2c034
[ 856.800088] RBP: ffff88869f710578 R08: ffff888807500d40 R09: 00000000fffffffe
[ 856.800951] R10: 0000000064c66670 R11: 00000000ef955201 R12: ffff8887e7d2c034
[ 856.801807] R13: ffff88869f7105c8 R14: 0000000000000040 R15: ffff88869f710440
[ 856.802667] FS: 00007f6a22bd8780(0000) GS:ffff888813a00000(0000) knlGS:0000000000000000
[ 856.803635] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 856.804367] CR2: 0000000000000068 CR3: 00000006d73e0000 CR4: 00000000003506e0
[ 856.805283] Call Trace:
[ 856.805613] nvmet_req_init+0x27c/0x480 [nvmet]
[ 856.806200] nvme_loop_queue_rq+0xcb/0x1d0 [nvme_loop]
[ 856.806862] blk_mq_dispatch_rq_list+0x123/0x7b0
[ 856.807459] ? kvm_sched_clock_read+0x14/0x30
[ 856.808025] __blk_mq_sched_dispatch_requests+0xc7/0x170
[ 856.808708] blk_mq_sched_dispatch_requests+0x30/0x60
[ 856.809372] __blk_mq_run_hw_queue+0x70/0x100
[ 856.809935] __blk_mq_delay_run_hw_queue+0x156/0x170
[ 856.810574] blk_mq_run_hw_queue+0x86/0xe0
[ 856.811104] blk_mq_sched_insert_request+0xef/0x160
[ 856.811733] blk_execute_rq+0x69/0xc0
[ 856.812212] ? blk_mq_rq_ctx_init+0xd0/0x230
[ 856.812784] nvme_execute_passthru_rq+0x57/0x130 [nvme_core]
[ 856.813461] nvme_submit_user_cmd+0xeb/0x300 [nvme_core]
[ 856.814099] nvme_user_cmd.isra.82+0x11e/0x1a0 [nvme_core]
[ 856.814752] blkdev_ioctl+0x1dc/0x2c0
[ 856.815197] block_ioctl+0x3f/0x50
[ 856.815606] __x64_sys_ioctl+0x84/0xc0
[ 856.816074] do_syscall_64+0x33/0x40
[ 856.816533] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 856.817168] RIP: 0033:0x7f6a222ed107
[ 856.817617] Code: 44 00 00 48 8b 05 81 cd 2c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 8
[ 856.819901] RSP: 002b:00007ffca848f058 EFLAGS: 00000202 ORIG_RAX: 0000000000000010
[ 856.820846] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007f6a222ed107
[ 856.821726] RDX: 00007ffca848f060 RSI: 00000000c0484e43 RDI: 0000000000000003
[ 856.822603] RBP: 0000000000000003 R08: 000000000000003f R09: 0000000000000005
[ 856.823478] R10: 00007ffca848ece0 R11: 0000000000000202 R12: 00007ffca84912d3
[ 856.824359] R13: 00007ffca848f4d0 R14: 0000000000000002 R15: 000000000067e900
[ 856.825236] Modules linked in: nvme_loop(OE) nvmet(OE) nvme_fabrics(OE) null_blk nvme(OE) nvme_corel
Move the nvmet_req_init() tracepoint after we parse the command in
nvmet_req_init() so that we can get rid of the duplicate
nvmet_find_namespace() call.
Rename __assign_disk_name() -> __assign_req_name(). Now that we call
tracepoint after parsing the command simplify the newly added
__assign_req_name() which fixes this bug.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/target/core.c | 4 ++--
drivers/nvme/target/trace.h | 21 +++++++--------------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index aafcbc424b7a..957b39a82431 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -907,8 +907,6 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
req->error_loc = NVMET_NO_ERROR_LOC;
req->error_slba = 0;
- trace_nvmet_req_init(req, req->cmd);
-
/* no support for fused commands yet */
if (unlikely(flags & (NVME_CMD_FUSE_FIRST | NVME_CMD_FUSE_SECOND))) {
req->error_loc = offsetof(struct nvme_common_command, flags);
@@ -938,6 +936,8 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
if (status)
goto fail;
+ trace_nvmet_req_init(req, req->cmd);
+
if (unlikely(!percpu_ref_tryget_live(&sq->ref))) {
status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
goto fail;
diff --git a/drivers/nvme/target/trace.h b/drivers/nvme/target/trace.h
index 0458046d6501..c14e3249a14d 100644
--- a/drivers/nvme/target/trace.h
+++ b/drivers/nvme/target/trace.h
@@ -46,19 +46,12 @@ static inline struct nvmet_ctrl *nvmet_req_to_ctrl(struct nvmet_req *req)
return req->sq->ctrl;
}
-static inline void __assign_disk_name(char *name, struct nvmet_req *req,
- bool init)
+static inline void __assign_req_name(char *name, struct nvmet_req *req)
{
- struct nvmet_ctrl *ctrl = nvmet_req_to_ctrl(req);
- struct nvmet_ns *ns;
-
- if ((init && req->sq->qid) || (!init && req->cq->qid)) {
- ns = nvmet_find_namespace(ctrl, req->cmd->rw.nsid);
- strncpy(name, ns->device_path, DISK_NAME_LEN);
- return;
- }
-
- memset(name, 0, DISK_NAME_LEN);
+ if (req->ns)
+ strncpy(name, req->ns->device_path, DISK_NAME_LEN);
+ else
+ memset(name, 0, DISK_NAME_LEN);
}
#endif
@@ -81,7 +74,7 @@ TRACE_EVENT(nvmet_req_init,
TP_fast_assign(
__entry->cmd = cmd;
__entry->ctrl = nvmet_req_to_ctrl(req);
- __assign_disk_name(__entry->disk, req, true);
+ __assign_req_name(__entry->disk, req);
__entry->qid = req->sq->qid;
__entry->cid = cmd->common.command_id;
__entry->opcode = cmd->common.opcode;
@@ -121,7 +114,7 @@ TRACE_EVENT(nvmet_req_complete,
__entry->cid = req->cqe->command_id;
__entry->result = le64_to_cpu(req->cqe->result.u64);
__entry->status = le16_to_cpu(req->cqe->status) >> 1;
- __assign_disk_name(__entry->disk, req, false);
+ __assign_req_name(__entry->disk, req);
),
TP_printk("nvmet%s: %sqid=%d, cmdid=%u, res=%#llx, status=%#x",
__print_ctrl_name(__entry->ctrl),
From 35bc10b2eafbb701064b94f283b77c54d3304842 Mon Sep 17 00:00:00 2001
From: Kanchan Joshi <joshi.k@samsung.com>
Date: Mon, 28 Sep 2020 15:25:49 +0530
Subject: [PATCH 10/17] null_blk: synchronization fix for zoned device
Parallel write,read,zone-mgmt operations accessing/altering zone state
and write-pointer may get into race. Avoid the situation by using a new
spinlock for zoned device.
Concurrent zone-appends (on a zone) returning same write-pointer issue
is also avoided using this lock.
Cc: stable@vger.kernel.org
Fixes: e0489ed5daeb ("null_blk: Support REQ_OP_ZONE_APPEND")
Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/block/null_blk.h | 1 +
drivers/block/null_blk_zoned.c | 22 ++++++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index d2e7db43a52a..3176b269b822 100644
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -47,6 +47,7 @@ struct nullb_device {
unsigned int nr_zones_closed;
struct blk_zone *zones;
sector_t zone_size_sects;
+ spinlock_t zone_lock;
unsigned long size; /* device size in MB */
unsigned long completion_nsec; /* time in ns to complete a request */
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index 7d94f2d47a6a..98056c88926b 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -45,6 +45,7 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
if (!dev->zones)
return -ENOMEM;
+ spin_lock_init(&dev->zone_lock);
if (dev->zone_nr_conv >= dev->nr_zones) {
dev->zone_nr_conv = dev->nr_zones - 1;
pr_info("changed the number of conventional zones to %u",
@@ -149,8 +150,11 @@ int null_report_zones(struct gendisk *disk, sector_t sector,
* So use a local copy to avoid corruption of the device zone
* array.
*/
+ spin_lock_irq(&dev->zone_lock);
memcpy(&zone, &dev->zones[first_zone + i],
sizeof(struct blk_zone));
+ spin_unlock_irq(&dev->zone_lock);
+
error = cb(&zone, i, data);
if (error)
return error;
@@ -499,18 +503,28 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
blk_status_t null_process_zoned_cmd(struct nullb_cmd *cmd, enum req_opf op,
sector_t sector, sector_t nr_sectors)
{
+ blk_status_t sts;
+ struct nullb_device *dev = cmd->nq->dev;
+
+ spin_lock_irq(&dev->zone_lock);
switch (op) {
case REQ_OP_WRITE:
- return null_zone_write(cmd, sector, nr_sectors, false);
+ sts = null_zone_write(cmd, sector, nr_sectors, false);
+ break;
case REQ_OP_ZONE_APPEND:
- return null_zone_write(cmd, sector, nr_sectors, true);
+ sts = null_zone_write(cmd, sector, nr_sectors, true);
+ break;
case REQ_OP_ZONE_RESET:
case REQ_OP_ZONE_RESET_ALL:
case REQ_OP_ZONE_OPEN:
case REQ_OP_ZONE_CLOSE:
case REQ_OP_ZONE_FINISH:
- return null_zone_mgmt(cmd, op, sector);
+ sts = null_zone_mgmt(cmd, op, sector);
+ break;
default:
- return null_process_cmd(cmd, op, sector, nr_sectors);
+ sts = null_process_cmd(cmd, op, sector, nr_sectors);
}
+ spin_unlock_irq(&dev->zone_lock);
+
+ return sts;
}
From 4977d121bc9bc5138d4d48b85469123001859573 Mon Sep 17 00:00:00 2001
From: Naohiro Aota <naohiro.aota@wdc.com>
Date: Wed, 28 Oct 2020 16:25:36 +0900
Subject: [PATCH 11/17] block: advance iov_iter on bio_add_hw_page failure
When the bio's size reaches max_append_sectors, bio_add_hw_page returns
0 then __bio_iov_append_get_pages returns -EINVAL. This is an expected
result of building a small enough bio not to be split in the IO path.
However, iov_iter is not advanced in this case, causing the same pages
are filled for the bio again and again.
Fix the case by properly advancing the iov_iter for already processed
pages.
Fixes: 0512a75b98f8 ("block: Introduce REQ_OP_ZONE_APPEND")
Cc: stable@vger.kernel.org # 5.8+
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/bio.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index e6e26d7a1ffb..fa01bef35bb1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1044,6 +1044,7 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
ssize_t size, left;
unsigned len, i;
size_t offset;
+ int ret = 0;
if (WARN_ON_ONCE(!max_append_sectors))
return 0;
@@ -1066,15 +1067,17 @@ static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
len = min_t(size_t, PAGE_SIZE - offset, left);
if (bio_add_hw_page(q, bio, page, len, offset,
- max_append_sectors, &same_page) != len)
- return -EINVAL;
+ max_append_sectors, &same_page) != len) {
+ ret = -EINVAL;
+ break;
+ }
if (same_page)
put_page(page);
offset = 0;
}
- iov_iter_advance(iter, size);
- return 0;
+ iov_iter_advance(iter, size - left);
+ return ret;
}
/**
From b40813ddcd6bf9f01d020804e4cb8febc480b9e4 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Wed, 28 Oct 2020 15:24:34 +0800
Subject: [PATCH 12/17] nbd: don't update block size after device is started
Mounted NBD device can be resized, one use case is rbd-nbd.
Fix the issue by setting up default block size, then not touch it
in nbd_size_update() any more. This kind of usage is aligned with loop
which has same use case too.
Cc: stable@vger.kernel.org
Fixes: c8a83a6b54d0 ("nbd: Use set_blocksize() to set device blocksize")
Reported-by: lining <lining2020x@163.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: Jan Kara <jack@suse.cz>
Tested-by: lining <lining2020x@163.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/block/nbd.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0bed21c0c81b..c4f9ccf5cc2a 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -296,7 +296,7 @@ static void nbd_size_clear(struct nbd_device *nbd)
}
}
-static void nbd_size_update(struct nbd_device *nbd)
+static void nbd_size_update(struct nbd_device *nbd, bool start)
{
struct nbd_config *config = nbd->config;
struct block_device *bdev = bdget_disk(nbd->disk, 0);
@@ -313,7 +313,8 @@ static void nbd_size_update(struct nbd_device *nbd)
if (bdev) {
if (bdev->bd_disk) {
bd_set_nr_sectors(bdev, nr_sectors);
- set_blocksize(bdev, config->blksize);
+ if (start)
+ set_blocksize(bdev, config->blksize);
} else
set_bit(GD_NEED_PART_SCAN, &nbd->disk->state);
bdput(bdev);
@@ -328,7 +329,7 @@ static void nbd_size_set(struct nbd_device *nbd, loff_t blocksize,
config->blksize = blocksize;
config->bytesize = blocksize * nr_blocks;
if (nbd->task_recv != NULL)
- nbd_size_update(nbd);
+ nbd_size_update(nbd, false);
}
static void nbd_complete_rq(struct request *req)
@@ -1308,7 +1309,7 @@ static int nbd_start_device(struct nbd_device *nbd)
args->index = i;
queue_work(nbd->recv_workq, &args->work);
}
- nbd_size_update(nbd);
+ nbd_size_update(nbd, true);
return error;
}
From f9c9104288da543cd64f186f9e2fba389f415630 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <damien.lemoal@wdc.com>
Date: Thu, 29 Oct 2020 20:04:59 +0900
Subject: [PATCH 13/17] null_blk: Fix zone reset all tracing
In the cae of the REQ_OP_ZONE_RESET_ALL operation, the command sector is
ignored and the operation is applied to all sequential zones. For these
commands, tracing the effect of the command using the command sector to
determine the target zone is thus incorrect.
Fix null_zone_mgmt() zone condition tracing in the case of
REQ_OP_ZONE_RESET_ALL to apply tracing to all sequential zones that are
not already empty.
Fixes: 766c3297d7e1 ("null_blk: add trace in null_blk_zoned.c")
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/block/null_blk_zoned.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index 98056c88926b..b637b16a5f54 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -475,9 +475,14 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
switch (op) {
case REQ_OP_ZONE_RESET_ALL:
- for (i = dev->zone_nr_conv; i < dev->nr_zones; i++)
- null_reset_zone(dev, &dev->zones[i]);
- break;
+ for (i = dev->zone_nr_conv; i < dev->nr_zones; i++) {
+ zone = &dev->zones[i];
+ if (zone->cond != BLK_ZONE_COND_EMPTY) {
+ null_reset_zone(dev, zone);
+ trace_nullb_zone_op(cmd, i, zone->cond);
+ }
+ }
+ return BLK_STS_OK;
case REQ_OP_ZONE_RESET:
ret = null_reset_zone(dev, zone);
break;
From aa1c09cb65e2ed17cb8e652bc7ec84e0af1229eb Mon Sep 17 00:00:00 2001
From: Damien Le Moal <damien.lemoal@wdc.com>
Date: Thu, 29 Oct 2020 20:05:00 +0900
Subject: [PATCH 14/17] null_blk: Fix locking in zoned mode
When the zoned mode is enabled in null_blk, Serializing read, write
and zone management operations for each zone is necessary to protect
device level information for managing zone resources (zone open and
closed counters) as well as each zone condition and write pointer
position. Commit 35bc10b2eafb ("null_blk: synchronization fix for
zoned device") introduced a spinlock to implement this serialization.
However, when memory backing is also enabled, GFP_NOIO memory
allocations are executed under the spinlock, resulting in might_sleep()
warnings. Furthermore, the zone_lock spinlock is locked/unlocked using
spin_lock_irq/spin_unlock_irq, similarly to the memory backing code with
the nullb->lock spinlock. This nested use of irq locks wrecks the irq
enabled/disabled state.
Fix all this by introducing a bitmap for per-zone lock, with locking
implemented using wait_on_bit_lock_io() and clear_and_wake_up_bit().
This locking mechanism allows keeping a zone locked while executing
null_process_cmd(), serializing all operations to the zone while
allowing to sleep during memory backing allocation with GFP_NOIO.
Device level zone resource management information is protected using
a spinlock which is not held while executing null_process_cmd();
Fixes: 35bc10b2eafb ("null_blk: synchronization fix for zoned device")
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/block/null_blk.h | 3 +-
drivers/block/null_blk_zoned.c | 104 +++++++++++++++++++++++++--------
2 files changed, 82 insertions(+), 25 deletions(-)
diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index 3176b269b822..cfd00ad40355 100644
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -47,7 +47,8 @@ struct nullb_device {
unsigned int nr_zones_closed;
struct blk_zone *zones;
sector_t zone_size_sects;
- spinlock_t zone_lock;
+ spinlock_t zone_dev_lock;
+ unsigned long *zone_locks;
unsigned long size; /* device size in MB */
unsigned long completion_nsec; /* time in ns to complete a request */
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index b637b16a5f54..8775acbb4f8f 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/vmalloc.h>
+#include <linux/bitmap.h>
#include "null_blk.h"
#define CREATE_TRACE_POINTS
@@ -45,7 +46,13 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
if (!dev->zones)
return -ENOMEM;
- spin_lock_init(&dev->zone_lock);
+ spin_lock_init(&dev->zone_dev_lock);
+ dev->zone_locks = bitmap_zalloc(dev->nr_zones, GFP_KERNEL);
+ if (!dev->zone_locks) {
+ kvfree(dev->zones);
+ return -ENOMEM;
+ }
+
if (dev->zone_nr_conv >= dev->nr_zones) {
dev->zone_nr_conv = dev->nr_zones - 1;
pr_info("changed the number of conventional zones to %u",
@@ -124,15 +131,26 @@ int null_register_zoned_dev(struct nullb *nullb)
void null_free_zoned_dev(struct nullb_device *dev)
{
+ bitmap_free(dev->zone_locks);
kvfree(dev->zones);
}
+static inline void null_lock_zone(struct nullb_device *dev, unsigned int zno)
+{
+ wait_on_bit_lock_io(dev->zone_locks, zno, TASK_UNINTERRUPTIBLE);
+}
+
+static inline void null_unlock_zone(struct nullb_device *dev, unsigned int zno)
+{
+ clear_and_wake_up_bit(zno, dev->zone_locks);
+}
+
int null_report_zones(struct gendisk *disk, sector_t sector,
unsigned int nr_zones, report_zones_cb cb, void *data)
{
struct nullb *nullb = disk->private_data;
struct nullb_device *dev = nullb->dev;
- unsigned int first_zone, i;
+ unsigned int first_zone, i, zno;
struct blk_zone zone;
int error;
@@ -143,17 +161,17 @@ int null_report_zones(struct gendisk *disk, sector_t sector,
nr_zones = min(nr_zones, dev->nr_zones - first_zone);
trace_nullb_report_zones(nullb, nr_zones);
- for (i = 0; i < nr_zones; i++) {
+ zno = first_zone;
+ for (i = 0; i < nr_zones; i++, zno++) {
/*
* Stacked DM target drivers will remap the zone information by
* modifying the zone information passed to the report callback.
* So use a local copy to avoid corruption of the device zone
* array.
*/
- spin_lock_irq(&dev->zone_lock);
- memcpy(&zone, &dev->zones[first_zone + i],
- sizeof(struct blk_zone));
- spin_unlock_irq(&dev->zone_lock);
+ null_lock_zone(dev, zno);
+ memcpy(&zone, &dev->zones[zno], sizeof(struct blk_zone));
+ null_unlock_zone(dev, zno);
error = cb(&zone, i, data);
if (error)
@@ -163,6 +181,10 @@ int null_report_zones(struct gendisk *disk, sector_t sector,
return nr_zones;
}
+/*
+ * This is called in the case of memory backing from null_process_cmd()
+ * with the target zone already locked.
+ */
size_t null_zone_valid_read_len(struct nullb *nullb,
sector_t sector, unsigned int len)
{
@@ -299,22 +321,27 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
return null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);
+ null_lock_zone(dev, zno);
+ spin_lock(&dev->zone_dev_lock);
+
switch (zone->cond) {
case BLK_ZONE_COND_FULL:
/* Cannot write to a full zone */
- return BLK_STS_IOERR;
+ ret = BLK_STS_IOERR;
+ goto unlock;
case BLK_ZONE_COND_EMPTY:
case BLK_ZONE_COND_CLOSED:
ret = null_check_zone_resources(dev, zone);
if (ret != BLK_STS_OK)
- return ret;
+ goto unlock;
break;
case BLK_ZONE_COND_IMP_OPEN:
case BLK_ZONE_COND_EXP_OPEN:
break;
default:
/* Invalid zone condition */
- return BLK_STS_IOERR;
+ ret = BLK_STS_IOERR;
+ goto unlock;
}
/*
@@ -330,11 +357,14 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
else
cmd->rq->__sector = sector;
} else if (sector != zone->wp) {
- return BLK_STS_IOERR;
+ ret = BLK_STS_IOERR;
+ goto unlock;
}
- if (zone->wp + nr_sectors > zone->start + zone->capacity)
- return BLK_STS_IOERR;
+ if (zone->wp + nr_sectors > zone->start + zone->capacity) {
+ ret = BLK_STS_IOERR;
+ goto unlock;
+ }
if (zone->cond == BLK_ZONE_COND_CLOSED) {
dev->nr_zones_closed--;
@@ -345,9 +375,11 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
if (zone->cond != BLK_ZONE_COND_EXP_OPEN)
zone->cond = BLK_ZONE_COND_IMP_OPEN;
+ spin_unlock(&dev->zone_dev_lock);
ret = null_process_cmd(cmd, REQ_OP_WRITE, sector, nr_sectors);
+ spin_lock(&dev->zone_dev_lock);
if (ret != BLK_STS_OK)
- return ret;
+ goto unlock;
zone->wp += nr_sectors;
if (zone->wp == zone->start + zone->capacity) {
@@ -357,7 +389,13 @@ static blk_status_t null_zone_write(struct nullb_cmd *cmd, sector_t sector,
dev->nr_zones_imp_open--;
zone->cond = BLK_ZONE_COND_FULL;
}
- return BLK_STS_OK;
+ ret = BLK_STS_OK;
+
+unlock:
+ spin_unlock(&dev->zone_dev_lock);
+ null_unlock_zone(dev, zno);
+
+ return ret;
}
static blk_status_t null_open_zone(struct nullb_device *dev, struct blk_zone *zone)
@@ -468,21 +506,33 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
sector_t sector)
{
struct nullb_device *dev = cmd->nq->dev;
- unsigned int zone_no = null_zone_no(dev, sector);
- struct blk_zone *zone = &dev->zones[zone_no];
- blk_status_t ret = BLK_STS_OK;
+ unsigned int zone_no;
+ struct blk_zone *zone;
+ blk_status_t ret;
size_t i;
- switch (op) {
- case REQ_OP_ZONE_RESET_ALL:
+ if (op == REQ_OP_ZONE_RESET_ALL) {
for (i = dev->zone_nr_conv; i < dev->nr_zones; i++) {
+ null_lock_zone(dev, i);
zone = &dev->zones[i];
if (zone->cond != BLK_ZONE_COND_EMPTY) {
+ spin_lock(&dev->zone_dev_lock);
null_reset_zone(dev, zone);
+ spin_unlock(&dev->zone_dev_lock);
trace_nullb_zone_op(cmd, i, zone->cond);
}
+ null_unlock_zone(dev, i);
}
return BLK_STS_OK;
+ }
+
+ zone_no = null_zone_no(dev, sector);
+ zone = &dev->zones[zone_no];
+
+ null_lock_zone(dev, zone_no);
+ spin_lock(&dev->zone_dev_lock);
+
+ switch (op) {
case REQ_OP_ZONE_RESET:
ret = null_reset_zone(dev, zone);
break;
@@ -496,22 +546,27 @@ static blk_status_t null_zone_mgmt(struct nullb_cmd *cmd, enum req_opf op,
ret = null_finish_zone(dev, zone);
break;
default:
- return BLK_STS_NOTSUPP;
+ ret = BLK_STS_NOTSUPP;
+ break;
}
+ spin_unlock(&dev->zone_dev_lock);
+
if (ret == BLK_STS_OK)
trace_nullb_zone_op(cmd, zone_no, zone->cond);
+ null_unlock_zone(dev, zone_no);
+
return ret;
}
blk_status_t null_process_zoned_cmd(struct nullb_cmd *cmd, enum req_opf op,
sector_t sector, sector_t nr_sectors)
{
- blk_status_t sts;
struct nullb_device *dev = cmd->nq->dev;
+ unsigned int zno = null_zone_no(dev, sector);
+ blk_status_t sts;
- spin_lock_irq(&dev->zone_lock);
switch (op) {
case REQ_OP_WRITE:
sts = null_zone_write(cmd, sector, nr_sectors, false);
@@ -527,9 +582,10 @@ blk_status_t null_process_zoned_cmd(struct nullb_cmd *cmd, enum req_opf op,
sts = null_zone_mgmt(cmd, op, sector);
break;
default:
+ null_lock_zone(dev, zno);
sts = null_process_cmd(cmd, op, sector, nr_sectors);
+ null_unlock_zone(dev, zno);
}
- spin_unlock_irq(&dev->zone_lock);
return sts;
}
From 7cb6e22ba011c2dc1e94cc0dc94d9ef199715ed9 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Tue, 27 Oct 2020 19:11:30 +0200
Subject: [PATCH 15/17] xsysace: use platform_get_resource() and
platform_get_irq_optional()
Use platform_get_resource() to fetch the memory resource and
platform_get_irq_optional() to get optional IRQ instead of
open-coded variants.
IRQ is not supposed to be changed at runtime, so there is
no functional change in ace_fsm_yieldirq().
On the other hand we now take first resources instead of last ones
to proceed. I can't imagine how broken should be firmware to have
a garbage in the first resource slots. But if it the case, it needs
to be documented.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
drivers/block/xsysace.c | 49 ++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 8d581c7536fb..eb8ef65778c3 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -443,22 +443,27 @@ static void ace_fix_driveid(u16 *id)
#define ACE_FSM_NUM_STATES 11
/* Set flag to exit FSM loop and reschedule tasklet */
-static inline void ace_fsm_yield(struct ace_device *ace)
+static inline void ace_fsm_yieldpoll(struct ace_device *ace)
{
- dev_dbg(ace->dev, "ace_fsm_yield()\n");
tasklet_schedule(&ace->fsm_tasklet);
ace->fsm_continue_flag = 0;
}
+static inline void ace_fsm_yield(struct ace_device *ace)
+{
+ dev_dbg(ace->dev, "%s()\n", __func__);
+ ace_fsm_yieldpoll(ace);
+}
+
/* Set flag to exit FSM loop and wait for IRQ to reschedule tasklet */
static inline void ace_fsm_yieldirq(struct ace_device *ace)
{
dev_dbg(ace->dev, "ace_fsm_yieldirq()\n");
- if (!ace->irq)
- /* No IRQ assigned, so need to poll */
- tasklet_schedule(&ace->fsm_tasklet);
- ace->fsm_continue_flag = 0;
+ if (ace->irq > 0)
+ ace->fsm_continue_flag = 0;
+ else
+ ace_fsm_yieldpoll(ace);
}
static bool ace_has_next_request(struct request_queue *q)
@@ -1053,12 +1058,12 @@ static int ace_setup(struct ace_device *ace)
ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ);
/* Now we can hook up the irq handler */
- if (ace->irq) {
+ if (ace->irq > 0) {
rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace);
if (rc) {
/* Failure - fall back to polled mode */
dev_err(ace->dev, "request_irq failed\n");
- ace->irq = 0;
+ ace->irq = rc;
}
}
@@ -1110,7 +1115,7 @@ static void ace_teardown(struct ace_device *ace)
tasklet_kill(&ace->fsm_tasklet);
- if (ace->irq)
+ if (ace->irq > 0)
free_irq(ace->irq, ace);
iounmap(ace->baseaddr);
@@ -1123,11 +1128,6 @@ static int ace_alloc(struct device *dev, int id, resource_size_t physaddr,
int rc;
dev_dbg(dev, "ace_alloc(%p)\n", dev);
- if (!physaddr) {
- rc = -ENODEV;
- goto err_noreg;
- }
-
/* Allocate and initialize the ace device structure */
ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL);
if (!ace) {
@@ -1153,7 +1153,6 @@ err_setup:
dev_set_drvdata(dev, NULL);
kfree(ace);
err_alloc:
-err_noreg:
dev_err(dev, "could not initialize device, err=%i\n", rc);
return rc;
}
@@ -1176,10 +1175,11 @@ static void ace_free(struct device *dev)
static int ace_probe(struct platform_device *dev)
{
- resource_size_t physaddr = 0;
int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */
+ resource_size_t physaddr;
+ struct resource *res;
u32 id = dev->id;
- int irq = 0;
+ int irq;
int i;
dev_dbg(&dev->dev, "ace_probe(%p)\n", dev);
@@ -1190,12 +1190,15 @@ static int ace_probe(struct platform_device *dev)
if (of_find_property(dev->dev.of_node, "8-bit", NULL))
bus_width = ACE_BUS_WIDTH_8;
- for (i = 0; i < dev->num_resources; i++) {
- if (dev->resource[i].flags & IORESOURCE_MEM)
- physaddr = dev->resource[i].start;
- if (dev->resource[i].flags & IORESOURCE_IRQ)
- irq = dev->resource[i].start;
- }
+ res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ physaddr = res->start;
+ if (!physaddr)
+ return -ENODEV;
+
+ irq = platform_get_irq_optional(dev, 0);
/* Call the bus-independent setup code */
return ace_alloc(&dev->dev, id, physaddr, irq, bus_width);
From 1f41be7d4e90e36084037cecba5978b3d7f849db Mon Sep 17 00:00:00 2001
From: David Disseldorp <ddiss@suse.de>
Date: Mon, 26 Oct 2020 22:03:10 +0100
Subject: [PATCH 16/17] lib/scatterlist: use consistent sg_copy_buffer() return
type
sg_copy_buffer() returns a size_t with the number of bytes copied.
Return 0 instead of false if the copy is skipped.
Signed-off-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Douglas Gilbert <dgilbert@interlog.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
lib/scatterlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 0a482ef988e5..a59778946404 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -933,7 +933,7 @@ size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
sg_miter_start(&miter, sgl, nents, sg_flags);
if (!sg_miter_skip(&miter, skip))
- return false;
+ return 0;
while ((offset < buflen) && sg_miter_next(&miter)) {
unsigned int len;
From 65ff5cd04551daf2c11c7928e48fc3483391c900 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Fri, 30 Oct 2020 10:42:53 +0800
Subject: [PATCH 17/17] blk-mq: mark flush request as IDLE in flush_end_io()
Mark flush request as IDLE in its .end_io(), aligning it with how normal
requests behave. The flush request stays in in-flight tags if we're not
using an IO scheduler, so we need to change its state into IDLE.
Otherwise, we will hang in blk_mq_tagset_wait_completed_request() during
error recovery because flush the request state is kept as COMPLETED.
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Tested-by: Yi Zhang <yi.zhang@redhat.com>
Cc: Chao Leng <lengchao@huawei.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
block/blk-flush.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 53abb5c73d99..e32958f0b687 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -225,6 +225,7 @@ static void flush_end_io(struct request *flush_rq, blk_status_t error)
/* release the tag's ownership to the req cloned from */
spin_lock_irqsave(&fq->mq_flush_lock, flags);
+ WRITE_ONCE(flush_rq->state, MQ_RQ_IDLE);
if (!refcount_dec_and_test(&flush_rq->ref)) {
fq->rq_status = error;
spin_unlock_irqrestore(&fq->mq_flush_lock, flags);