[PATCH 0/8] blk-mq: fix and improve queue mapping

[PATCH 0/8] blk-mq: fix and improve queue mapping

[Ming Lei]

Hi Jens,

The first two patches fix issues about queue mapping.

The other 6 patches improve queue mapping for blk-mq.

Christian, this patches should fix your issue, so please give
a test, and the patches can be found in the following tree:

	https://github.com/ming1/linux/commits/v4.17-rc-blk-fix_mapping_v1

Thanks,
Ming

Ming Lei (8):
  blk-mq: make sure that correct hctx->next_cpu is set
  blk-mq: don't keep offline CPUs mapped to hctx 0
  blk-mq: avoid to write intermediate result to hctx->next_cpu
  blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu
  blk-mq: remove blk_mq_delay_queue()
  blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()
  blk-mq: reimplement blk_mq_hw_queue_mapped
  blk-mq: remove code for dealing with remapping queue

 block/blk-mq-cpumap.c  |   5 ---
 block/blk-mq-debugfs.c |   1 -
 block/blk-mq.c         | 101 +++++++++++--------------------------------------
 block/blk-mq.h         |   2 +-
 include/linux/blk-mq.h |   2 -
 5 files changed, 24 insertions(+), 87 deletions(-)

-- 
2.9.5

[PATCH 1/8] blk-mq: make sure that correct hctx->next_cpu is set

[Ming Lei]

>From commit 20e4d81393196 (blk-mq: simplify queue mapping & schedule
with each possisble CPU), one hctx can be mapped from all offline CPUs,
then hctx->next_cpu can be set as wrong.

This patch fixes this issue by making hctx->next_cpu pointing to the
first CPU in hctx->cpumask if all CPUs in hctx->cpumask are offline.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Fixes: 20e4d81393196 ("blk-mq: simplify queue mapping & schedule with each possisble CPU")
Cc: stable@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index f5c7dbcb954f..9b220dc415ac 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2432,6 +2432,8 @@ static void blk_mq_map_swqueue(struct request_queue *q)
 		 */
 		hctx->next_cpu = cpumask_first_and(hctx->cpumask,
 				cpu_online_mask);
+		if (hctx->next_cpu >= nr_cpu_ids)
+			hctx->next_cpu = cpumask_first(hctx->cpumask);
 		hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH;
 	}
 }
-- 
2.9.5

[PATCH 2/8] blk-mq: don't keep offline CPUs mapped to hctx 0

[Ming Lei]

