Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1656810 > unrolled thread

[PATCH 7/8] blk-mq: create hctx for each present CPU

Started byChristoph Hellwig <hch@lst.de>
First post2017-06-03 16:10 +0200
Last post2017-06-08 09:00 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 7/8] blk-mq: create hctx for each present CPU Christoph Hellwig <hch@lst.de> - 2017-06-03 16:10 +0200
    Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Sagi Grimberg <sagi@grimberg.me> - 2017-06-04 17:20 +0200
    Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Ming Lei <ming.lei@redhat.com> - 2017-06-07 11:20 +0200
      Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Christoph Hellwig <hch@lst.de> - 2017-06-07 21:10 +0200
        Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Ming Lei <ming.lei@redhat.com> - 2017-06-08 04:30 +0200
    Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Omar Sandoval <osandov@osandov.com> - 2017-06-08 00:10 +0200
      Re: [PATCH 7/8] blk-mq: create hctx for each present CPU Christoph Hellwig <hch@lst.de> - 2017-06-08 09:00 +0200

#1656810 — [PATCH 7/8] blk-mq: create hctx for each present CPU

FromChristoph Hellwig <hch@lst.de>
Date2017-06-03 16:10 +0200
Subject[PATCH 7/8] blk-mq: create hctx for each present CPU
Message-ID<tOhUB-81i-7@gated-at.bofh.it>
Currently we only create hctx for online CPUs, which can lead to a lot
of churn due to frequent soft offline / online operations.  Instead
allocate one for each present CPU to avoid this and dramatically simplify
the code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c             | 120 +++++----------------------------------------
 block/blk-mq.h             |   5 --
 include/linux/cpuhotplug.h |   1 -
 3 files changed, 11 insertions(+), 115 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1bcccedcc74f..66ca9a090984 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -37,9 +37,6 @@
 #include "blk-wbt.h"
 #include "blk-mq-sched.h"
 
-static DEFINE_MUTEX(all_q_mutex);
-static LIST_HEAD(all_q_list);
-
 static void blk_mq_poll_stats_start(struct request_queue *q);
 static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
 static void __blk_mq_stop_hw_queues(struct request_queue *q, bool sync);
@@ -1966,8 +1963,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
 		INIT_LIST_HEAD(&__ctx->rq_list);
 		__ctx->queue = q;
 
-		/* If the cpu isn't online, the cpu is mapped to first hctx */
-		if (!cpu_online(i))
+		/* If the cpu isn't present, the cpu is mapped to first hctx */
+		if (!cpu_present(i))
 			continue;
 
 		hctx = blk_mq_map_queue(q, i);
