[PATCH 0/3] Move tagset reinit to its only current consumer, nvme-core

[PATCH 0/3] Move tagset reinit to its only current consumer, nvme-core

[Sagi Grimberg]

As suggested by Bart Van Assche, get rid of
blk_mq_reinit_tagset and move it to nvme-core (its
only current consumer).

Instead, introduce a more generic tagset iterator helper.

Sagi Grimberg (3):
  block: introduce blk_mq_tagset_iter
  nvme: introduce nvme_reinit_tagset
  block: remove blk_mq_reinit_tagset

 block/blk-mq-tag.c       | 11 +++++------
 drivers/nvme/host/core.c | 13 +++++++++++++
 drivers/nvme/host/fc.c   |  3 ++-
 drivers/nvme/host/nvme.h |  2 ++
 drivers/nvme/host/rdma.c |  7 +++----
 include/linux/blk-mq.h   |  4 ++--
 6 files changed, 27 insertions(+), 13 deletions(-)

-- 
2.7.4

[PATCH 0/3] Move tagset reinit to its only current consumer, nvme-core

[Sagi Grimberg]

As suggested by Bart Van Assche, get rid of
blk_mq_reinit_tagset and move it to nvme-core (its
only current consumer).

Instead, introduce a more generic tagset iterator helper.

Sagi Grimberg (3):
  block: introduce blk_mq_tagset_iter
  nvme: introduce nvme_reinit_tagset
  block: remove blk_mq_reinit_tagset

 block/blk-mq-tag.c       | 11 +++++------
 drivers/nvme/host/core.c | 13 +++++++++++++
 drivers/nvme/host/fc.c   |  3 ++-
 drivers/nvme/host/nvme.h |  2 ++
 drivers/nvme/host/rdma.c |  7 +++----
 include/linux/blk-mq.h   |  4 ++--
 6 files changed, 27 insertions(+), 13 deletions(-)

-- 
2.7.4

[PATCH 1/3] block: introduce blk_mq_tagset_iter

[Sagi Grimberg]

Iterator helper to apply a function on all the
tags in a given tagset. export it as it will be used
outside the block layer later on.

Suggested-by: Bart Van Assche <bart.vanassche@wdc.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 block/blk-mq-tag.c     | 16 +++++++++++-----
 include/linux/blk-mq.h |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 6714507aa6c7..bce1c76fc768 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -298,12 +298,12 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 }
 EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
 
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *))
+int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
+			 int (fn)(void *, struct request *))
 {
 	int i, j, ret = 0;
 
-	if (WARN_ON_ONCE(!reinit_request))
+	if (WARN_ON_ONCE(!fn))
 		goto out;
 
 	for (i = 0; i < set->nr_hw_queues; i++) {
@@ -316,8 +316,7 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 			if (!tags->static_rqs[j])
 				continue;
 
-			ret = reinit_request(set->driver_data,
-					     tags->static_rqs[j]);
+			ret = fn(data, tags->static_rqs[j]);
 			if (ret)
 				goto out;
 		}
@@ -326,6 +325,13 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(blk_mq_tagset_iter);
+
+int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
+			 int (reinit_request)(void *, struct request *))
+{
+	return blk_mq_tagset_iter(set, set->driver_data, reinit_request);
+}
 EXPORT_SYMBOL_GPL(blk_mq_reinit_tagset);
 
 void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 50c6485cb04f..6bc29f73a9aa 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -259,6 +259,8 @@ void blk_freeze_queue_start(struct request_queue *q);
 void blk_mq_freeze_queue_wait(struct request_queue *q);
 int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);
+int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
+		int (reinit_request)(void *, struct request *));
 int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 			 int (reinit_request)(void *, struct request *));
 
-- 
2.7.4

[PATCH 1/3] block: introduce blk_mq_tagset_iter

[Sagi Grimberg]

Iterator helper to apply a function on all the
tags in a given tagset. export it as it will be used
outside the block layer later on.

