[PATCH 0/5] hpsa: driver updates, Oct 25, 2011

[PATCH 0/5] hpsa: driver updates, Oct 25, 2011

[Stephen M. Cameron]

The following series implements:

Scott Teel (2):
      hpsa: rename HPSA_MAX_SCSI_DEVS_PER_HBA
      hpsa: fix potential array overflow in hpsa_update_scsi_devices

Stephen M. Cameron (3):
      hpsa: set max sectors instead of taking the default
      hpsa: remove unused busy_initializing and busy_scanning
      hpsa: fix flush cache transfer length


 drivers/scsi/hpsa.c     |   37 ++++++++++++++++++++-----------------
 drivers/scsi/hpsa.h     |    5 +----
 drivers/scsi/hpsa_cmd.h |    5 ++++-
 3 files changed, 25 insertions(+), 22 deletions(-)

-- 
-- steve

[PATCH 1/5] hpsa: set max sectors instead of taking the default

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Set the max hardware sectors in the SCSI host template to 8192
to allow for larger i/o's (8192 is the same limit the cciss
driver currently has.)

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index b200b73..9b12aea 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -484,6 +484,7 @@ static struct scsi_host_template hpsa_driver_template = {
 #endif
 	.sdev_attrs = hpsa_sdev_attrs,
 	.shost_attrs = hpsa_shost_attrs,
+	.max_sectors = 8192,
 };
 
 

[PATCH 2/5] hpsa: remove unused busy_initializing and busy_scanning

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |    3 ---
 drivers/scsi/hpsa.h |    2 --
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 9b12aea..d59515d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4123,7 +4123,6 @@ reinit_after_soft_reset:
 		return -ENOMEM;
 
 	h->pdev = pdev;
-	h->busy_initializing = 1;
 	h->intr_mode = hpsa_simple_mode ? SIMPLE_MODE_INT : PERF_MODE_INT;
 	INIT_LIST_HEAD(&h->cmpQ);
 	INIT_LIST_HEAD(&h->reqQ);
@@ -4232,7 +4231,6 @@ reinit_after_soft_reset:
 
 	hpsa_hba_inquiry(h);
 	hpsa_register_scsi(h);	/* hook ourselves into SCSI subsystem */
-	h->busy_initializing = 0;
 	return 1;
 
 clean4:
@@ -4241,7 +4239,6 @@ clean4:
 	free_irq(h->intr[h->intr_mode], h);
 clean2:
 clean1:
-	h->busy_initializing = 0;
 	kfree(h);
 	return rc;
 }
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 7f53cea..111b79e 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -95,8 +95,6 @@ struct ctlr_info {
 	unsigned long  		*cmd_pool_bits;
 	int			nr_allocs;
 	int			nr_frees;
-	int			busy_initializing;
-	int			busy_scanning;
 	int			scan_finished;
 	spinlock_t		scan_lock;
 	wait_queue_head_t	scan_wait_queue;

[PATCH 3/5] hpsa: rename HPSA_MAX_SCSI_DEVS_PER_HBA

[Stephen M. Cameron]

From: Scott Teel <scott.teel@hp.com>

Rename HPSA_MAX_SCSI_DEVS_PER_HBA to HPSA_MAX_DEVICES

Signed-off-by: Scott Teel <scott.teel@hp.com>
Acked-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |   23 ++++++++++-------------
 drivers/scsi/hpsa.h |    4 ++--
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index d59515d..d359186 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -567,16 +567,16 @@ static int hpsa_find_target_lun(struct ctlr_info *h,
 	 * assumes h->devlock is held
 	 */
 	int i, found = 0;
-	DECLARE_BITMAP(lun_taken, HPSA_MAX_SCSI_DEVS_PER_HBA);
+	DECLARE_BITMAP(lun_taken, HPSA_MAX_DEVICES);
 
-	memset(&lun_taken[0], 0, HPSA_MAX_SCSI_DEVS_PER_HBA >> 3);
+	memset(&lun_taken[0], 0, HPSA_MAX_DEVICES >> 3);
 
 	for (i = 0; i < h->ndevices; i++) {
 		if (h->dev[i]->bus == bus && h->dev[i]->target != -1)
 			set_bit(h->dev[i]->target, lun_taken);
 	}
 
-	for (i = 0; i < HPSA_MAX_SCSI_DEVS_PER_HBA; i++) {
+	for (i = 0; i < HPSA_MAX_DEVICES; i++) {
 		if (!test_bit(i, lun_taken)) {
 			/* *bus = 1; */
 			*target = i;
@@ -599,7 +599,7 @@ static int hpsa_scsi_add_entry(struct ctlr_info *h, int hostno,
 	unsigned char addr1[8], addr2[8];
 	struct hpsa_scsi_dev_t *sd;
 
-	if (n >= HPSA_MAX_SCSI_DEVS_PER_HBA) {
+	if (n >= HPSA_MAX_DEVICES) {
 		dev_err(&h->pdev->dev, "too many devices, some will be "
 			"inaccessible.\n");
 		return -1;
@@ -674,7 +674,7 @@ static void hpsa_scsi_replace_entry(struct ctlr_info *h, int hostno,
 	struct hpsa_scsi_dev_t *removed[], int *nremoved)
 {
 	/* assumes h->devlock is held */
-	BUG_ON(entry < 0 || entry >= HPSA_MAX_SCSI_DEVS_PER_HBA);
+	BUG_ON(entry < 0 || entry >= HPSA_MAX_DEVICES);
 	removed[*nremoved] = h->dev[entry];
 	(*nremoved)++;
 
@@ -703,7 +703,7 @@ static void hpsa_scsi_remove_entry(struct ctlr_info *h, int hostno, int entry,
 	int i;
 	struct hpsa_scsi_dev_t *sd;
 
-	BUG_ON(entry < 0 || entry >= HPSA_MAX_SCSI_DEVS_PER_HBA);
+	BUG_ON(entry < 0 || entry >= HPSA_MAX_DEVICES);
 
 	sd = h->dev[entry];
 	removed[*nremoved] = h->dev[entry];
@@ -815,10 +815,8 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	int nadded, nremoved;
 	struct Scsi_Host *sh = NULL;
 
-	added = kzalloc(sizeof(*added) * HPSA_MAX_SCSI_DEVS_PER_HBA,
-		GFP_KERNEL);
-	removed = kzalloc(sizeof(*removed) * HPSA_MAX_SCSI_DEVS_PER_HBA,
-		GFP_KERNEL);
+	added = kzalloc(sizeof(*added) * HPSA_MAX_DEVICES, GFP_KERNEL);
+	removed = kzalloc(sizeof(*removed) * HPSA_MAX_DEVICES, GFP_KERNEL);
 
 	if (!added || !removed) {
 		dev_warn(&h->pdev->dev, "out of memory in "
@@ -1847,8 +1845,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 	int raid_ctlr_position;
 	DECLARE_BITMAP(lunzerobits, HPSA_MAX_TARGETS_PER_CTLR);
 
-	currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_SCSI_DEVS_PER_HBA,
-		GFP_KERNEL);
+	currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);
 	physdev_list = kzalloc(reportlunsize, GFP_KERNEL);
 	logdev_list = kzalloc(reportlunsize, GFP_KERNEL);
 	tmpdevice = kzalloc(sizeof(*tmpdevice), GFP_KERNEL);
@@ -1957,7 +1954,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		default:
 			break;
 		}
-		if (ncurrent >= HPSA_MAX_SCSI_DEVS_PER_HBA)
+		if (ncurrent >= HPSA_MAX_DEVICES)
 			break;
 	}
 	adjust_hpsa_scsi_table(h, hostno, currentsd, ncurrent);
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 111b79e..4de9f71 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -102,8 +102,8 @@ struct ctlr_info {
 	struct Scsi_Host *scsi_host;
 	spinlock_t devlock; /* to protect hba[ctlr]->dev[];  */
 	int ndevices; /* number of used elements in .dev[] array. */
-#define HPSA_MAX_SCSI_DEVS_PER_HBA 256
-	struct hpsa_scsi_dev_t *dev[HPSA_MAX_SCSI_DEVS_PER_HBA];
+#define HPSA_MAX_DEVICES 256
+	struct hpsa_scsi_dev_t *dev[HPSA_MAX_DEVICES];
 	/*
 	 * Performant mode tables.
 	 */

[PATCH 4/5] hpsa: fix potential array overflow in hpsa_update_scsi_devices

[Stephen M. Cameron]

From: Scott Teel <scott.teel@hp.com>

The currentsd[] array in hpsa_update_scsi_devices had room for
256 devices.  The code was iterating over however many physical
and logical devices plus an additional number of possible external
MSA2XXX controllers, which together could potentially exceed 256.

We increased the size of the currentsd array to 1024 + 1024 + 32 + 1
elements to reflect a reasonable maximum possible number of devices
which might be encountered.  We also don't just walk off the end
of the array if the array controller reports more devices than we
are prepared to handle, we just ignore the excessive devices.

Signed-off-by: Scott Teel <scott.teel@hp.com>
Acked-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c     |    8 +++++++-
 drivers/scsi/hpsa.h     |    1 -
 drivers/scsi/hpsa_cmd.h |    5 ++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index d359186..646516f 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1734,7 +1734,6 @@ static int add_msa2xxx_enclosure_device(struct ctlr_info *h,
 	if (is_scsi_rev_5(h))
 		return 0; /* p1210m doesn't need to do this. */
 
-#define MAX_MSA2XXX_ENCLOSURES 32
 	if (*nmsa2xxx_enclosures >= MAX_MSA2XXX_ENCLOSURES) {
 		dev_warn(&h->pdev->dev, "Maximum number of MSA2XXX "
 			"enclosures exceeded.  Check your hardware "
@@ -1868,6 +1867,13 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 
 	/* Allocate the per device structures */
 	for (i = 0; i < ndevs_to_allocate; i++) {
+		if (i >= HPSA_MAX_DEVICES) {
+			dev_warn(&h->pdev->dev, "maximum devices (%d) exceeded."
+				"  %d devices ignored.\n", HPSA_MAX_DEVICES,
+				ndevs_to_allocate - HPSA_MAX_DEVICES);
+			break;
+		}
+
 		currentsd[i] = kzalloc(sizeof(*currentsd[i]), GFP_KERNEL);
 		if (!currentsd[i]) {
 			dev_warn(&h->pdev->dev, "out of memory at %s:%d\n",
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 4de9f71..73858bc 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -102,7 +102,6 @@ struct ctlr_info {
 	struct Scsi_Host *scsi_host;
 	spinlock_t devlock; /* to protect hba[ctlr]->dev[];  */
 	int ndevices; /* number of used elements in .dev[] array. */
-#define HPSA_MAX_DEVICES 256
 	struct hpsa_scsi_dev_t *dev[HPSA_MAX_DEVICES];
 	/*
 	 * Performant mode tables.
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index 55d741b..3fd4715 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -123,8 +123,11 @@ union u64bit {
 
 /* FIXME this is a per controller value (barf!) */
 #define HPSA_MAX_TARGETS_PER_CTLR 16
-#define HPSA_MAX_LUN 256
+#define HPSA_MAX_LUN 1024
 #define HPSA_MAX_PHYS_LUN 1024
+#define MAX_MSA2XXX_ENCLOSURES 32
+#define HPSA_MAX_DEVICES (HPSA_MAX_PHYS_LUN + HPSA_MAX_LUN + \
+	MAX_MSA2XXX_ENCLOSURES + 1) /* + 1 is for the controller itself */
 
 /* SCSI-3 Commands */
 #pragma pack(1)

[PATCH 5/5] hpsa: fix flush cache transfer length

[Stephen M. Cameron]

From: Stephen M. Cameron <scameron@beardog.cce.hp.com>

We weren't filling in the transfer length of the
flush cache command (it transfers 4 bytes of zeroes).
Firmware didn't seem to be bothered by this, but it
should be fixed.

Signed-off-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
---
 drivers/scsi/hpsa.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 646516f..45477b0 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2876,6 +2876,8 @@ static void fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
 			c->Request.Timeout = 0;
 			c->Request.CDB[0] = BMIC_WRITE;
 			c->Request.CDB[6] = BMIC_CACHE_FLUSH;
+			c->Request.CDB[7] = (size >> 8) & 0xFF;
+			c->Request.CDB[8] = size & 0xFF;
 			break;
 		case TEST_UNIT_READY:
 			c->Request.CDBLen = 6;

Re: [PATCH 4/5] hpsa: fix potential array overflow in hpsa_update_scsi_devices

[James Bottomley]

On Wed, 2011-10-26 at 16:21 -0500, Stephen M. Cameron wrote:
> From: Scott Teel <scott.teel@hp.com>
> 
> The currentsd[] array in hpsa_update_scsi_devices had room for
> 256 devices.  The code was iterating over however many physical
> and logical devices plus an additional number of possible external
> MSA2XXX controllers, which together could potentially exceed 256.
> 
> We increased the size of the currentsd array to 1024 + 1024 + 32 + 1
> elements to reflect a reasonable maximum possible number of devices
> which might be encountered.  We also don't just walk off the end
> of the array if the array controller reports more devices than we
> are prepared to handle, we just ignore the excessive devices.
> 
> Signed-off-by: Scott Teel <scott.teel@hp.com>
> Acked-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>

This should be Signed-off-by not Acked-by.  The reason is that the
signoffs track whose hands the patch passes through.  If you send
Scott's patch to me, it must have your signoff.  If Scott sends the
patch directly to me and then you OK it on the list, then I'll add
Acked-by.  I've assumed your acquiescence to correcting this in my tree.

James

Re: [PATCH 4/5] hpsa: fix potential array overflow in hpsa_update_scsi_devices

[scameron]

On Sun, Oct 30, 2011 at 02:16:22PM +0400, James Bottomley wrote:
> On Wed, 2011-10-26 at 16:21 -0500, Stephen M. Cameron wrote:
> > From: Scott Teel <scott.teel@hp.com>
> > 
> > The currentsd[] array in hpsa_update_scsi_devices had room for
> > 256 devices.  The code was iterating over however many physical
> > and logical devices plus an additional number of possible external
> > MSA2XXX controllers, which together could potentially exceed 256.
> > 
> > We increased the size of the currentsd array to 1024 + 1024 + 32 + 1
> > elements to reflect a reasonable maximum possible number of devices
> > which might be encountered.  We also don't just walk off the end
> > of the array if the array controller reports more devices than we
> > are prepared to handle, we just ignore the excessive devices.
> > 
> > Signed-off-by: Scott Teel <scott.teel@hp.com>
> > Acked-by: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> 
> This should be Signed-off-by not Acked-by.  The reason is that the
> signoffs track whose hands the patch passes through.  If you send
> Scott's patch to me, it must have your signoff.  If Scott sends the
> patch directly to me and then you OK it on the list, then I'll add
> Acked-by.  I've assumed your acquiescence to correcting this in my tree.
> 
> James
> 

Thanks James.  

I don't often have patches from other people to forward, so I tend to
forget how it's supposed to work.   I'll try to remember that for next time.

-- steve