>From commit 4b855ad37194 ("blk-mq: Create hctx for each present CPU),
blk-mq doesn't remap queue after CPU topo is changed, that said when
some of these offline CPUs become online, they are still mapped to
hctx 0, then hctx 0 may become the bottleneck of IO dispatch and
completion.

This patch sets up the mapping from the beginning, and aligns to
queue mapping for PCI device (blk_mq_pci_map_queues()).

Fixes: 4b855ad37194 ("blk-mq: Create hctx for each present CPU)
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-cpumap.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 9f8cffc8a701..3eb169f15842 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -16,11 +16,6 @@
 
 static int cpu_to_queue_index(unsigned int nr_queues, const int cpu)
 {
-	/*
-	 * Non present CPU will be mapped to queue index 0.
-	 */
-	if (!cpu_present(cpu))
-		return 0;
 	return cpu % nr_queues;
 }
 
-- 
2.9.5

[PATCH 3/8] blk-mq: avoid to write intermediate result to hctx->next_cpu

[Ming Lei]

This patch figures out the final selected CPU, then writes
it to hctx->next_cpu once, then we can avoid to intermediate
next cpu observed from other dispatch paths.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 9b220dc415ac..a16efa6f2e7f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1345,26 +1345,24 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 {
 	bool tried = false;
+	int next_cpu = hctx->next_cpu;
 
 	if (hctx->queue->nr_hw_queues == 1)
 		return WORK_CPU_UNBOUND;
 
 	if (--hctx->next_cpu_batch <= 0) {
-		int next_cpu;
 select_cpu:
-		next_cpu = cpumask_next_and(hctx->next_cpu, hctx->cpumask,
+		next_cpu = cpumask_next_and(next_cpu, hctx->cpumask,
 				cpu_online_mask);
 		if (next_cpu >= nr_cpu_ids)
-			next_cpu = cpumask_first_and(hctx->cpumask,cpu_online_mask);
+			next_cpu = cpumask_first_and(hctx->cpumask, cpu_online_mask);
 
 		/*
 		 * No online CPU is found, so have to make sure hctx->next_cpu
 		 * is set correctly for not breaking workqueue.
 		 */
 		if (next_cpu >= nr_cpu_ids)
-			hctx->next_cpu = cpumask_first(hctx->cpumask);
-		else
-			hctx->next_cpu = next_cpu;
+			next_cpu = cpumask_first(hctx->cpumask);
 		hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH;
 	}
 
@@ -1372,7 +1370,7 @@ static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 	 * Do unbound schedule if we can't find a online CPU for this hctx,
 	 * and it should only happen in the path of handling CPU DEAD.
 	 */
-	if (!cpu_online(hctx->next_cpu)) {
+	if (!cpu_online(next_cpu)) {
 		if (!tried) {
 			tried = true;
 			goto select_cpu;
@@ -1382,10 +1380,13 @@ static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 		 * Make sure to re-select CPU next time once after CPUs
 		 * in hctx->cpumask become online again.
 		 */
+		hctx->next_cpu = next_cpu;
 		hctx->next_cpu_batch = 1;
 		return WORK_CPU_UNBOUND;
 	}
-	return hctx->next_cpu;
+
+	hctx->next_cpu = next_cpu;
+	return next_cpu;
 }
 
 static void __blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async,
-- 
2.9.5

[PATCH 4/8] blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu

[Ming Lei]

This patch introduces helper of blk_mq_hw_queue_first_cpu() for
figuring out the hctx's first cpu, and code duplication can be
avoided.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a16efa6f2e7f..e3d02af79010 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1336,6 +1336,15 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 	hctx_unlock(hctx, srcu_idx);
 }
 
+static inline int blk_mq_first_mapped_cpu(struct blk_mq_hw_ctx *hctx)
+{
+	int cpu = cpumask_first_and(hctx->cpumask, cpu_online_mask);
+
+	if (cpu >= nr_cpu_ids)
+		cpu = cpumask_first(hctx->cpumask);
+	return cpu;
+}
+
 /*
  * It'd be great if the workqueue API had a way to pass
  * in a mask and had some smarts for more clever placement.
@@ -1355,14 +1364,7 @@ static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 		next_cpu = cpumask_next_and(next_cpu, hctx->cpumask,
 				cpu_online_mask);
 		if (next_cpu >= nr_cpu_ids)
-			next_cpu = cpumask_first_and(hctx->cpumask, cpu_online_mask);
-
-		/*
-		 * No online CPU is found, so have to make sure hctx->next_cpu
-		 * is set correctly for not breaking workqueue.
-		 */
-		if (next_cpu >= nr_cpu_ids)
-			next_cpu = cpumask_first(hctx->cpumask);
+			next_cpu = blk_mq_first_mapped_cpu(hctx);
 		hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH;
 	}
 
@@ -2431,10 +2433,7 @@ static void blk_mq_map_swqueue(struct request_queue *q)
 		/*
 		 * Initialize batch roundrobin counts
 		 */
-		hctx->next_cpu = cpumask_first_and(hctx->cpumask,
-				cpu_online_mask);
-		if (hctx->next_cpu >= nr_cpu_ids)
-			hctx->next_cpu = cpumask_first(hctx->cpumask);
+		hctx->next_cpu = blk_mq_first_mapped_cpu(hctx);
 		hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH;
 	}
 }
-- 
2.9.5

[PATCH 5/8] blk-mq: remove blk_mq_delay_queue()

[Ming Lei]

No driver uses this interface any more, so remove it.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-debugfs.c |  1 -
 block/blk-mq.c         | 30 ++----------------------------
 include/linux/blk-mq.h |  2 --
 3 files changed, 2 insertions(+), 31 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 58b3b79cbe83..3080e18cb859 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -235,7 +235,6 @@ static const char *const hctx_state_name[] = {
 	HCTX_STATE_NAME(STOPPED),
 	HCTX_STATE_NAME(TAG_ACTIVE),
 	HCTX_STATE_NAME(SCHED_RESTART),
-	HCTX_STATE_NAME(START_ON_RUN),
 };
 #undef HCTX_STATE_NAME
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index e3d02af79010..8743e9105612 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1563,40 +1563,14 @@ static void blk_mq_run_work_fn(struct work_struct *work)
 	hctx = container_of(work, struct blk_mq_hw_ctx, run_work.work);
 
 	/*
-	 * If we are stopped, don't run the queue. The exception is if
-	 * BLK_MQ_S_START_ON_RUN is set. For that case, we auto-clear
-	 * the STOPPED bit and run it.
+	 * If we are stopped, don't run the queue.
 	 */
-	if (test_bit(BLK_MQ_S_STOPPED, &hctx->state)) {
-		if (!test_bit(BLK_MQ_S_START_ON_RUN, &hctx->state))
-			return;
-
-		clear_bit(BLK_MQ_S_START_ON_RUN, &hctx->state);
+	if (test_bit(BLK_MQ_S_STOPPED, &hctx->state))
 		clear_bit(BLK_MQ_S_STOPPED, &hctx->state);
-	}
 
 	__blk_mq_run_hw_queue(hctx);
 }
 
