* Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
@ 2013-05-01 21:37 Ramachandra Rao Gajula
2013-05-01 21:50 ` Keith Busch
0 siblings, 1 reply; 4+ messages in thread
From: Ramachandra Rao Gajula @ 2013-05-01 21:37 UTC (permalink / raw)
---
Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
Changes made to consider Keith's review comments
---
Signed-off-by: Ramachandra Rao Gajula <rama at fastorsystems.com>
---
drivers/block/nvme-core.c | 32 ++++++++++++++++++++++++++++----
1 files changed, 28 insertions(+), 4 deletions(-)
---
diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index bcb81c8..0f6225b 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1431,7 +1431,7 @@ static int set_queue_count(struct nvme_dev *dev,
int count)
static int nvme_setup_io_queues(struct nvme_dev *dev)
{
- int result, cpu, i, nr_io_queues, db_bar_size, q_depth;
+ int result, cpu, i, nr_io_queues, db_bar_size, q_depth, q_cnt;
nr_io_queues = num_online_cpus();
result = set_queue_count(dev, nr_io_queues);
@@ -1440,6 +1440,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
if (result < nr_io_queues)
nr_io_queues = result;
+ q_cnt = nr_io_queues;
/* Deregister the admin queue's interrupt */
free_irq(dev->entry[0].vector, dev->queues[0]);
@@ -1463,10 +1464,27 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
nr_io_queues = result;
continue;
} else {
- nr_io_queues = 1;
+ nr_io_queues = q_cnt;
break;
}
}
+ /* if MSI-X failed, then try for MSI for nr_io_queues vectors */
+ if (!dev->pci_dev->msix_enabled) {
+ for (;;) {
+ result = pci_enable_msi_block(dev->pci_dev,
nr_io_queues);
+ if (result == 0) {
+ for (i = 0; i < nr_io_queues; i++)
+ dev->entry[i].vector = i +
dev->pci_dev->irq;
+ break;
+ } else if (result > 0) {
+ nr_io_queues = result;
+ continue;
+ } else {
+ nr_io_queues = 1;
+ break;
+ }
+ }
+ }
result = queue_request_irq(dev, dev->queues[0], "nvme admin");
/* XXX: handle failure here */
@@ -1651,7 +1669,10 @@ static void nvme_free_dev(struct kref *kref)
{
struct nvme_dev *dev = container_of(kref, struct nvme_dev, kref);
nvme_dev_remove(dev);
- pci_disable_msix(dev->pci_dev);
+ if (dev->pci_dev->msi_enabled)
+ pci_disable_msi(dev->pci_dev);
+ else
+ pci_disable_msix(dev->pci_dev);
iounmap(dev->bar);
nvme_release_instance(dev);
nvme_release_prp_pools(dev);
@@ -1778,7 +1799,10 @@ static int nvme_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
unmap:
iounmap(dev->bar);
disable_msix:
- pci_disable_msix(pdev);
+ if (dev->pci_dev->msi_enabled)
+ pci_disable_msi(pdev);
+ else
+ pci_disable_msix(pdev);
nvme_release_instance(dev);
nvme_release_prp_pools(dev);
disable:
---
^ permalink raw reply related [flat|nested] 4+ messages in thread* Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
2013-05-01 21:37 Added MSI supoort (includes MSI Multiple) to Linux NVME Driver Ramachandra Rao Gajula
@ 2013-05-01 21:50 ` Keith Busch
0 siblings, 0 replies; 4+ messages in thread
From: Keith Busch @ 2013-05-01 21:50 UTC (permalink / raw)
I think your mail text editor or email client appears to be messing with
the text of your patch somewhat. Just wanted to let you know since this
won't apply cleanly without the maintainer fixing the wrapped lines.
I use 'git format-patch' to generate the patches and 'git send-email'
to post patches to the mailing list.
On Wed, 1 May 2013, Ramachandra Rao Gajula wrote:
> ---
> Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
> Changes made to consider Keith's review comments
> ---
> Signed-off-by: Ramachandra Rao Gajula <rama at fastorsystems.com>
> ---
> drivers/block/nvme-core.c | 32 ++++++++++++++++++++++++++++----
> 1 files changed, 28 insertions(+), 4 deletions(-)
> ---
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index bcb81c8..0f6225b 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -1431,7 +1431,7 @@ static int set_queue_count(struct nvme_dev *dev,
> int count)
>
> static int nvme_setup_io_queues(struct nvme_dev *dev)
> {
> - int result, cpu, i, nr_io_queues, db_bar_size, q_depth;
> + int result, cpu, i, nr_io_queues, db_bar_size, q_depth, q_cnt;
>
> nr_io_queues = num_online_cpus();
> result = set_queue_count(dev, nr_io_queues);
> @@ -1440,6 +1440,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
> if (result < nr_io_queues)
> nr_io_queues = result;
>
> + q_cnt = nr_io_queues;
> /* Deregister the admin queue's interrupt */
> free_irq(dev->entry[0].vector, dev->queues[0]);
>
> @@ -1463,10 +1464,27 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
> nr_io_queues = result;
> continue;
> } else {
> - nr_io_queues = 1;
> + nr_io_queues = q_cnt;
> break;
> }
> }
> + /* if MSI-X failed, then try for MSI for nr_io_queues vectors */
> + if (!dev->pci_dev->msix_enabled) {
> + for (;;) {
> + result = pci_enable_msi_block(dev->pci_dev,
> nr_io_queues);
> + if (result == 0) {
> + for (i = 0; i < nr_io_queues; i++)
> + dev->entry[i].vector = i +
> dev->pci_dev->irq;
> + break;
> + } else if (result > 0) {
> + nr_io_queues = result;
> + continue;
> + } else {
> + nr_io_queues = 1;
> + break;
> + }
> + }
> + }
>
> result = queue_request_irq(dev, dev->queues[0], "nvme admin");
> /* XXX: handle failure here */
> @@ -1651,7 +1669,10 @@ static void nvme_free_dev(struct kref *kref)
> {
> struct nvme_dev *dev = container_of(kref, struct nvme_dev, kref);
> nvme_dev_remove(dev);
> - pci_disable_msix(dev->pci_dev);
> + if (dev->pci_dev->msi_enabled)
> + pci_disable_msi(dev->pci_dev);
> + else
> + pci_disable_msix(dev->pci_dev);
> iounmap(dev->bar);
> nvme_release_instance(dev);
> nvme_release_prp_pools(dev);
> @@ -1778,7 +1799,10 @@ static int nvme_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
> unmap:
> iounmap(dev->bar);
> disable_msix:
> - pci_disable_msix(pdev);
> + if (dev->pci_dev->msi_enabled)
> + pci_disable_msi(pdev);
> + else
> + pci_disable_msix(pdev);
> nvme_release_instance(dev);
> nvme_release_prp_pools(dev);
> disable:
>
> ---
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
@ 2013-03-05 18:16 Ramachandra Rao Gajula
2013-04-30 22:13 ` Busch, Keith
0 siblings, 1 reply; 4+ messages in thread
From: Ramachandra Rao Gajula @ 2013-03-05 18:16 UTC (permalink / raw)
------
Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
------
diff --git a/drivers/block/nvme.c b/drivers/block/nvme.c
------
@@ -1416,6 +1416,7 @@ static int set_queue_count(struct nvme_dev *dev,
int count)
static int __devinit nvme_setup_io_queues(struct nvme_dev *dev)
{
int result, cpu, i, nr_io_queues, db_bar_size, q_depth;
+ int try_msi=0;
nr_io_queues = num_online_cpus();
result = set_queue_count(dev, nr_io_queues);
@@ -1436,21 +1437,42 @@ static int __devinit
nvme_setup_io_queues(struct nvme_dev *dev)
dev->queues[0]->q_db = dev->dbs;
}
+ /* init for MSI-X */
for (i = 0; i < nr_io_queues; i++)
dev->entry[i].entry = i;
for (;;) {
result = pci_enable_msix(dev->pci_dev, dev->entry,
nr_io_queues);
- if (result == 0) {
+ if (result == 0) { /* got all vectors */
+ dev->pci_dev.msix_enabled = 1;
break;
} else if (result > 0) {
nr_io_queues = result;
- continue;
- } else {
- nr_io_queues = 1;
+ continue; /* get as many as vectors as possible */
+ } else { /* MSI-X failed; so try MSI and if not, finally intx */
+ nr_io_queues = num_online_cpus();
+ try_msi = 1;
break;
}
}
+ /* if MSI-X failed, then try for MSI for nr_io_queues vectors */
+ if (try_msi) {
+ for (;;) {
+ result = pci_enable_msi_block(dev->pci_dev,nr_io_queues);
+ if (result == 0) {
+ dev->pci_dev.msi_enabled = 1;
+ for (i = 0; i < nr_io_queues; i++)
+ dev->entry[i].vector = i + dev->pci_dev->irq;
+ break;
+ } else if (result > 0) {
+ nr_io_queues = result;
+ continue; /* get as many messages as we can */
+ } else { /* no MSI, fall back to intx with just 1 queue */
+ nr_io_queues = 1;
+ break;
+ }
+ }
+ }
result = queue_request_irq(dev, dev->queues[0], "nvme admin");
/* XXX: handle failure here */
@@ -1662,12 +1684,12 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
result = nvme_setup_prp_pools(dev);
if (result)
- goto disable_msix;
+ goto free_instance;
dev->bar = ioremap(pci_resource_start(pdev, 0), 8192);
if (!dev->bar) {
result = -ENOMEM;
- goto disable_msix;
+ goto free_prps;
}
result = nvme_configure_admin_queue(dev);
@@ -1693,10 +1715,15 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
nvme_free_queues(dev);
unmap:
iounmap(dev->bar);
- disable_msix:
- pci_disable_msix(pdev);
- nvme_release_instance(dev);
+
+ if (dev->pci_dev.msi_enabled)
+ pci_disable_msi(pdev);
+ else
+ pci_disable_msix(pdev);
+ free_prps:
nvme_release_prp_pools(dev);
+ free_instance:
+ nvme_release_instance(dev);
disable:
pci_disable_device(pdev);
pci_release_regions(pdev);
@@ -1711,7 +1738,10 @@ static void __devexit nvme_remove(struct pci_dev *pdev)
{
struct nvme_dev *dev = pci_get_drvdata(pdev);
nvme_dev_remove(dev);
- pci_disable_msix(pdev);
+ if (dev->pci_dev.msi_enabled)
+ pci_disable_msi(pdev);
+ else
+ pci_disable_msix(pdev);
iounmap(dev->bar);
nvme_release_instance(dev);
nvme_release_prp_pools(dev);
@@ -1796,3 +1826,4 @@ MODULE_LICENSE("GPL");
MODULE_VERSION("0.8");
module_init(nvme_init);
module_exit(nvme_exit);
+
^ permalink raw reply [flat|nested] 4+ messages in thread* Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
2013-03-05 18:16 Ramachandra Rao Gajula
@ 2013-04-30 22:13 ` Busch, Keith
0 siblings, 0 replies; 4+ messages in thread
From: Busch, Keith @ 2013-04-30 22:13 UTC (permalink / raw)
On Mon, 5 Mar 2013, Ramachandra Rao Gajula wrote:
> ------
>
> Added MSI supoort (includes MSI Multiple) to Linux NVME Driver
>
> ------
Could you make this a proper git commit message, including Signed-off-by tag?
> @@ -1416,6 +1416,7 @@ static int set_queue_count(struct nvme_dev *dev, int count)
> static int __devinit nvme_setup_io_queues(struct nvme_dev *dev) {
> int result, cpu, i, nr_io_queues, db_bar_size, q_depth;
> + int try_msi=0;
I don't think you need this variable 'try_msi' when you can just check
pci_dev->msix_enabled to know if you should try msi.
> + if (result == 0) { /* got all vectors */
> + dev->pci_dev.msix_enabled = 1;
You shouldn't be setting this directly. pci_enable_msix will set the value if
it was successful.
> + if (result == 0) {
> + dev->pci_dev.msi_enabled = 1;
Same situation here as msix_enabled.
> result = nvme_setup_prp_pools(dev);
> if (result)
> - goto disable_msix;
> + goto free_instance;
>
> dev->bar = ioremap(pci_resource_start(pdev, 0), 8192);
> if (!dev->bar) {
> result = -ENOMEM;
> - goto disable_msix;
> + goto free_prps;
> }
Looks like you're fixing up some of the error out cases. Maybe separate this in
a different patch?
> @@ -1796,3 +1826,4 @@
> MODULE_LICENSE("GPL");
> MODULE_VERSION("0.8");
> module_init(nvme_init);
> module_exit(nvme_exit);
> +
Extra newline here at the end?
In general, code comments should be used to explain something that isn't
obvious from reading the code, and I think you've used comments a bit
liberally in this patch.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-05-01 21:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-01 21:37 Added MSI supoort (includes MSI Multiple) to Linux NVME Driver Ramachandra Rao Gajula
2013-05-01 21:50 ` Keith Busch
-- strict thread matches above, loose matches on Subject: below --
2013-03-05 18:16 Ramachandra Rao Gajula
2013-04-30 22:13 ` Busch, Keith
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox