* [PATCH 1/3] nvme/pci: Fix write and poll queue types
2019-12-06 17:13 [PATCH 0/3] nvme specialized queue fixes Keith Busch
@ 2019-12-06 17:13 ` Keith Busch
2019-12-06 17:13 ` [PATCH 2/3] nvme/pci Limit write queue sizes to possible cpus Keith Busch
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2019-12-06 17:13 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The number of poll or write queues should never be negative. Use unsigned
types so that it's not possible to have the driver allocate no queues.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/pci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 68701921dc23..7186f21ad899 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -66,14 +66,14 @@ static int io_queue_depth = 1024;
module_param_cb(io_queue_depth, &io_queue_depth_ops, &io_queue_depth, 0644);
MODULE_PARM_DESC(io_queue_depth, "set io queue depth, should >= 2");
-static int write_queues;
-module_param(write_queues, int, 0644);
+static unsigned int write_queues;
+module_param(write_queues, uint, 0644);
MODULE_PARM_DESC(write_queues,
"Number of queues to use for writes. If not set, reads and writes "
"will share a queue set.");
-static int poll_queues;
-module_param(poll_queues, int, 0644);
+static unsigned int poll_queues;
+module_param(poll_queues, uint, 0644);
MODULE_PARM_DESC(poll_queues, "Number of queues to use for polled IO.");
struct nvme_dev;
--
2.21.0
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] nvme/pci Limit write queue sizes to possible cpus
2019-12-06 17:13 [PATCH 0/3] nvme specialized queue fixes Keith Busch
2019-12-06 17:13 ` [PATCH 1/3] nvme/pci: Fix write and poll queue types Keith Busch
@ 2019-12-06 17:13 ` Keith Busch
2019-12-06 17:13 ` [PATCH 3/3] nvme/pci: Fix read queue count Keith Busch
2019-12-06 17:46 ` [PATCH 0/3] nvme specialized queue fixes Jens Axboe
3 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2019-12-06 17:13 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The driver can never use more queues of any type than the number of
possible CPUs. A larger parameter value causes the driver to allocate
more memory for IO queues than it could ever use. Limit the parameter
at module load time to the number of possible cpus.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/pci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 7186f21ad899..6b6452486155 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3146,6 +3146,9 @@ static int __init nvme_init(void)
BUILD_BUG_ON(sizeof(struct nvme_create_sq) != 64);
BUILD_BUG_ON(sizeof(struct nvme_delete_queue) != 64);
BUILD_BUG_ON(IRQ_AFFINITY_MAX_SETS < 2);
+
+ write_queues = min(write_queues, num_possible_cpus());
+ poll_queues = min(poll_queues, num_possible_cpus());
return pci_register_driver(&nvme_driver);
}
--
2.21.0
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] nvme/pci: Fix read queue count
2019-12-06 17:13 [PATCH 0/3] nvme specialized queue fixes Keith Busch
2019-12-06 17:13 ` [PATCH 1/3] nvme/pci: Fix write and poll queue types Keith Busch
2019-12-06 17:13 ` [PATCH 2/3] nvme/pci Limit write queue sizes to possible cpus Keith Busch
@ 2019-12-06 17:13 ` Keith Busch
2019-12-07 8:55 ` Ming Lei
2019-12-06 17:46 ` [PATCH 0/3] nvme specialized queue fixes Jens Axboe
3 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2019-12-06 17:13 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
If nvme.write_queues and poll_queues are specified, we expect the driver
may request more queues than CPUs if the device's queue count feature
is large enough. The driver, however, had been decreasing the number of
possible interrupt enabled queues, artifically limiting the number of
read queues even if the controller could support more.
The driver doesn't request more IO queues than CPUs for each queues type
anyway, so remove the cpu count comparison to the number of interrupt
enabled io queues.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/pci.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 6b6452486155..d3bed5df9ef1 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2062,7 +2062,6 @@ static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
.priv = dev,
};
unsigned int irq_queues, this_p_queues;
- unsigned int nr_cpus = num_possible_cpus();
/*
* Poll queues don't need interrupts, but we need at least one IO
@@ -2073,10 +2072,7 @@ static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
this_p_queues = nr_io_queues - 1;
irq_queues = 1;
} else {
- if (nr_cpus < nr_io_queues - this_p_queues)
- irq_queues = nr_cpus + 1;
- else
- irq_queues = nr_io_queues - this_p_queues + 1;
+ irq_queues = nr_io_queues - this_p_queues + 1;
}
dev->io_queues[HCTX_TYPE_POLL] = this_p_queues;
--
2.21.0
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] nvme/pci: Fix read queue count
2019-12-06 17:13 ` [PATCH 3/3] nvme/pci: Fix read queue count Keith Busch
@ 2019-12-07 8:55 ` Ming Lei
0 siblings, 0 replies; 7+ messages in thread
From: Ming Lei @ 2019-12-07 8:55 UTC (permalink / raw)
To: Keith Busch; +Cc: Minwoo Im, hch, linux-nvme, sagi
On 12/7/19, Keith Busch <kbusch@kernel.org> wrote:
> If nvme.write_queues and poll_queues are specified, we expect the driver
> may request more queues than CPUs if the device's queue count feature
> is large enough. The driver, however, had been decreasing the number of
> possible interrupt enabled queues, artifically limiting the number of
> read queues even if the controller could support more.
>
> The driver doesn't request more IO queues than CPUs for each queues type
> anyway, so remove the cpu count comparison to the number of interrupt
> enabled io queues.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/pci.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 6b6452486155..d3bed5df9ef1 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2062,7 +2062,6 @@ static int nvme_setup_irqs(struct nvme_dev *dev,
> unsigned int nr_io_queues)
> .priv = dev,
> };
> unsigned int irq_queues, this_p_queues;
> - unsigned int nr_cpus = num_possible_cpus();
>
> /*
> * Poll queues don't need interrupts, but we need at least one IO
> @@ -2073,10 +2072,7 @@ static int nvme_setup_irqs(struct nvme_dev *dev,
> unsigned int nr_io_queues)
> this_p_queues = nr_io_queues - 1;
> irq_queues = 1;
> } else {
> - if (nr_cpus < nr_io_queues - this_p_queues)
> - irq_queues = nr_cpus + 1;
> - else
> - irq_queues = nr_io_queues - this_p_queues + 1;
> + irq_queues = nr_io_queues - this_p_queues + 1;
> }
> dev->io_queues[HCTX_TYPE_POLL] = this_p_queues;
>
> --
> 2.21.0
Fixes: dad77d63903e("nvme-pci: adjust irq max_vector using num_possible_cpus()")
Thanks,
Ming Lei
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] nvme specialized queue fixes
2019-12-06 17:13 [PATCH 0/3] nvme specialized queue fixes Keith Busch
` (2 preceding siblings ...)
2019-12-06 17:13 ` [PATCH 3/3] nvme/pci: Fix read queue count Keith Busch
@ 2019-12-06 17:46 ` Jens Axboe
2019-12-06 17:58 ` Keith Busch
3 siblings, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2019-12-06 17:46 UTC (permalink / raw)
To: Keith Busch, linux-nvme, hch, sagi
On 12/6/19 10:13 AM, Keith Busch wrote:
> The nvme pci module had been allowing bad values to the specialized
> polled and write queues that could have caused the driver to allocate
> queues incorrectly or unoptimally.
>
> For example, on a system with 16 CPUs, if I have module parameter
> nvme.write_queues=17, we would get only 1 read queue no matter how many
> the drive supported:
>
> # dmesg | grep nvme | grep "poll queues"
> nvme nvme2: 16/1/2 default/read/poll queues
> nvme nvme1: 16/1/2 default/read/poll queues
> nvme nvme0: 16/1/2 default/read/poll queues
> nvme nvme3: 16/1/2 default/read/poll queues
>
> But after fixing:
>
> # dmesg | grep nvme | grep "poll queues"
> nvme nvme1: 16/16/2 default/read/poll queues
> nvme nvme2: 16/13/2 default/read/poll queues
> nvme nvme0: 16/16/2 default/read/poll queues
> nvme nvme3: 16/13/2 default/read/poll queues
>
> We just need to fix the calculation so that we don't throttle the total
> number of possible desired queues incorrectly. The first two patches
> ensure the module parameters are withing reasonable boundaries to
> simplify counting the number of interrupts we want to allocate.
>
> Keith Busch (3):
> nvme/pci: Fix write and poll queue types
> nvme/pci Limit write queue sizes to possible cpus
> nvme/pci: Fix read queue count
>
> drivers/nvme/host/pci.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
Looks good to me:
Reviewed-by: Jens Axboe <axboe@kernel.dk>
--
Jens Axboe
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread