[PATCH RFC 0/4] Move active IO termination to the core

[PATCH RFC 0/4] Move active IO termination to the core

[Sagi Grimberg]

This patchset adds helper to active IO termination to the nvme
core based on the pci nvme_clear_queues() functionality. This
is needed for live shutdowns and resets during live IO and it's
not pci specific and will be needed for other transports as well.

Sagi Grimberg (4):
  blk-mq: Export tagset iter function
  nvme: Add a controller reference to the admin tagset
  nvme: Move IO termination code to the core
  blk-mq: Make blk_mq_all_tag_busy_iter static

 block/blk-mq-tag.c         |   15 ++++++++++++---
 drivers/nvme/host/core.c   |   14 ++++++++++++++
 drivers/nvme/host/nvme.h   |    5 +++++
 drivers/nvme/host/pci.c    |   23 ++++++++---------------
 drivers/nvme/host/rdma.c   |    2 ++
 drivers/nvme/target/loop.c |    2 ++
 include/linux/blk-mq.h     |    4 ++--
 7 files changed, 45 insertions(+), 20 deletions(-)

[PATCH RFC 1/4] blk-mq: Export a busy tagset iterator helper

[Sagi Grimberg]

Its useful to iterate on all the active tags the tag-set in cases
where we will need to fail all the active requests on all the
queues.

Signed-off-by: Sagi Grimberg <sagig at mellanox.com>
---
 block/blk-mq-tag.c     |   10 ++++++++++
 include/linux/blk-mq.h |    2 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index abdbb47..39660ca 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -474,6 +474,16 @@ void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
 }
 EXPORT_SYMBOL(blk_mq_all_tag_busy_iter);
 
+void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
+		busy_tag_iter_fn *fn, void *priv)
+{
+	int i;
+
+	for (i = 0; i < tagset->nr_hw_queues; i++)
+		blk_mq_all_tag_busy_iter(tagset->tags[i], fn, priv);
+}
+EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
+
 void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,
 		void *priv)
 {
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index be77551..97a4652 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -242,6 +242,8 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs);
 void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
 		void *priv);
+void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
+		busy_tag_iter_fn *fn, void *priv);
 void blk_mq_freeze_queue(struct request_queue *q);
 void blk_mq_unfreeze_queue(struct request_queue *q);
 void blk_mq_freeze_queue_start(struct request_queue *q);
-- 
1.7.1

[PATCH RFC 2/4] nvme: Add a controller reference to the admin tagset

[Sagi Grimberg]

We will need it in order to move the IO failure helpers to the core.

Signed-off-by: Sagi Grimberg <sagig at mellanox.com>
---
 drivers/nvme/host/nvme.h   |    1 +
 drivers/nvme/host/pci.c    |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 8bc6660..2e3475e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -49,6 +49,7 @@ struct nvme_ctrl {
 	struct kref kref;
 	int instance;
 	struct blk_mq_tag_set *tagset;
+	struct blk_mq_tag_set *admin_tagset;
 	struct list_head namespaces;
 	struct device *device;	/* char device */
 	struct list_head node;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index a5c2390..0d78b3a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1261,6 +1261,7 @@ static int nvme_alloc_admin_tags(struct nvme_dev *dev)
 			blk_mq_free_tag_set(&dev->admin_tagset);
 			return -ENOMEM;
 		}
+		dev->ctrl.admin_tagset = &dev->admin_tagset;
 		if (!blk_get_queue(dev->ctrl.admin_q)) {
 			nvme_dev_remove_admin(dev);
 			dev->ctrl.admin_q = NULL;
-- 
1.7.1

[PATCH RFC 3/4] nvme: Move IO termination code to the core

[Sagi Grimberg]

We'll need IO termination helpers also for other transports,
so move this code to the core. The difference is that we are
iterating on tagsets and not queues. We distinguish between
io and admin commands because each might be needed for different
flows (and they iterate on different tagsets).

Note, we changed nvme_queue_cancel_ios name to nvme_cancel_io
as there is no concept of a queue now in this function (we
also lost the print).

Signed-off-by: Sagi Grimberg <sagig at mellanox.com>
---
 drivers/nvme/host/core.c |   14 ++++++++++++++
 drivers/nvme/host/nvme.h |    4 ++++
 drivers/nvme/host/pci.c  |   22 +++++++---------------
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 31aa8ed..2c1109b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -499,6 +499,20 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvme_shutdown_ctrl);
 
+void nvme_fail_admin_cmds(struct nvme_ctrl *ctrl,
+		busy_tag_iter_fn *terminate_io, void *priv)
+{
+	blk_mq_tagset_busy_iter(ctrl->admin_tagset, terminate_io, priv);
+}
+EXPORT_SYMBOL_GPL(nvme_fail_admin_cmds);
+
+void nvme_fail_io_cmds(struct nvme_ctrl *ctrl,
+		busy_tag_iter_fn *terminate_io, void *priv)
+{
+	blk_mq_tagset_busy_iter(ctrl->tagset, terminate_io, priv);
+}
+EXPORT_SYMBOL_GPL(nvme_fail_io_cmds);
+
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {
 	struct nvme_user_io io;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 2e3475e..baf67d9 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -246,6 +246,10 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int count);
 int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap);
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap, unsigned page_shift);
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl);
+void nvme_fail_admin_cmds(struct nvme_ctrl *ctrl,
+		busy_tag_iter_fn *terminate_io, void *priv);
+void nvme_fail_io_cmds(struct nvme_ctrl *ctrl,
+		busy_tag_iter_fn *terminate_io, void *priv);
 
 extern spinlock_t dev_list_lock;
 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0d78b3a..a74f68c 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -984,16 +984,15 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
 	return BLK_EH_RESET_TIMER;
 }
 
-static void nvme_cancel_queue_ios(struct request *req, void *data, bool reserved)
+static void nvme_cancel_io(struct request *req, void *data, bool reserved)
 {
-	struct nvme_queue *nvmeq = data;
+	struct nvme_dev *dev = data;
 	u16 status = NVME_SC_ABORT_REQ;
 
 	if (!blk_mq_request_started(req))
 		return;
 
-	dev_warn(nvmeq->q_dmadev, "Cancelling I/O %d QID %d\n",
-						req->tag, nvmeq->qid);
+	dev_warn(dev->dev, "Cancelling I/O %d\n", req->tag);
 
 	if (blk_queue_dying(req->q))
 		status |= NVME_SC_DNR;
@@ -1049,14 +1048,6 @@ static int nvme_suspend_queue(struct nvme_queue *nvmeq)
 	return 0;
 }
 
-static void nvme_clear_queue(struct nvme_queue *nvmeq)
-{
-	spin_lock_irq(&nvmeq->q_lock);
-	if (nvmeq->tags && *nvmeq->tags)
-		blk_mq_all_tag_busy_iter(*nvmeq->tags, nvme_cancel_queue_ios, nvmeq);
-	spin_unlock_irq(&nvmeq->q_lock);
-}
-
 static void nvme_disable_queue(struct nvme_dev *dev, int qid)
 {
 	struct nvme_queue *nvmeq = dev->queues[qid];
@@ -1712,7 +1703,8 @@ static void nvme_wait_dq(struct nvme_delq_ctx *dq, struct nvme_dev *dev)
 			set_current_state(TASK_RUNNING);
 			nvme_disable_ctrl(&dev->ctrl,
 				lo_hi_readq(dev->bar + NVME_REG_CAP));
-			nvme_clear_queue(dev->queues[0]);
+			nvme_fail_admin_cmds(&dev->ctrl,
+				nvme_cancel_io, dev);
 			flush_kthread_worker(dq->worker);
 			nvme_disable_queue(dev, 0);
 			return;
@@ -1929,8 +1921,8 @@ static void nvme_dev_shutdown(struct nvme_dev *dev)
 	}
 	nvme_dev_unmap(dev);
 
-	for (i = dev->queue_count - 1; i >= 0; i--)
-		nvme_clear_queue(dev->queues[i]);
+	nvme_fail_io_cmds(&dev->ctrl, nvme_cancel_io, dev);
+	nvme_fail_admin_cmds(&dev->ctrl, nvme_cancel_io, dev);
 }
 
 static int nvme_setup_prp_pools(struct nvme_dev *dev)
-- 
1.7.1

[PATCH RFC 4/4] blk-mq: Make blk_mq_all_tag_busy_iter static

[Sagi Grimberg]

No caller outside the blk-mq code so we can settle with it static.

