Skip to contentSkip to conversation

Provide better active I/O termination handlers

PATCH New Sagi Grimberg 5 participants 11 messages Mar 10, 2016 – Apr 12, 2016

Subsystems (2)
Block layer NVMe
lore
Share & tools
lore

Sagi Grimberg

author
#0 lore raw

v3 cover Provide better active I/O termination handlers

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

This patchset applies on top of "for-4.6-drivers" patchset.

Changes from v2:
- Added mtip32xxx conversion from Keith

Changes from v1:
- rebase on top of for-4.6-drivers (small fuzz)
- incorporate a fix from hch to carefully check for valid tagsets

Changes from v0:
- Removed the nvme core helpers for IO termination and have pci use blk-mq helper directly
- Rebased on top of Keith and Christoph's patches

Keith Busch (1):
mtip32xx: Convert to use blk_mq_tagset_busy_iter

Sagi Grimberg (3):
blk-mq: Export tagset iter function
nvme: Use blk-mq helper for IO termination
blk-mq: Make blk_mq_all_tag_busy_iter static

 block/blk-mq-tag.c                | 17 ++++++++++++++---
 drivers/block/mtip32xx/mtip32xx.c |  6 +++---
 drivers/nvme/host/pci.c           | 19 +++++--------------
 include/linux/blk-mq.h            |  4 ++--
 4 files changed, 24 insertions(+), 22 deletions(-)
Signature · 1 line
1.8.4.3
author
#1 lore raw

PATCH v3 1/4 blk-mq: Export tagset iter function

Its useful to iterate on all the active tags in cases where we will need to fail all the queues IO.

Signed-off-by Sagi Grimberg <sagig at mellanox.com>
[hch: carefully check for valid tagsets]
Reviewed-by Christoph Hellwig <hch at lst.de>
Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
---
 block/blk-mq-tag.c     | 12 ++++++++++++
 include/linux/blk-mq.h |  2 ++
 2 files changed, 14 insertions(+)
block/blk-mq-tag.c +12 -0
index abdbb47405cb..2fd04286f103 100644
@@ -474,6 +474,18 @@ 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++) {
+		if (tagset->tags && tagset->tags[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)
 {
include/linux/blk-mq.h +2 -0
index 15a73d49fd1d..865ee7016b72 100644
@@ -240,6 +240,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);
Signature · 1 line
1.8.4.3
author
#2 lore raw

PATCH v3 2/4 nvme: Use blk-mq helper for IO termination

In reply to Sagi Grimberg

blk-mq offers a tagset iterator so let's use that instead of using nvme_clear_queues.

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>
Reviewed-by Christoph Hellwig <hch at lst.de>
Acked-by Keith Busch <keith.busch at intel.com>
Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/nvme/host/pci.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)
drivers/nvme/host/pci.c +5 -14
index e9f18e1d73e5..b713b390aa4f 100644
@@ -982,16 +982,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;
 	int status;
 
 	if (!blk_mq_request_started(req))
 		return;
 
-	dev_warn(nvmeq->dev->ctrl.device,
-		 "Cancelling I/O %d QID %d\n", req->tag, nvmeq->qid);
+	dev_warn(dev->ctrl.device, "Cancelling I/O %d\n", req->tag);
 
 	status = NVME_SC_ABORT_REQ;
 	if (blk_queue_dying(req->q))

@@ -1048,14 +1047,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_admin_queue(struct nvme_dev *dev, bool shutdown)
 {
 	struct nvme_queue *nvmeq = dev->queues[0];

@@ -1779,8 +1770,8 @@ static void nvme_dev_disable(struct nvme_dev *dev, bool shutdown)
 	}
 	nvme_dev_unmap(dev);
 
-	for (i = dev->queue_count - 1; i >= 0; i--)
-		nvme_clear_queue(dev->queues[i]);
+	blk_mq_tagset_busy_iter(&dev->tagset, nvme_cancel_io, dev);
+	blk_mq_tagset_busy_iter(&dev->admin_tagset, nvme_cancel_io, dev);
 	mutex_unlock(&dev->shutdown_lock);
 }
Signature · 1 line
1.8.4.3
author
#3 lore raw