-
-void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs)
-{
-	if (WARN_ON_ONCE(!blk_mq_hw_queue_mapped(hctx)))
-		return;
-
-	/*
-	 * Stop the hw queue, then modify currently delayed work.
-	 * This should prevent us from running the queue prematurely.
-	 * Mark the queue as auto-clearing STOPPED when it runs.
-	 */
-	blk_mq_stop_hw_queue(hctx);
-	set_bit(BLK_MQ_S_START_ON_RUN, &hctx->state);
-	kblockd_mod_delayed_work_on(blk_mq_hctx_next_cpu(hctx),
-					&hctx->run_work,
-					msecs_to_jiffies(msecs));
-}
-EXPORT_SYMBOL(blk_mq_delay_queue);
-
 static inline void __blk_mq_insert_req_list(struct blk_mq_hw_ctx *hctx,
 					    struct request *rq,
 					    bool at_head)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 8efcf49796a3..e3986f4b3461 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -183,7 +183,6 @@ enum {
 	BLK_MQ_S_STOPPED	= 0,
 	BLK_MQ_S_TAG_ACTIVE	= 1,
 	BLK_MQ_S_SCHED_RESTART	= 2,
-	BLK_MQ_S_START_ON_RUN	= 3,
 
 	BLK_MQ_MAX_DEPTH	= 10240,
 
@@ -270,7 +269,6 @@ void blk_mq_unquiesce_queue(struct request_queue *q);
 void blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs);
 bool blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, 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_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);
-- 
2.9.5

[PATCH 6/8] blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()

[Ming Lei]

There are several reasons for removing the check:

1) blk_mq_hw_queue_mapped() returns true always now since each hctx
may be mapped by one CPU at least

2) when there isn't any online CPU mapped to this hctx, there won't
be any IO queued to this CPU, blk_mq_run_hw_queue() only runs queue
if there is IO queued to this hctx