Suggested-by: Bart Van Assche <bart.vanassche at wdc.com>
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 block/blk-mq-tag.c     | 16 +++++++++++-----
 include/linux/blk-mq.h |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 6714507aa6c7..bce1c76fc768 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -298,12 +298,12 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 }
 EXPORT_SYMBOL(blk_mq_tagset_busy_iter);
 
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *))
+int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
+			 int (fn)(void *, struct request *))
 {
 	int i, j, ret = 0;
 
-	if (WARN_ON_ONCE(!reinit_request))
+	if (WARN_ON_ONCE(!fn))
 		goto out;
 
 	for (i = 0; i < set->nr_hw_queues; i++) {
@@ -316,8 +316,7 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 			if (!tags->static_rqs[j])
 				continue;
 
-			ret = reinit_request(set->driver_data,
-					     tags->static_rqs[j]);
+			ret = fn(data, tags->static_rqs[j]);
 			if (ret)
 				goto out;
 		}
@@ -326,6 +325,13 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(blk_mq_tagset_iter);
+
+int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
+			 int (reinit_request)(void *, struct request *))
+{
+	return blk_mq_tagset_iter(set, set->driver_data, reinit_request);
+}
 EXPORT_SYMBOL_GPL(blk_mq_reinit_tagset);
 
 void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_iter_fn *fn,
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 50c6485cb04f..6bc29f73a9aa 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -259,6 +259,8 @@ void blk_freeze_queue_start(struct request_queue *q);
 void blk_mq_freeze_queue_wait(struct request_queue *q);
 int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);
+int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
+		int (reinit_request)(void *, struct request *));
 int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
 			 int (reinit_request)(void *, struct request *));
 
-- 
2.7.4

Re: [PATCH 1/3] block: introduce blk_mq_tagset_iter

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/3] block: introduce blk_mq_tagset_iter

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 1/3] block: introduce blk_mq_tagset_iter

[Bart Van Assche]

T24gV2VkLCAyMDE3LTA5LTA2IGF0IDE4OjMwICswMzAwLCBTYWdpIEdyaW1iZXJnIHdyb3RlOg0K
PiBJdGVyYXRvciBoZWxwZXIgdG8gYXBwbHkgYSBmdW5jdGlvbiBvbiBhbGwgdGhlDQo+IHRhZ3Mg
aW4gYSBnaXZlbiB0YWdzZXQuIGV4cG9ydCBpdCBhcyBpdCB3aWxsIGJlIHVzZWQNCj4gb3V0c2lk
ZSB0aGUgYmxvY2sgbGF5ZXIgbGF0ZXIgb24uDQoNClRoYW5rcyBTYWdpIGZvciBoYXZpbmcgZG9u
ZSB0aGlzIHdvcmsuDQoNClJldmlld2VkLWJ5OiBCYXJ0IFZhbiBBc3NjaGUgPGJhcnQudmFuYXNz
Y2hlQHdkYy5jb20+DQo=

[PATCH 1/3] block: introduce blk_mq_tagset_iter

[Bart Van Assche]

On Wed, 2017-09-06@18:30 +0300, Sagi Grimberg wrote:
> Iterator helper to apply a function on all the
> tags in a given tagset. export it as it will be used
> outside the block layer later on.

Thanks Sagi for having done this work.

Reviewed-by: Bart Van Assche <bart.vanassche at wdc.com>

[PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Sagi Grimberg]

Move blk_mq_reinit_tagset from blk-mq to nvme core
as the only user of it. Current transports that use
it (rdma, fc) simply implement .reinit_request op.

This patch does not change any functionality.

Suggested-by: Bart Van Assche <bart.vanassche@wdc.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/core.c | 13 +++++++++++++
 drivers/nvme/host/fc.c   |  3 ++-
 drivers/nvme/host/nvme.h |  2 ++
 drivers/nvme/host/rdma.c |  7 +++----
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 277a7a02cba5..beeb51d0eecc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2942,6 +2942,19 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvme_start_queues);
 