@@ -2010,8 +2007,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,
-			       const struct cpumask *online_mask)
+static void blk_mq_map_swqueue(struct request_queue *q)
 {
 	unsigned int i, hctx_idx;
 	struct blk_mq_hw_ctx *hctx;
@@ -2029,13 +2025,11 @@ static void blk_mq_map_swqueue(struct request_queue *q,
 	}
 
 	/*
-	 * Map software to hardware queues
+	 * Map software to hardware queues.
+	 *
+	 * If the cpu isn't present, the cpu is mapped to first hctx.
 	 */
-	for_each_possible_cpu(i) {
-		/* If the cpu isn't online, the cpu is mapped to first hctx */
-		if (!cpumask_test_cpu(i, online_mask))
-			continue;
-
+	for_each_present_cpu(i) {
 		hctx_idx = q->mq_map[i];
 		/* unmapped hw queue can be remapped after CPU topo changed */
 		if (!set->tags[hctx_idx] &&
@@ -2321,16 +2315,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 		blk_queue_softirq_done(q, set->ops->complete);
 
 	blk_mq_init_cpu_queues(q, set->nr_hw_queues);
-
-	get_online_cpus();
-	mutex_lock(&all_q_mutex);
-
-	list_add_tail(&q->all_q_node, &all_q_list);
 	blk_mq_add_queue_tag_set(set, q);
-	blk_mq_map_swqueue(q, cpu_online_mask);
-
-	mutex_unlock(&all_q_mutex);
-	put_online_cpus();
+	blk_mq_map_swqueue(q);
 
 	if (!(set->flags & BLK_MQ_F_NO_SCHED)) {
 		int ret;
@@ -2356,18 +2342,12 @@ void blk_mq_free_queue(struct request_queue *q)
 {
 	struct blk_mq_tag_set	*set = q->tag_set;
 
-	mutex_lock(&all_q_mutex);
-	list_del_init(&q->all_q_node);
-	mutex_unlock(&all_q_mutex);
-
 	blk_mq_del_queue_tag_set(q);
-
 	blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
 }
 
 /* Basically redo blk_mq_init_queue with queue frozen */
-static void blk_mq_queue_reinit(struct request_queue *q,
-				const struct cpumask *online_mask)
+static void blk_mq_queue_reinit(struct request_queue *q)
 {
 	WARN_ON_ONCE(!atomic_read(&q->mq_freeze_depth));
 
@@ -2380,76 +2360,12 @@ static void blk_mq_queue_reinit(struct request_queue *q,
 	 * involves free and re-allocate memory, worthy doing?)
 	 */
 
-	blk_mq_map_swqueue(q, online_mask);
+	blk_mq_map_swqueue(q);
 
 	blk_mq_sysfs_register(q);
 	blk_mq_debugfs_register_hctxs(q);
 }
 
-/*
- * New online cpumask which is going to be set in this hotplug event.
- * Declare this cpumasks as global as cpu-hotplug operation is invoked
- * one-by-one and dynamically allocating this could result in a failure.
- */
-static struct cpumask cpuhp_online_new;
-
-static void blk_mq_queue_reinit_work(void)
-{
-	struct request_queue *q;
-
-	mutex_lock(&all_q_mutex);
-	/*
-	 * We need to freeze and reinit all existing queues.  Freezing
-	 * involves synchronous wait for an RCU grace period and doing it
-	 * one by one may take a long time.  Start freezing all queues in
-	 * one swoop and then wait for the completions so that freezing can
-	 * take place in parallel.
-	 */
-	list_for_each_entry(q, &all_q_list, all_q_node)
-		blk_freeze_queue_start(q);
-	list_for_each_entry(q, &all_q_list, all_q_node)
-		blk_mq_freeze_queue_wait(q);
-
-	list_for_each_entry(q, &all_q_list, all_q_node)
-		blk_mq_queue_reinit(q, &cpuhp_online_new);
-
-	list_for_each_entry(q, &all_q_list, all_q_node)
-		blk_mq_unfreeze_queue(q);
-
-	mutex_unlock(&all_q_mutex);
-}
-
-static int blk_mq_queue_reinit_dead(unsigned int cpu)
-{
-	cpumask_copy(&cpuhp_online_new, cpu_online_mask);
-	blk_mq_queue_reinit_work();
-	return 0;
-}
-
-/*
- * Before hotadded cpu starts handling requests, new mappings must be
- * established.  Otherwise, these requests in hw queue might never be
- * dispatched.
- *
- * For example, there is a single hw queue (hctx) and two CPU queues (ctx0
- * for CPU0, and ctx1 for CPU1).
- *
- * Now CPU1 is just onlined and a request is inserted into ctx1->rq_list
- * and set bit0 in pending bitmap as ctx1->index_hw is still zero.
- *
- * And then while running hw queue, blk_mq_flush_busy_ctxs() finds bit0 is set
- * in pending bitmap and tries to retrieve requests in hctx->ctxs[0]->rq_list.
- * But htx->ctxs[0] is a pointer to ctx0, so the request in ctx1->rq_list is
- * ignored.
- */
-static int blk_mq_queue_reinit_prepare(unsigned int cpu)
-{
-	cpumask_copy(&cpuhp_online_new, cpu_online_mask);
-	cpumask_set_cpu(cpu, &cpuhp_online_new);
-	blk_mq_queue_reinit_work();
-	return 0;
-}
-
 static int __blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
 {
 	int i;
@@ -2660,7 +2576,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 	blk_mq_update_queue_map(set);
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
 		blk_mq_realloc_hw_ctxs(set, q);
-		blk_mq_queue_reinit(q, cpu_online_mask);
+		blk_mq_queue_reinit(q);
 	}
 
 	list_for_each_entry(q, &set->tag_list, tag_set_list)
@@ -2876,24 +2792,10 @@ bool blk_mq_poll(struct request_queue *q, blk_qc_t cookie)
 }
 EXPORT_SYMBOL_GPL(blk_mq_poll);
 
-void blk_mq_disable_hotplug(void)
-{
-	mutex_lock(&all_q_mutex);
-}
-
-void blk_mq_enable_hotplug(void)
-{
-	mutex_unlock(&all_q_mutex);
-}
-
 static int __init blk_mq_init(void)
 {
 	cpuhp_setup_state_multi(CPUHP_BLK_MQ_DEAD, "block/mq:dead", NULL,
 				blk_mq_hctx_notify_dead);
-
-	cpuhp_setup_state_nocalls(CPUHP_BLK_MQ_PREPARE, "block/mq:prepare",
-				  blk_mq_queue_reinit_prepare,
-				  blk_mq_queue_reinit_dead);
 	return 0;
 }
 subsys_initcall(blk_mq_init);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index cc67b48e3551..558df56544d2 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -56,11 +56,6 @@ void __blk_mq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 				bool at_head);
 void blk_mq_insert_requests(struct blk_mq_hw_ctx *hctx, struct blk_mq_ctx *ctx,
 				struct list_head *list);
-/*
- * CPU hotplug helpers
- */
-void blk_mq_enable_hotplug(void);
-void blk_mq_disable_hotplug(void);
 
 /*
  * CPU -> queue mappings
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index c15f22c54535..7f815d915977 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -58,7 +58,6 @@ enum cpuhp_state {
 	CPUHP_XEN_EVTCHN_PREPARE,
 	CPUHP_ARM_SHMOBILE_SCU_PREPARE,
 	CPUHP_SH_SH3X_PREPARE,
-	CPUHP_BLK_MQ_PREPARE,
 	CPUHP_NET_FLOW_PREPARE,
 	CPUHP_TOPOLOGY_PREPARE,
 	CPUHP_NET_IUCV_PREPARE,
-- 
2.11.0

[toc] | [next] | [standalone]


#1657100

FromSagi Grimberg <sagi@grimberg.me>
Date2017-06-04 17:20 +0200
Message-ID<tOFtT-6QY-15@gated-at.bofh.it>
In reply to#1656810
Nice cleanup!

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

[toc] | [prev] | [next] | [standalone]


#1659569

FromMing Lei <ming.lei@redhat.com>
Date2017-06-07 11:20 +0200
Message-ID<tPFia-4ID-3@gated-at.bofh.it>
In reply to#1656810
On Sat, Jun 03, 2017 at 04:04:02PM +0200, Christoph Hellwig wrote:
> Currently we only create hctx for online CPUs, which can lead to a lot
> of churn due to frequent soft offline / online operations.  Instead
> allocate one for each present CPU to avoid this and dramatically simplify
> the code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-mq.c             | 120 +++++----------------------------------------
>  block/blk-mq.h             |   5 --
>  include/linux/cpuhotplug.h |   1 -
>  3 files changed, 11 insertions(+), 115 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 1bcccedcc74f..66ca9a090984 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -37,9 +37,6 @@
>  #include "blk-wbt.h"
>  #include "blk-mq-sched.h"
>  
> -static DEFINE_MUTEX(all_q_mutex);
> -static LIST_HEAD(all_q_list);
> -
>  static void blk_mq_poll_stats_start(struct request_queue *q);
>  static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
>  static void __blk_mq_stop_hw_queues(struct request_queue *q, bool sync);
> @@ -1966,8 +1963,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
>  		INIT_LIST_HEAD(&__ctx->rq_list);
>  		__ctx->queue = q;
>  
> -		/* If the cpu isn't online, the cpu is mapped to first hctx */
> -		if (!cpu_online(i))
> +		/* If the cpu isn't present, the cpu is mapped to first hctx */
> +		if (!cpu_present(i))
>  			continue;
>  
>  		hctx = blk_mq_map_queue(q, i);
> @@ -2010,8 +2007,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,
> -			       const struct cpumask *online_mask)
> +static void blk_mq_map_swqueue(struct request_queue *q)
>  {
>  	unsigned int i, hctx_idx;
>  	struct blk_mq_hw_ctx *hctx;
> @@ -2029,13 +2025,11 @@ static void blk_mq_map_swqueue(struct request_queue *q,
>  	}
>  
>  	/*
> -	 * Map software to hardware queues
> +	 * Map software to hardware queues.
> +	 *
> +	 * If the cpu isn't present, the cpu is mapped to first hctx.
>  	 */
> -	for_each_possible_cpu(i) {
> -		/* If the cpu isn't online, the cpu is mapped to first hctx */
> -		if (!cpumask_test_cpu(i, online_mask))
> -			continue;
> -
> +	for_each_present_cpu(i) {
>  		hctx_idx = q->mq_map[i];
>  		/* unmapped hw queue can be remapped after CPU topo changed */
>  		if (!set->tags[hctx_idx] &&
> @@ -2321,16 +2315,8 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
>  		blk_queue_softirq_done(q, set->ops->complete);
>  
>  	blk_mq_init_cpu_queues(q, set->nr_hw_queues);
> -
> -	get_online_cpus();
> -	mutex_lock(&all_q_mutex);
> -
> -	list_add_tail(&q->all_q_node, &all_q_list);
>  	blk_mq_add_queue_tag_set(set, q);
> -	blk_mq_map_swqueue(q, cpu_online_mask);
> -
> -	mutex_unlock(&all_q_mutex);
> -	put_online_cpus();
> +	blk_mq_map_swqueue(q);
>  
>  	if (!(set->flags & BLK_MQ_F_NO_SCHED)) {
>  		int ret;
> @@ -2356,18 +2342,12 @@ void blk_mq_free_queue(struct request_queue *q)
>  {
>  	struct blk_mq_tag_set	*set = q->tag_set;
>  
> -	mutex_lock(&all_q_mutex);
> -	list_del_init(&q->all_q_node);
> -	mutex_unlock(&all_q_mutex);
> -
>  	blk_mq_del_queue_tag_set(q);
> -
>  	blk_mq_exit_hw_queues(q, set, set->nr_hw_queues);
>  }
>  
>  /* Basically redo blk_mq_init_queue with queue frozen */
> -static void blk_mq_queue_reinit(struct request_queue *q,
> -				const struct cpumask *online_mask)
> +static void blk_mq_queue_reinit(struct request_queue *q)
>  {
>  	WARN_ON_ONCE(!atomic_read(&q->mq_freeze_depth));
>  
> @@ -2380,76 +2360,12 @@ static void blk_mq_queue_reinit(struct request_queue *q,
>  	 * involves free and re-allocate memory, worthy doing?)
>  	 */
>  
> -	blk_mq_map_swqueue(q, online_mask);
> +	blk_mq_map_swqueue(q);
>  
>  	blk_mq_sysfs_register(q);
>  	blk_mq_debugfs_register_hctxs(q);
>  }
>  
> -/*
> - * New online cpumask which is going to be set in this hotplug event.
> - * Declare this cpumasks as global as cpu-hotplug operation is invoked
> - * one-by-one and dynamically allocating this could result in a failure.
> - */
> -static struct cpumask cpuhp_online_new;
> -
> -static void blk_mq_queue_reinit_work(void)
> -{
> -	struct request_queue *q;
> -
> -	mutex_lock(&all_q_mutex);
> -	/*
> -	 * We need to freeze and reinit all existing queues.  Freezing
> -	 * involves synchronous wait for an RCU grace period and doing it
> -	 * one by one may take a long time.  Start freezing all queues in
> -	 * one swoop and then wait for the completions so that freezing can
> -	 * take place in parallel.
> -	 */
> -	list_for_each_entry(q, &all_q_list, all_q_node)
> -		blk_freeze_queue_start(q);
> -	list_for_each_entry(q, &all_q_list, all_q_node)
> -		blk_mq_freeze_queue_wait(q);
> -
> -	list_for_each_entry(q, &all_q_list, all_q_node)
> -		blk_mq_queue_reinit(q, &cpuhp_online_new);
> -
> -	list_for_each_entry(q, &all_q_list, all_q_node)
> -		blk_mq_unfreeze_queue(q);
> -
> -	mutex_unlock(&all_q_mutex);
> -}
> -
> -static int blk_mq_queue_reinit_dead(unsigned int cpu)
> -{
> -	cpumask_copy(&cpuhp_online_new, cpu_online_mask);
> -	blk_mq_queue_reinit_work();
> -	return 0;
> -}
> -
> -/*
> - * Before hotadded cpu starts handling requests, new mappings must be
> - * established.  Otherwise, these requests in hw queue might never be
> - * dispatched.
> - *
> - * For example, there is a single hw queue (hctx) and two CPU queues (ctx0
> - * for CPU0, and ctx1 for CPU1).
> - *
> - * Now CPU1 is just onlined and a request is inserted into ctx1->rq_list
> - * and set bit0 in pending bitmap as ctx1->index_hw is still zero.
> - *
> - * And then while running hw queue, blk_mq_flush_busy_ctxs() finds bit0 is set
> - * in pending bitmap and tries to retrieve requests in hctx->ctxs[0]->rq_list.
> - * But htx->ctxs[0] is a pointer to ctx0, so the request in ctx1->rq_list is
> - * ignored.
> - */
> -static int blk_mq_queue_reinit_prepare(unsigned int cpu)
> -{
> -	cpumask_copy(&cpuhp_online_new, cpu_online_mask);
> -	cpumask_set_cpu(cpu, &cpuhp_online_new);
> -	blk_mq_queue_reinit_work();
> -	return 0;
> -}
> -
>  static int __blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
>  {
>  	int i;
> @@ -2660,7 +2576,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>  	blk_mq_update_queue_map(set);
>  	list_for_each_entry(q, &set->tag_list, tag_set_list) {
>  		blk_mq_realloc_hw_ctxs(set, q);
> -		blk_mq_queue_reinit(q, cpu_online_mask);
> +		blk_mq_queue_reinit(q);
>  	}
>  
>  	list_for_each_entry(q, &set->tag_list, tag_set_list)
> @@ -2876,24 +2792,10 @@ bool blk_mq_poll(struct request_queue *q, blk_qc_t cookie)
>  }
>  EXPORT_SYMBOL_GPL(blk_mq_poll);
>  
> -void blk_mq_disable_hotplug(void)
> -{
> -	mutex_lock(&all_q_mutex);
> -}
> -
> -void blk_mq_enable_hotplug(void)
> -{
> -	mutex_unlock(&all_q_mutex);
> -}
> -
>  static int __init blk_mq_init(void)
>  {
>  	cpuhp_setup_state_multi(CPUHP_BLK_MQ_DEAD, "block/mq:dead", NULL,
>  				blk_mq_hctx_notify_dead);
> -
> -	cpuhp_setup_state_nocalls(CPUHP_BLK_MQ_PREPARE, "block/mq:prepare",
> -				  blk_mq_queue_reinit_prepare,
> -				  blk_mq_queue_reinit_dead);

Hi Christoph,

One thing not sure is that we may need to handle new CPU hotplug
after initialization. Without the CPU hotplug handler, system may
not scale well when more CPUs are added to sockets.

Another thing is that I don't see how NVMe handles this situation,
blk_mq_update_nr_hw_queues() is called in nvme_reset_work(), so
that means RESET need to be triggered after new CPUs are added to
system? I have tried to add new CPUs runtime on Qemu, and not see
new hw queues are added no matter this patchset is applied or not.

Thanks,
Ming

[toc] | [prev] | [next] | [standalone]


#1660116

FromChristoph Hellwig <hch@lst.de>
Date2017-06-07 21:10 +0200
Message-ID<tPOv7-2fF-11@gated-at.bofh.it>
In reply to#1659569
On Wed, Jun 07, 2017 at 05:10:46PM +0800, Ming Lei wrote:
> One thing not sure is that we may need to handle new CPU hotplug
> after initialization. Without the CPU hotplug handler, system may
> not scale well when more CPUs are added to sockets.

Adding physical CPUs to sockets is a very rare activity, and we
should not optimize for it.  Taking CPUs that are physically present
offline and online is the case that is interesting, and that's what
this patchset changes.

> Another thing is that I don't see how NVMe handles this situation,
> blk_mq_update_nr_hw_queues() is called in nvme_reset_work(), so
> that means RESET need to be triggered after new CPUs are added to
> system?

Yes.

> I have tried to add new CPUs runtime on Qemu, and not see
> new hw queues are added no matter this patchset is applied or not.

Do you even see the CPUs in your VM?  For physical hotplug you'll
need to reserve spots in the cpumap beforehand.

[toc] | [prev] | [next] | [standalone]


#1660672

FromMing Lei <ming.lei@redhat.com>
Date2017-06-08 04:30 +0200
Message-ID<tPVmW-6Cm-15@gated-at.bofh.it>
In reply to#1660116
On Wed, Jun 07, 2017 at 09:06:59PM +0200, Christoph Hellwig wrote:
> On Wed, Jun 07, 2017 at 05:10:46PM +0800, Ming Lei wrote:
> > One thing not sure is that we may need to handle new CPU hotplug
> > after initialization. Without the CPU hotplug handler, system may
> > not scale well when more CPUs are added to sockets.
> 
> Adding physical CPUs to sockets is a very rare activity, and we
> should not optimize for it.  Taking CPUs that are physically present
> offline and online is the case that is interesting, and that's what
> this patchset changes.

Yeah, I understand. It depends if there are real use cases in which
system can't be rebooted, but sometimes CPUs need to be added or removed
for some reasons. Looks CONFIG_HOTPLUG_CPU addresses this requirement,

From searching on google, looks there are some for virtualization,
and both Qemu and Vmware support that.

> 
> > Another thing is that I don't see how NVMe handles this situation,
> > blk_mq_update_nr_hw_queues() is called in nvme_reset_work(), so
> > that means RESET need to be triggered after new CPUs are added to
> > system?
> 
> Yes.

Unfortunately I don't see the Reset comes when new CPU cores becomes
online.

> 
> > I have tried to add new CPUs runtime on Qemu, and not see
> > new hw queues are added no matter this patchset is applied or not.
> 
> Do you even see the CPUs in your VM?  For physical hotplug you'll
> need to reserve spots in the cpumap beforehand.

Yes, I can add the new CPUs in console of Qemu, and these CPUs become
present first in VM, then switched online via command line after the hotplug,
but number of hw queues doesn't change.

Please see the following log:

root@ming:/sys/kernel/debug/block# echo "before adding one CPU"
before adding one CPU
root@ming:/sys/kernel/debug/block#
root@ming:/sys/kernel/debug/block# lscpu | head -n 10
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                4
On-line CPU(s) list:   0-3
Thread(s) per core:    1
Core(s) per socket:    2
Socket(s):             2
NUMA node(s):          1
Vendor ID:             GenuineIntel
root@ming:/sys/kernel/debug/block# ls nvme0n1/
hctx0  hctx1  hctx2  hctx3  poll_stat  requeue_list  state
root@ming:/sys/kernel/debug/block#
root@ming:/sys/kernel/debug/block# echo "one CPU will be added"
one CPU will be added
root@ming:/sys/kernel/debug/block#
device: 'cpu4': device_add
bus: 'cpu': add device cpu4
PM: Adding info for cpu:cpu4
CPU4 has been hot-added
bus: 'cpu': driver_probe_device: matched device cpu4 with driver processor
bus: 'cpu': really_probe: probing driver processor with device cpu4
processor cpu4: no default pinctrl state
devices_kset: Moving cpu4 to end of list
driver: 'processor': driver_bound: bound to device 'cpu4'
bus: 'cpu': really_probe: bound device cpu4 to driver processor

root@ming:/sys/kernel/debug/block# echo 1 > /sys/devices/system/cpu/cpu4/online
smpboot: Booting Node 0 Processor 4 APIC 0x4/sys/devices/system/cpu/cpu4/online
kvm-clock: cpu 4, msr 2:7ff5e101, secondary cpu clock
TSC ADJUST compensate: CPU4 observed 349440008297 warp. Adjust: 2147483647
TSC ADJUST compensate: CPU4 observed 347292524597 warp. Adjust: 2147483647
TSC synchronization [CPU#1 -> CPU#4]:
Measured 347292524561 cycles TSC warp between CPUs, turning off TSC clock.
tsc: Marking TSC unstable due to check_tsc_sync_source failed
KVM setup async PF for cpu 4
kvm-stealtime: cpu 4, msr 27fd0d900
Will online and init hotplugged CPU: 4
device: 'cooling_device4': device_add
PM: Adding info for No Bus:cooling_device4
device: 'cache': device_add
PM: Adding info for No Bus:cache
device: 'index0': device_add
PM: Adding info for No Bus:index0
device: 'index1': device_add
PM: Adding info for No Bus:index1
device: 'index2': device_add
PM: Adding info for No Bus:index2
device: 'index3': device_add
PM: Adding info for No Bus:index3
device: 'machinecheck4': device_add
bus: 'machinecheck': add device machinecheck4
PM: Adding info for machinecheck:machinecheck4
root@ming:/sys/kernel/debug/block# lscpu | head -n 10
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                5
On-line CPU(s) list:   0-4
Thread(s) per core:    1
Core(s) per socket:    1
Socket(s):             3
NUMA node(s):          1
Vendor ID:             GenuineIntel
root@ming:/sys/kernel/debug/block# ls nvme0n1/
hctx0  hctx1  hctx2  hctx3  poll_stat  requeue_list  state


Thanks,
Ming

[toc] | [prev] | [next] | [standalone]


#1660280

FromOmar Sandoval <osandov@osandov.com>
Date2017-06-08 00:10 +0200
Message-ID<tPRjk-44i-9@gated-at.bofh.it>
In reply to#1656810
On Sat, Jun 03, 2017 at 04:04:02PM +0200, Christoph Hellwig wrote:
> Currently we only create hctx for online CPUs, which can lead to a lot
> of churn due to frequent soft offline / online operations.  Instead
> allocate one for each present CPU to avoid this and dramatically simplify
> the code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Oh man, this cleanup is great. Did you run blktests on this? block/008
does a bunch of hotplugging while I/O is running.

[toc] | [prev] | [next] | [standalone]


#1660804

FromChristoph Hellwig <hch@lst.de>
Date2017-06-08 09:00 +0200
Message-ID<tPZAf-Nh-27@gated-at.bofh.it>
In reply to#1660280
On Wed, Jun 07, 2017 at 03:04:11PM -0700, Omar Sandoval wrote:
> On Sat, Jun 03, 2017 at 04:04:02PM +0200, Christoph Hellwig wrote:
> > Currently we only create hctx for online CPUs, which can lead to a lot
> > of churn due to frequent soft offline / online operations.  Instead
> > allocate one for each present CPU to avoid this and dramatically simplify
> > the code.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Oh man, this cleanup is great. Did you run blktests on this? block/008
> does a bunch of hotplugging while I/O is running.

I haven't run blktests yet, in fact when I did the work blktests didn't
exist yet.   But thanks for the reminder, I'll run it now.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web