Signed-off-by: Sagi Grimberg <sagig at mellanox.com>
---
 block/blk-mq-tag.c     |    5 ++---
 include/linux/blk-mq.h |    2 --
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 39660ca..be635fa 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -464,15 +464,14 @@ static void bt_tags_for_each(struct blk_mq_tags *tags,
 	}
 }
 
-void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
-		void *priv)
+static void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags,
+		busy_tag_iter_fn *fn, void *priv)
 {
 	if (tags->nr_reserved_tags)
 		bt_tags_for_each(tags, &tags->breserved_tags, 0, fn, priv, true);
 	bt_tags_for_each(tags, &tags->bitmap_tags, tags->nr_reserved_tags, fn, priv,
 			false);
 }
-EXPORT_SYMBOL(blk_mq_all_tag_busy_iter);
 
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		busy_tag_iter_fn *fn, void *priv)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 97a4652..b5e6af4 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -240,8 +240,6 @@ void blk_mq_start_hw_queues(struct request_queue *q);
 void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
 void blk_mq_run_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs);
-void blk_mq_all_tag_busy_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
-		void *priv);
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		busy_tag_iter_fn *fn, void *priv);
 void blk_mq_freeze_queue(struct request_queue *q);
-- 
1.7.1

[PATCH RFC 1/4] blk-mq: Export a busy tagset iterator helper

[Christoph Hellwig]

Looks okay to me, although I'm not really sold on absolutely
needing this helper..

[PATCH RFC 3/4] nvme: Move IO termination code to the core

[Christoph Hellwig]

> +void nvme_fail_admin_cmds(struct nvme_ctrl *ctrl,
> +		busy_tag_iter_fn *terminate_io, void *priv)
> +{
> +	blk_mq_tagset_busy_iter(ctrl->admin_tagset, terminate_io, priv);
> +}
> +EXPORT_SYMBOL_GPL(nvme_fail_admin_cmds);
> +
> +void nvme_fail_io_cmds(struct nvme_ctrl *ctrl,
> +		busy_tag_iter_fn *terminate_io, void *priv)
> +{
> +	blk_mq_tagset_busy_iter(ctrl->tagset, terminate_io, priv);
> +}
> +EXPORT_SYMBOL_GPL(nvme_fail_io_cmds);

But I see absolutely no point for these helpers.  These are trivial
one-liners that can stay in the transport drivers.  And with that we
also don't need the admin_tagset pointer in the generic controller
(at least yet..)

[PATCH RFC 1/4] blk-mq: Export a busy tagset iterator helper

[Sagi Grimberg]

> Looks okay to me, although I'm not really sold on absolutely
> needing this helper..

We only use blk_mq_all_tag_busy_iter to iterate on *all* the queues, so
I just figured we can move this loop to blk-mq and just pass in the
tagset.

[PATCH RFC 3/4] nvme: Move IO termination code to the core

[Sagi Grimberg]

>> +void nvme_fail_admin_cmds(struct nvme_ctrl *ctrl,
>> +		busy_tag_iter_fn *terminate_io, void *priv)
>> +{
>> +	blk_mq_tagset_busy_iter(ctrl->admin_tagset, terminate_io, priv);
>> +}
>> +EXPORT_SYMBOL_GPL(nvme_fail_admin_cmds);
>> +
>> +void nvme_fail_io_cmds(struct nvme_ctrl *ctrl,
>> +		busy_tag_iter_fn *terminate_io, void *priv)
>> +{
>> +	blk_mq_tagset_busy_iter(ctrl->tagset, terminate_io, priv);
>> +}
>> +EXPORT_SYMBOL_GPL(nvme_fail_io_cmds);
>
> But I see absolutely no point for these helpers.  These are trivial
> one-liners that can stay in the transport drivers.  And with that we
> also don't need the admin_tagset pointer in the generic controller
> (at least yet..)

I just figured that it'd be nice to have a meaningfully named helpers,
it's not uncommon in the kernel...

I have no problem losing them anyways so unless someone votes to keep
them I'll remove them in v1.

[PATCH RFC 3/4] nvme: Move IO termination code to the core

[Christoph Hellwig]

On Thu, Dec 24, 2015@04:53:16PM +0200, Sagi Grimberg wrote:
> I just figured that it'd be nice to have a meaningfully named helpers,
> it's not uncommon in the kernel...
> 
> I have no problem losing them anyways so unless someone votes to keep
> them I'll remove them in v1.

Yeah.  Let's keep blk_mq_tagset_busy_iter and kill these wrappers.