+int nvme_reinit_tagset(struct nvme_ctrl *ctrl, bool admin)
+{
+	struct blk_mq_tag_set *set = admin ?
+			ctrl->admin_tagset : ctrl->tagset;
+
+	if (!ctrl->ops->reinit_request)
+		return 0;
+
+	return blk_mq_tagset_iter(set, set->driver_data,
+			ctrl->ops->reinit_request);
+}
+EXPORT_SYMBOL_GPL(nvme_reinit_tagset);
+
 int __init nvme_core_init(void)
 {
 	int result;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index d2e882c0f496..310be96543f8 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2336,7 +2336,7 @@ nvme_fc_reinit_io_queues(struct nvme_fc_ctrl *ctrl)
 
 	nvme_fc_init_io_queues(ctrl);
 
-	ret = blk_mq_reinit_tagset(&ctrl->tag_set, nvme_fc_reinit_request);
+	ret = nvme_reinit_tagset(&ctrl->ctrl, false);
 	if (ret)
 		goto out_free_io_queues;
 
@@ -2696,6 +2696,7 @@ static const struct nvme_ctrl_ops nvme_fc_ctrl_ops = {
 	.submit_async_event	= nvme_fc_submit_async_event,
 	.delete_ctrl		= nvme_fc_del_nvme_ctrl,
 	.get_address		= nvmf_get_address,
+	.reinit_request		= nvme_fc_reinit_request,
 };
 
 static void
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a19a587d60ed..ce51fc55a502 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -229,6 +229,7 @@ struct nvme_ctrl_ops {
 	void (*submit_async_event)(struct nvme_ctrl *ctrl, int aer_idx);
 	int (*delete_ctrl)(struct nvme_ctrl *ctrl);
 	int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size);
+	int (*reinit_request)(void *data, struct request *rq);
 };
 
 static inline bool nvme_ctrl_ready(struct nvme_ctrl *ctrl)
@@ -303,6 +304,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
 void nvme_start_freeze(struct nvme_ctrl *ctrl);
+int nvme_reinit_tagset(struct nvme_ctrl *ctrl, bool admin);
 
 #define NVME_QID_ANY -1
 struct request *nvme_alloc_request(struct request_queue *q,
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 6a7682620d87..4697af602ad4 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -767,8 +767,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
 			goto out_free_tagset;
 		}
 	} else {
-		error = blk_mq_reinit_tagset(&ctrl->admin_tag_set,
-					     nvme_rdma_reinit_request);
+		error = nvme_reinit_tagset(&ctrl->ctrl, true);
 		if (error)
 			goto out_free_queue;
 	}
@@ -848,8 +847,7 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 			goto out_free_tag_set;
 		}
 	} else {
-		ret = blk_mq_reinit_tagset(&ctrl->tag_set,
-					   nvme_rdma_reinit_request);
+		ret = nvme_reinit_tagset(&ctrl->ctrl, false);
 		if (ret)
 			goto out_free_io_queues;
 
@@ -1821,6 +1819,7 @@ static const struct nvme_ctrl_ops nvme_rdma_ctrl_ops = {
 	.submit_async_event	= nvme_rdma_submit_async_event,
 	.delete_ctrl		= nvme_rdma_del_ctrl,
 	.get_address		= nvmf_get_address,
+	.reinit_request		= nvme_rdma_reinit_request,
 };
 
 static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
-- 
2.7.4

[PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Sagi Grimberg]

Move blk_mq_reinit_tagset from blk-mq to nvme core
as the only user of it. Current transports that use
it (rdma, fc) simply implement .reinit_request op.

This patch does not change any functionality.

Suggested-by: Bart Van Assche <bart.vanassche at wdc.com>
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 13 +++++++++++++
 drivers/nvme/host/fc.c   |  3 ++-
 drivers/nvme/host/nvme.h |  2 ++
 drivers/nvme/host/rdma.c |  7 +++----
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 277a7a02cba5..beeb51d0eecc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2942,6 +2942,19 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvme_start_queues);
 