PATCH v3 3/4 mtip32xx: Convert to use blk_mq_tagset_busy_iter

In reply to Sagi Grimberg

From: Keith Busch <[email protected]>

Only a single tags array anyway.

Signed-off-by Keith Busch <keith.busch at intel.com>
Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
Signed-off-by Sagi Grimberg <sagig at mellanox.com>
---
 drivers/block/mtip32xx/mtip32xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/block/mtip32xx/mtip32xx.c +3 -3
index cc2e71d0a77f..2cc86f70c01f 100644
@@ -3000,14 +3000,14 @@ restart_eh:
 					"Completion workers still active!");
 
 			spin_lock(dd->queue->queue_lock);
-			blk_mq_all_tag_busy_iter(*dd->tags.tags,
+			blk_mq_tagset_busy_iter(&dd->tags,
 							mtip_queue_cmd, dd);
 			spin_unlock(dd->queue->queue_lock);
 
 			set_bit(MTIP_PF_ISSUE_CMDS_BIT, &dd->port->flags);
 
 			if (mtip_device_reset(dd))
-				blk_mq_all_tag_busy_iter(*dd->tags.tags,
+				blk_mq_tagset_busy_iter(&dd->tags,
 							mtip_abort_cmd, dd);
 
 			clear_bit(MTIP_PF_TO_ACTIVE_BIT, &dd->port->flags);

@@ -4174,7 +4174,7 @@ static int mtip_block_remove(struct driver_data *dd)
 
 	blk_mq_freeze_queue_start(dd->queue);
 	blk_mq_stop_hw_queues(dd->queue);
-	blk_mq_all_tag_busy_iter(dd->tags.tags[0], mtip_no_dev_cleanup, dd);
+	blk_mq_tagset_busy_iter(&dd->tags, mtip_no_dev_cleanup, dd);
 
 	/*
 	 * Delete our gendisk structure. This also removes the device
Signature · 1 line
1.8.4.3
#4 lore raw

PATCH v3 3/4 mtip32xx: Convert to use blk_mq_tagset_busy_iter

Looks fine,

Reviewed-by Christoph Hellwig <hch at lst.de>
author
#5 lore raw

PATCH v3 4/4 blk-mq: Make blk_mq_all_tag_busy_iter static

In reply to 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>
Reviewed-by Christoph Hellwig <hch at lst.de>
Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
---
 block/blk-mq-tag.c     | 5 ++---
 include/linux/blk-mq.h | 2 --
 2 files changed, 2 insertions(+), 5 deletions(-)
block/blk-mq-tag.c +2 -3
index 2fd04286f103..56a0c37a3d06 100644
@@ -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)
include/linux/blk-mq.h +0 -2
index 865ee7016b72..ecd7ea33a99f 100644
@@ -238,8 +238,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);
Signature · 1 line
1.8.4.3
#6 lore raw

v3 cover Provide better active I/O termination handlers

In reply to Sagi Grimberg
Quoted 7 lines

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

This patchset applies on top of "for-4.6-drivers" patchset.

Actually, it applies against for-next now...

#7 lore raw

v3 cover Provide better active I/O termination handlers

Jens, is this something still worth picking up for 4.6?

maintainer
#8 lore raw

v3 cover Provide better active I/O termination handlers

In reply to Sagi Grimberg

Hey Jens,

So this got left behind in the 4.6 pull request...

Can we at least queue it up for 4.7?

Cheers,
Sagi.

maintainer
#9 lore raw

v3 cover Provide better active I/O termination handlers

Quoted 9 lines · sagig

Hey Jens,

So this got left behind in the 4.6 pull request...

Can we at least queue it up for 4.7?

I'll queue it up for 4.7 - traveling this week, will get to it Monday.

maintainer
#10 lore raw

v3 cover Provide better active I/O termination handlers

In reply to sagig
Quoted 3 lines · sagig

Hey Jens,

So this got left behind in the 4.6 pull request...

It was late for 4.6, sent out the day before...

Quoted 1 line

Can we at least queue it up for 4.7?

Certainly, being queued up now.

Signature · 1 line
Jens Axboe