3) If __blk_mq_delay_run_hw_queue() is called by blk_mq_delay_run_hw_queue(),
which is run from blk_mq_dispatch_rq_list() or scsi_mq_get_budget(), and
the hctx to be handled has to be mapped.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8743e9105612..3b4ce83a0ba2 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1394,9 +1394,6 @@ static int blk_mq_hctx_next_cpu(struct blk_mq_hw_ctx *hctx)
 static void __blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async,
 					unsigned long msecs)
 {
-	if (WARN_ON_ONCE(!blk_mq_hw_queue_mapped(hctx)))
-		return;
-
 	if (unlikely(blk_mq_hctx_stopped(hctx)))
 		return;
 
-- 
2.9.5

[PATCH 7/8] blk-mq: reimplement blk_mq_hw_queue_mapped

[Ming Lei]

Now the actual meaning of queue mapped is that if there is any online
CPU mapped to this hctx, so implement blk_mq_hw_queue_mapped() in this
way.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.h b/block/blk-mq.h
index 88c558f71819..502af371b83b 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -181,7 +181,7 @@ static inline bool blk_mq_hctx_stopped(struct blk_mq_hw_ctx *hctx)
 
 static inline bool blk_mq_hw_queue_mapped(struct blk_mq_hw_ctx *hctx)
 {
-	return hctx->nr_ctx && hctx->tags;
+	return cpumask_first_and(hctx->cpumask, cpu_online_mask) < nr_cpu_ids;
 }
 
 void blk_mq_in_flight(struct request_queue *q, struct hd_struct *part,
-- 
2.9.5

[PATCH 8/8] blk-mq: remove code for dealing with remapping queue

[Ming Lei]

Firstly, from commit 4b855ad37194 ("blk-mq: Create hctx for each present CPU),
blk-mq doesn't remap queue any more after CPU topo is changed.

Secondly, set->nr_hw_queues can't be bigger than nr_cpu_ids, and now we map
all possible CPUs to hw queues, so at least one CPU is mapped to each hctx.

So queue mapping has became static and fixed just like percpu variable, and
we don't need to handle queue remapping any more.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Stefan Haberland <sth@linux.vnet.ibm.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 34 +++-------------------------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3b4ce83a0ba2..c3964a79638e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2330,7 +2330,7 @@ static void blk_mq_free_map_and_requests(struct blk_mq_tag_set *set,
 
 static void blk_mq_map_swqueue(struct request_queue *q)
 {
-	unsigned int i, hctx_idx;
+	unsigned int i;
 	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_ctx *ctx;
 	struct blk_mq_tag_set *set = q->tag_set;
@@ -2347,23 +2347,8 @@ static void blk_mq_map_swqueue(struct request_queue *q)
 
 	/*
 	 * Map software to hardware queues.
-	 *
-	 * If the cpu isn't present, the cpu is mapped to first hctx.
 	 */
 	for_each_possible_cpu(i) {
-		hctx_idx = q->mq_map[i];
-		/* unmapped hw queue can be remapped after CPU topo changed */
-		if (!set->tags[hctx_idx] &&
-		    !__blk_mq_alloc_rq_map(set, hctx_idx)) {
-			/*
-			 * If tags initialization fail for some hctx,
-			 * that hctx won't be brought online.  In this
-			 * case, remap the current ctx to hctx[0] which
-			 * is guaranteed to always have tags allocated
-			 */
-			q->mq_map[i] = 0;
-		}
-
 		ctx = per_cpu_ptr(q->queue_ctx, i);
 		hctx = blk_mq_map_queue(q, i);
 
@@ -2375,21 +2360,8 @@ static void blk_mq_map_swqueue(struct request_queue *q)
 	mutex_unlock(&q->sysfs_lock);
 
 	queue_for_each_hw_ctx(q, hctx, i) {
-		/*
-		 * If no software queues are mapped to this hardware queue,
-		 * disable it and free the request entries.
-		 */
-		if (!hctx->nr_ctx) {
-			/* Never unmap queue 0.  We need it as a
-			 * fallback in case of a new remap fails
-			 * allocation
-			 */
-			if (i && set->tags[i])
-				blk_mq_free_map_and_requests(set, i);
-
-			hctx->tags = NULL;
-			continue;
-		}
+		/* every hctx should get mapped by at least one CPU */
+		WARN_ON(!hctx->nr_ctx);
 
 		hctx->tags = set->tags[i];
 		WARN_ON(!hctx->tags);
-- 
2.9.5

Re: [PATCH 1/8] blk-mq: make sure that correct hctx->next_cpu is set

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:07PM +0800, Ming Lei wrote:
> >From commit 20e4d81393196 (blk-mq: simplify queue mapping & schedule
> with each possisble CPU), one hctx can be mapped from all offline CPUs,
> then hctx->next_cpu can be set as wrong.
> 
> This patch fixes this issue by making hctx->next_cpu pointing to the
> first CPU in hctx->cpumask if all CPUs in hctx->cpumask are offline.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/8] blk-mq: don't keep offline CPUs mapped to hctx 0

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:08PM +0800, Ming Lei wrote:
> >From commit 4b855ad37194 ("blk-mq: Create hctx for each present CPU),
> blk-mq doesn't remap queue after CPU topo is changed, that said when
> some of these offline CPUs become online, they are still mapped to
> hctx 0, then hctx 0 may become the bottleneck of IO dispatch and
> completion.
> 
> This patch sets up the mapping from the beginning, and aligns to
> queue mapping for PCI device (blk_mq_pci_map_queues()).

Please kill the now pointless cpu_to_queue_index function.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/8] blk-mq: avoid to write intermediate result to hctx->next_cpu

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:09PM +0800, Ming Lei wrote:
> This patch figures out the final selected CPU, then writes
> it to hctx->next_cpu once, then we can avoid to intermediate
> next cpu observed from other dispatch paths.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/8] blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:10PM +0800, Ming Lei wrote:
> This patch introduces helper of blk_mq_hw_queue_first_cpu() for
> figuring out the hctx's first cpu, and code duplication can be
> avoided.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/8] blk-mq: remove blk_mq_delay_queue()

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:11PM +0800, Ming Lei wrote:
> No driver uses this interface any more, so remove it.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 6/8] blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:12PM +0800, Ming Lei wrote:
> There are several reasons for removing the check:
> 
> 1) blk_mq_hw_queue_mapped() returns true always now since each hctx
> may be mapped by one CPU at least

Sounds like we should remove it, then..

The patch looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 7/8] blk-mq: reimplement blk_mq_hw_queue_mapped

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:13PM +0800, Ming Lei wrote:
> Now the actual meaning of queue mapped is that if there is any online
> CPU mapped to this hctx, so implement blk_mq_hw_queue_mapped() in this
> way.

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 8/8] blk-mq: remove code for dealing with remapping queue