+int nvme_reinit_tagset(struct nvme_ctrl *ctrl, bool admin)
+{
+	struct blk_mq_tag_set *set = admin ?
+			ctrl->admin_tagset : ctrl->tagset;
+
+	if (!ctrl->ops->reinit_request)
+		return 0;
+
+	return blk_mq_tagset_iter(set, set->driver_data,
+			ctrl->ops->reinit_request);
+}
+EXPORT_SYMBOL_GPL(nvme_reinit_tagset);
+
 int __init nvme_core_init(void)
 {
 	int result;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index d2e882c0f496..310be96543f8 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2336,7 +2336,7 @@ nvme_fc_reinit_io_queues(struct nvme_fc_ctrl *ctrl)
 
 	nvme_fc_init_io_queues(ctrl);
 
-	ret = blk_mq_reinit_tagset(&ctrl->tag_set, nvme_fc_reinit_request);
+	ret = nvme_reinit_tagset(&ctrl->ctrl, false);
 	if (ret)
 		goto out_free_io_queues;
 
@@ -2696,6 +2696,7 @@ static const struct nvme_ctrl_ops nvme_fc_ctrl_ops = {
 	.submit_async_event	= nvme_fc_submit_async_event,
 	.delete_ctrl		= nvme_fc_del_nvme_ctrl,
 	.get_address		= nvmf_get_address,
+	.reinit_request		= nvme_fc_reinit_request,
 };
 
 static void
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a19a587d60ed..ce51fc55a502 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -229,6 +229,7 @@ struct nvme_ctrl_ops {
 	void (*submit_async_event)(struct nvme_ctrl *ctrl, int aer_idx);
 	int (*delete_ctrl)(struct nvme_ctrl *ctrl);
 	int (*get_address)(struct nvme_ctrl *ctrl, char *buf, int size);
+	int (*reinit_request)(void *data, struct request *rq);
 };
 
 static inline bool nvme_ctrl_ready(struct nvme_ctrl *ctrl)
@@ -303,6 +304,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
 void nvme_start_freeze(struct nvme_ctrl *ctrl);
+int nvme_reinit_tagset(struct nvme_ctrl *ctrl, bool admin);
 
 #define NVME_QID_ANY -1
 struct request *nvme_alloc_request(struct request_queue *q,
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 6a7682620d87..4697af602ad4 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -767,8 +767,7 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl,
 			goto out_free_tagset;
 		}
 	} else {
-		error = blk_mq_reinit_tagset(&ctrl->admin_tag_set,
-					     nvme_rdma_reinit_request);
+		error = nvme_reinit_tagset(&ctrl->ctrl, true);
 		if (error)
 			goto out_free_queue;
 	}
@@ -848,8 +847,7 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 			goto out_free_tag_set;
 		}
 	} else {
-		ret = blk_mq_reinit_tagset(&ctrl->tag_set,
-					   nvme_rdma_reinit_request);
+		ret = nvme_reinit_tagset(&ctrl->ctrl, false);
 		if (ret)
 			goto out_free_io_queues;
 
@@ -1821,6 +1819,7 @@ static const struct nvme_ctrl_ops nvme_rdma_ctrl_ops = {
 	.submit_async_event	= nvme_rdma_submit_async_event,
 	.delete_ctrl		= nvme_rdma_del_ctrl,
 	.get_address		= nvmf_get_address,
+	.reinit_request		= nvme_rdma_reinit_request,
 };
 
 static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
-- 
2.7.4

Re: [PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Bart Van Assche]

T24gV2VkLCAyMDE3LTA5LTA2IGF0IDE4OjMwICswMzAwLCBTYWdpIEdyaW1iZXJnIHdyb3RlOg0K
PiAraW50IG52bWVfcmVpbml0X3RhZ3NldChzdHJ1Y3QgbnZtZV9jdHJsICpjdHJsLCBib29sIGFk
bWluKQ0KPiArew0KPiArCXN0cnVjdCBibGtfbXFfdGFnX3NldCAqc2V0ID0gYWRtaW4gPw0KPiAr
CQkJY3RybC0+YWRtaW5fdGFnc2V0IDogY3RybC0+dGFnc2V0Ow0KPiArDQo+ICsJaWYgKCFjdHJs
LT5vcHMtPnJlaW5pdF9yZXF1ZXN0KQ0KPiArCQlyZXR1cm4gMDsNCj4gKw0KPiArCXJldHVybiBi
bGtfbXFfdGFnc2V0X2l0ZXIoc2V0LCBzZXQtPmRyaXZlcl9kYXRhLA0KPiArCQkJY3RybC0+b3Bz
LT5yZWluaXRfcmVxdWVzdCk7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQTChudm1lX3JlaW5p
dF90YWdzZXQpOw0KDQpIZWxsbyBTYWdpLA0KDQpIYXZlIHlvdSBjb25zaWRlcmVkIHRvIHBhc3Mg
YSB0YWcgc2V0IHBvaW50ZXIgYXMgc2Vjb25kIGFyZ3VtZW50IHRvIHRoaXMNCm5ldyBmdW5jdGlv
biBpbnN0ZWFkIG9mIGEgYm9vbGVhbj8gSSB0aGluayB0aGF0IHdvdWxkIG5vdCBvbmx5IG1ha2Ug
dGhpcw0KZnVuY3Rpb24gc2hvcnRlciBidXQgYWxzbyB0aGF0IGl0IHdvdWxkIG1ha2UgdGhlIGNh
bGxlcnMgZWFzaWVyIHRvIHJlYWQuDQoNClRoYW5rcywNCg0KQmFydC4=

[PATCH 2/3] nvme: introduce nvme_reinit_tagset

[Bart Van Assche]

On Wed, 2017-09-06@18:30 +0300, Sagi Grimberg wrote:
> +int nvme_reinit_tagset(struct nvme_ctrl *ctrl, bool admin)
> +{
> +	struct blk_mq_tag_set *set = admin ?
> +			ctrl->admin_tagset : ctrl->tagset;
> +
> +	if (!ctrl->ops->reinit_request)
> +		return 0;
> +
> +	return blk_mq_tagset_iter(set, set->driver_data,
> +			ctrl->ops->reinit_request);
> +}
> +EXPORT_SYMBOL_GPL(nvme_reinit_tagset);

Hello Sagi,

Have you considered to pass a tag set pointer as second argument to this
new function instead of a boolean? I think that would not only make this
function shorter but also that it would make the callers easier to read.

Thanks,

Bart.

[PATCH 3/3] block: remove blk_mq_reinit_tagset

[Sagi Grimberg]

No callers left.

Suggested-by: Bart Van Assche <bart.vanassche@wdc.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 block/blk-mq-tag.c     | 7 -------
 include/linux/blk-mq.h | 2 --
 2 files changed, 9 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index bce1c76fc768..c81b40ecd3f1 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -327,13 +327,6 @@ int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
 }
 EXPORT_SYMBOL_GPL(blk_mq_tagset_iter);
 
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *))
-{
-	return blk_mq_tagset_iter(set, set->driver_data, reinit_request);
-}
-EXPORT_SYMBOL_GPL(blk_mq_reinit_tagset);
-
 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 6bc29f73a9aa..cfd64e500d82 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -261,8 +261,6 @@ int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);
 int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
 		int (reinit_request)(void *, struct request *));
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *));
 
 int blk_mq_map_queues(struct blk_mq_tag_set *set);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
-- 
2.7.4

[PATCH 3/3] block: remove blk_mq_reinit_tagset

[Sagi Grimberg]

No callers left.

Suggested-by: Bart Van Assche <bart.vanassche at wdc.com>
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 block/blk-mq-tag.c     | 7 -------
 include/linux/blk-mq.h | 2 --
 2 files changed, 9 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index bce1c76fc768..c81b40ecd3f1 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -327,13 +327,6 @@ int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
 }
 EXPORT_SYMBOL_GPL(blk_mq_tagset_iter);
 
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *))
-{
-	return blk_mq_tagset_iter(set, set->driver_data, reinit_request);
-}
-EXPORT_SYMBOL_GPL(blk_mq_reinit_tagset);
-
 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 6bc29f73a9aa..cfd64e500d82 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -261,8 +261,6 @@ int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);
 int blk_mq_tagset_iter(struct blk_mq_tag_set *set, void *data,
 		int (reinit_request)(void *, struct request *));
-int blk_mq_reinit_tagset(struct blk_mq_tag_set *set,
-			 int (reinit_request)(void *, struct request *));
 
 int blk_mq_map_queues(struct blk_mq_tag_set *set);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
-- 
2.7.4

Re: [PATCH 3/3] block: remove blk_mq_reinit_tagset

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 3/3] block: remove blk_mq_reinit_tagset

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 3/3] block: remove blk_mq_reinit_tagset

[Bart Van Assche]

T24gV2VkLCAyMDE3LTA5LTA2IGF0IDE4OjMwICswMzAwLCBTYWdpIEdyaW1iZXJnIHdyb3RlOg0K
PiBObyBjYWxsZXJzIGxlZnQuDQoNClJldmlld2VkLWJ5OiBCYXJ0IFZhbiBBc3NjaGUgPGJhcnQu
dmFuYXNzY2hlQHdkYy5jb20+

[PATCH 3/3] block: remove blk_mq_reinit_tagset

[Bart Van Assche]

On Wed, 2017-09-06@18:30 +0300, Sagi Grimberg wrote:
> No callers left.

Reviewed-by: Bart Van Assche <bart.vanassche at wdc.com>

Re: [PATCH 0/3] Move tagset reinit to its only current consumer, nvme-core

[Max Gurtovoy]

On 9/6/2017 6:30 PM, Sagi Grimberg wrote:
> As suggested by Bart Van Assche, get rid of
> blk_mq_reinit_tagset and move it to nvme-core (its
> only current consumer).
> 
> Instead, introduce a more generic tagset iterator helper.
> 
> Sagi Grimberg (3):
>    block: introduce blk_mq_tagset_iter
>    nvme: introduce nvme_reinit_tagset
>    block: remove blk_mq_reinit_tagset
> 
>   block/blk-mq-tag.c       | 11 +++++------
>   drivers/nvme/host/core.c | 13 +++++++++++++
>   drivers/nvme/host/fc.c   |  3 ++-
>   drivers/nvme/host/nvme.h |  2 ++
>   drivers/nvme/host/rdma.c |  7 +++----
>   include/linux/blk-mq.h   |  4 ++--
>   6 files changed, 27 insertions(+), 13 deletions(-)
> 

The series looks good to me,

Reviewed-by: Max Gurtovoy <maxg@mellanox.com>

BTW, if we talking about the reinit_tagset, can you explain the 
motivation for dereg_mr and alloc new mr for the RDMA transport layer ?
we don't do it in iSER/SRP so I wonder what is special here ?
I got some local protection errors in MP tests and under high load FIO 
(that cause cmd TMO to expire and abort the cmds and then reconnect).
I wonder if there is a connection between the two. I'm also debugging a 
situation that we might have free/dereg/local_invalidate the MR before 
the QP moved to ERR state.

[PATCH 0/3] Move tagset reinit to its only current consumer, nvme-core

[Max Gurtovoy]

On 9/6/2017 6:30 PM, Sagi Grimberg wrote:
> As suggested by Bart Van Assche, get rid of
> blk_mq_reinit_tagset and move it to nvme-core (its
> only current consumer).
> 
> Instead, introduce a more generic tagset iterator helper.
> 
> Sagi Grimberg (3):
>    block: introduce blk_mq_tagset_iter
>    nvme: introduce nvme_reinit_tagset
>    block: remove blk_mq_reinit_tagset
> 
>   block/blk-mq-tag.c       | 11 +++++------
>   drivers/nvme/host/core.c | 13 +++++++++++++
>   drivers/nvme/host/fc.c   |  3 ++-
>   drivers/nvme/host/nvme.h |  2 ++
>   drivers/nvme/host/rdma.c |  7 +++----
>   include/linux/blk-mq.h   |  4 ++--
>   6 files changed, 27 insertions(+), 13 deletions(-)
> 

The series looks good to me,

Reviewed-by: Max Gurtovoy <maxg at mellanox.com>

BTW, if we talking about the reinit_tagset, can you explain the 
motivation for dereg_mr and alloc new mr for the RDMA transport layer ?
we don't do it in iSER/SRP so I wonder what is special here ?
I got some local protection errors in MP tests and under high load FIO 
(that cause cmd TMO to expire and abort the cmds and then reconnect).
I wonder if there is a connection between the two. I'm also debugging a 
situation that we might have free/dereg/local_invalidate the MR before 
the QP moved to ERR state.