[PATCH] NVMe: handle ioremap failure

[PATCH] NVMe: handle ioremap failure

[Keith Busch]

Decrement the number of queues required for doorbell remapping until
the memory is successfully mapped for that size.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 711b51c..aaada3d 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1686,7 +1686,17 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 	db_bar_size = 4096 + ((nr_io_queues + 1) << (dev->db_stride + 3));
 	if (db_bar_size > 8192) {
 		iounmap(dev->bar);
-		dev->bar = ioremap(pci_resource_start(pdev, 0), db_bar_size);
+		while (nr_io_queues > 0) {
+			dev->bar = ioremap(pci_resource_start(pdev, 0), db_bar_size);
+			if (!dev->bar)
+				nr_io_queues--;
+			else
+				break;
+			db_bar_size = 4096 + ((nr_io_queues + 1) << (dev->db_stride + 3));
+		}
+		if (!dev->bar)
+			return -ENOMEM;
+
 		dev->dbs = ((void __iomem *)dev->bar) + 4096;
 		dev->queues[0]->q_db = dev->dbs;
 	}
-- 
1.7.0.4

[PATCH] NVMe: handle ioremap failure

[Matthew Wilcox]

On Mon, Jul 01, 2013@10:23:40AM -0600, Keith Busch wrote:
> Decrement the number of queues required for doorbell remapping until
> the memory is successfully mapped for that size.

Cool, thanks!

> -		dev->bar = ioremap(pci_resource_start(pdev, 0), db_bar_size);
> +		while (nr_io_queues > 0) {
> +			dev->bar = ioremap(pci_resource_start(pdev, 0), db_bar_size);
*cough* *checkpatch* *line length*
> +			if (!dev->bar)
> +				nr_io_queues--;
> +			else
> +				break;

I think this might more naturally be written as:

			dev->bar = ioremap(pci_resource_start(pdev, 0),
								db_bar_size);
			if (dev->bar)
				break;

			nr_io_queues--;
> +			db_bar_size = 4096 + ((nr_io_queues + 1) << (dev->db_stride + 3));
> +		}
> +		if (!dev->bar)
> +			return -ENOMEM;
> +

Although maybe we should instead redo the entire thing as ...

static size_t db_bar_size(struct nvme_dev *dev, unsigned nr_io_queues)
{
	return 4096 + ((nr_io_queues + 1) << (dev->db_stride + 3));
}

	size = db_bar_size(dev, nr_io_queues);
	if (size > 8192) {
		iounmap(dev->bar);
		do {
			dev->bar = ioremap(pci_resource_start(pdev, 0), size);
			if (dev->bar)
				break;
			if (!--nr_io_queues)
				return -ENOMEM;
			size = db_bar_size(dev, --nr_io_queues);
		} while (1);
		dev->dbs = ((void __iomem *)dev->bar) + 4096;
		dev->queues[0]->q_db = dev->dbs;
	}

Although, now I'm wondering whether we free_irq() the admin queue exactly
the right number of times.  We might need to set ->queue_count to -1
while we've called free_irq() on the admin queue ... but then we'll need
to handle the queue memory freeing correctly.  Awkward ...