Skip to contentSkip to conversation

NVMe: Remove remnants of cpu hotplug

PATCH New Sam Bradshaw 2 participants 5 messages Dec 4, 2014 – Dec 9, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 NVMe: Remove remnants of cpu hotplug

This patch cleans up some code inherited from the cpu hotplug handling, including correcting a bug where hctx->cpumask included non-schedulable cpus (backtrace attached).

Signed-off-by Selvan Mani <smani at micron.com>
Signed-off-by Sam Bradshaw <sbradshaw at micron.com>

---

drivers/block/nvme-core.c +1 -3
index bcbdf83..57ed698 100644
@@ -76,7 +76,6 @@ static LIST_HEAD(dev_list);
 static struct task_struct *nvme_thread;
 static struct workqueue_struct *nvme_workq;
 static wait_queue_head_t nvme_kthread_wait;
-static struct notifier_block nvme_nb;
 
 static void nvme_reset_failed_dev(struct work_struct *ws);
 static int nvme_process_cq(struct nvme_queue *nvmeq);
@@ -1981,7 +1980,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 	struct pci_dev *pdev = dev->pci_dev;
 	int result, i, vecs, nr_io_queues, size;
 
-	nr_io_queues = num_possible_cpus();
+	nr_io_queues = num_online_cpus();
 	result = set_queue_count(dev, nr_io_queues);
 	if (result <= 0)
 		return result;
@@ -2878,7 +2877,6 @@ static int __init nvme_init(void)
 static void __exit nvme_exit(void)
 {
 	pci_unregister_driver(&nvme_driver);
-	unregister_hotcpu_notifier(&nvme_nb);
 	unregister_blkdev(nvme_major, "nvme");
 	destroy_workqueue(nvme_workq);
 	BUG_ON(nvme_thread && !IS_ERR(nvme_thread));

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: bt
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20141204/645d4d01/attachment-0001.ksh>

maintainer
#1 lore raw

PATCH 1/1 NVMe: Remove remnants of cpu hotplug

Quoted 3 lines · Sam Bradshaw

This patch cleans up some code inherited from the cpu hotplug handling, including correcting a bug where hctx->cpumask included non-schedulable cpus (backtrace attached).

Getting rid of the notifier_block is definitely right, that should not be there anymore.

I'm curious with the num_possible -> num_online change. This should be handled by blk-mq, so no worries there. But how did it trigger the unset CPU in the run mask? Ran into the same issue here on a different test case, just curious if you looked into this.

Signature · 1 line
Jens Axboe
#2 lore raw

PATCH 1/1 NVMe: Remove remnants of cpu hotplug

Quoted 14 lines

-----Original Message-----
From: Jens Axboe [mailto:axboe at kernel.dk]
Sent: Thursday, December 04, 2014 9:57 AM
To: Sam Bradshaw (sbradshaw); linux-nvme at lists.infradead.org
Cc: Selvan Mani (smani) [CONT - Type 2]
Subject: Re: [PATCH] NVMe: Remove remnants of cpu hotplug

Quoted 3 lines · Sam Bradshaw

This patch cleans up some code inherited from the cpu hotplug handling, including correcting a bug where hctx->cpumask included non-schedulable cpus (backtrace attached).

Quoted 7 lines

Getting rid of the notifier_block is definitely right, that should not be there anymore.

I'm curious with the num_possible -> num_online change. This should be handled by blk-mq, so no worries there. But how did it trigger the unset CPU in the run mask? Ran into the same issue here on a different test case, just curious if you looked into this.

The systems that exhibit the problem are Dell R620 and R720 with only 1 cpu socket populated out of 2, which creates a difference between cpus online (24) and possible (32). I don't have direct access to these systems at the moment to verify this but my thinking is that a sw ctx is getting associated to an uninitialized hctx in the mapping step, which appears to be possible if the zeroth cpu in the cpus possible map is offline. In that case, the sw ctx maps to the zeroth hctx, which may not have that (or any) cpu in the hctx->cpu_mask. Maybe the default hctx shouldn't be the zeroth but instead the first that has at least one cpu associated with it?

#3 lore raw

PATCH 1/1 NVMe: Remove remnants of cpu hotplug

In reply to Jens Axboe
Quoted 5 lines

I'm curious with the num_possible -> num_online change. This should be handled by blk-mq, so no worries there. But how did it trigger the unset CPU in the run mask? Ran into the same issue here on a different test case, just curious if you looked into this.

After looking into it, the issue has been fixed on your for-3.19/core branch due to:

[PATCH] blk-mq: prevent unmapped hw queue from being scheduled

It's straightforward enough to test that the fix works by setting possible_cpus on the kernel command line to something larger than online cpus.

maintainer
#4 lore raw

PATCH 1/1 NVMe: Remove remnants of cpu hotplug

Quoted 6 lines · Sam Bradshaw (sbradshaw)
Quoted 5 lines

I'm curious with the num_possible -> num_online change. This should be handled by blk-mq, so no worries there. But how did it trigger the unset CPU in the run mask? Ran into the same issue here on a different test case, just curious if you looked into this.

Quoted 8 lines

After looking into it, the issue has been fixed on your for-3.19/core branch due to:

[PATCH] blk-mq: prevent unmapped hw queue from being scheduled

It's straightforward enough to test that the fix works by setting possible_cpus on the kernel command line to something larger than online cpus.

Great, that's much preferred :-)

Signature · 1 line
Jens Axboe