[Christoph Hellwig]

On Sun, Apr 08, 2018 at 05:48:14PM +0800, Ming Lei wrote:
> Firstly, from commit 4b855ad37194 ("blk-mq: Create hctx for each present CPU),
> blk-mq doesn't remap queue any more after CPU topo is changed.
> 
> Secondly, set->nr_hw_queues can't be bigger than nr_cpu_ids, and now we map
> all possible CPUs to hw queues, so at least one CPU is mapped to each hctx.
> 
> So queue mapping has became static and fixed just like percpu variable, and
> we don't need to handle queue remapping any more.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 0/8] blk-mq: fix and improve queue mapping

[Christian Borntraeger]

On 04/08/2018 11:48 AM, Ming Lei wrote:
> Hi Jens,
> 
> The first two patches fix issues about queue mapping.

> 
> The other 6 patches improve queue mapping for blk-mq.
> 
> Christian, this patches should fix your issue, so please give
> a test, and the patches can be found in the following tree:
> 
> 	https://github.com/ming1/linux/commits/v4.17-rc-blk-fix_mapping_v1

Looks good. With all patches applied, I can no longer reproduce any warning message.
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

> 
> Thanks,
> Ming
> 
> Ming Lei (8):
>   blk-mq: make sure that correct hctx->next_cpu is set
>   blk-mq: don't keep offline CPUs mapped to hctx 0
>   blk-mq: avoid to write intermediate result to hctx->next_cpu
>   blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu
>   blk-mq: remove blk_mq_delay_queue()
>   blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()
>   blk-mq: reimplement blk_mq_hw_queue_mapped
>   blk-mq: remove code for dealing with remapping queue
> 
>  block/blk-mq-cpumap.c  |   5 ---
>  block/blk-mq-debugfs.c |   1 -
>  block/blk-mq.c         | 101 +++++++++++--------------------------------------
>  block/blk-mq.h         |   2 +-
>  include/linux/blk-mq.h |   2 -
>  5 files changed, 24 insertions(+), 87 deletions(-)
> 

Re: [PATCH 1/8] blk-mq: make sure that correct hctx->next_cpu is set

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/8] blk-mq: don't keep offline CPUs mapped to hctx 0

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 3/8] blk-mq: avoid to write intermediate result to hctx->next_cpu

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 4/8] blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 5/8] blk-mq: remove blk_mq_delay_queue()

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 6/8] blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 7/8] blk-mq: reimplement blk_mq_hw_queue_mapped

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 8/8] blk-mq: remove code for dealing with remapping queue

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 0/8] blk-mq: fix and improve queue mapping

[Jens Axboe]

On 4/8/18 3:48 AM, Ming Lei wrote:
> Hi Jens,
> 
> The first two patches fix issues about queue mapping.
> 
> The other 6 patches improve queue mapping for blk-mq.
> 
> Christian, this patches should fix your issue, so please give
> a test, and the patches can be found in the following tree:
> 
> 	https://github.com/ming1/linux/commits/v4.17-rc-blk-fix_mapping_v1
> 
> Thanks,
> Ming
> 
> Ming Lei (8):
>   blk-mq: make sure that correct hctx->next_cpu is set
>   blk-mq: don't keep offline CPUs mapped to hctx 0
>   blk-mq: avoid to write intermediate result to hctx->next_cpu
>   blk-mq: introduce blk_mq_hw_queue_first_cpu() to figure out first cpu
>   blk-mq: remove blk_mq_delay_queue()
>   blk-mq: don't check queue mapped in __blk_mq_delay_run_hw_queue()
>   blk-mq: reimplement blk_mq_hw_queue_mapped
>   blk-mq: remove code for dealing with remapping queue

Thanks Ming, applied for this series.

-- 
Jens Axboe

Re: [PATCH 0/8] blk-mq: fix and improve queue mapping

[Stefan Haberland]

On 08.04.2018 11:48, Ming Lei wrote:
> Hi Jens,
>
> The first two patches fix issues about queue mapping.
>
> The other 6 patches improve queue mapping for blk-mq.
>
> Christian, this patches should fix your issue, so please give
> a test, and the patches can be found in the following tree:
>
> 	https://github.com/ming1/linux/commits/v4.17-rc-blk-fix_mapping_v1
>
> Thanks,
> Ming

Hi Ming,

thanks for the patches.
Unfortunately I was not able to try them with your git tree since I ran 
into the same problem, that Li Wang reported today.
But your patches applied to our local git tree seem to work fine. A 
small DASD regression test showed no issues or warnings.

Regards,
Stefan