[PATCH V3 0/3] hpsa updates

[PATCH V3 0/3] hpsa updates

[Don Brace]

These patches are based on Linus's tree

The changes are:
 - add in a new offline volume status
 - limit the number of outstanding rescan operation
 - do not timeout reset operations

Changes since V1:
 - hpsa-update-check-for-volume-status
   - changed return type of hpsa_volume_offline from
     int to unsigned char
   - return more specific values.
   - removed unnecessary code
   Based on review from: Tomas Henzl <thenzl@redhat.com>

Changes since V2:
 - hpsa-limit-outstanding-rescans
   corrected a comment typo (alreay - already)

---

Don Brace (3):
      hpsa: update check for logical volume status
      hpsa: limit outstanding rescans
      hpsa: do not timeout reset operations


 drivers/scsi/hpsa.c     |   53 ++++++++++++++++++++++++++++-------------------
 drivers/scsi/hpsa.h     |    1 +
 drivers/scsi/hpsa_cmd.h |    2 ++
 3 files changed, 35 insertions(+), 21 deletions(-)

--
Signature

[PATCH V3 1/3] hpsa: update check for logical volume status

[Don Brace]

 - Add in a new case for volume offline. Resolves internal
   testing bug for multilun array management.
 - Return correct status for failed TURs.

Reviewed-by: Scott Benesh <scott.benesh@microsemi.com>
Reviewed-by: Scott Teel <scott.teel@microsemi.com>
Signed-off-by: Don Brace <don.brace@microsemi.com>
---
 drivers/scsi/hpsa.c     |   35 ++++++++++++++++-------------------
 drivers/scsi/hpsa_cmd.h |    2 ++
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 524a0c7..90b76c4 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3714,7 +3714,7 @@ static int hpsa_get_volume_status(struct ctlr_info *h,
  *  # (integer code indicating one of several NOT READY states
  *     describing why a volume is to be kept offline)
  */
-static int hpsa_volume_offline(struct ctlr_info *h,
+static unsigned char hpsa_volume_offline(struct ctlr_info *h,
 					unsigned char scsi3addr[])
 {
 	struct CommandList *c;
@@ -3735,7 +3735,7 @@ static int hpsa_volume_offline(struct ctlr_info *h,
 					DEFAULT_TIMEOUT);
 	if (rc) {
 		cmd_free(h, c);
-		return 0;
+		return HPSA_VPD_LV_STATUS_UNSUPPORTED;
 	}
 	sense = c->err_info->SenseInfo;
 	if (c->err_info->SenseLen > sizeof(c->err_info->SenseInfo))
@@ -3746,19 +3746,13 @@ static int hpsa_volume_offline(struct ctlr_info *h,
 	cmd_status = c->err_info->CommandStatus;
 	scsi_status = c->err_info->ScsiStatus;
 	cmd_free(h, c);
-	/* Is the volume 'not ready'? */
-	if (cmd_status != CMD_TARGET_STATUS ||
-		scsi_status != SAM_STAT_CHECK_CONDITION ||
-		sense_key != NOT_READY ||
-		asc != ASC_LUN_NOT_READY)  {
-		return 0;
-	}
 
 	/* Determine the reason for not ready state */
 	ldstat = hpsa_get_volume_status(h, scsi3addr);
 
 	/* Keep volume offline in certain cases: */
 	switch (ldstat) {
+	case HPSA_LV_FAILED:
 	case HPSA_LV_UNDERGOING_ERASE:
 	case HPSA_LV_NOT_AVAILABLE:
 	case HPSA_LV_UNDERGOING_RPI:
@@ -3780,7 +3774,7 @@ static int hpsa_volume_offline(struct ctlr_info *h,
 	default:
 		break;
 	}
-	return 0;
+	return HPSA_LV_OK;
 }
 
 /*
@@ -3853,10 +3847,10 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 	/* Do an inquiry to the device to see what it is. */
 	if (hpsa_scsi_do_inquiry(h, scsi3addr, 0, inq_buff,
 		(unsigned char) OBDR_TAPE_INQ_SIZE) != 0) {
-		/* Inquiry failed (msg printed already) */
 		dev_err(&h->pdev->dev,
-			"hpsa_update_device_info: inquiry failed\n");
-		rc = -EIO;
+			"%s: inquiry failed, device will be skipped.\n",
+			__func__);
+		rc = HPSA_INQUIRY_FAILED;
 		goto bail_out;
 	}
 
@@ -3885,15 +3879,19 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 	if ((this_device->devtype == TYPE_DISK ||
 		this_device->devtype == TYPE_ZBC) &&
 		is_logical_dev_addr_mode(scsi3addr)) {
-		int volume_offline;
+		unsigned char volume_offline;
 
 		hpsa_get_raid_level(h, scsi3addr, &this_device->raid_level);
 		if (h->fw_support & MISC_FW_RAID_OFFLOAD_BASIC)
 			hpsa_get_ioaccel_status(h, scsi3addr, this_device);
 		volume_offline = hpsa_volume_offline(h, scsi3addr);
-		if (volume_offline < 0 || volume_offline > 0xff)
-			volume_offline = HPSA_VPD_LV_STATUS_UNSUPPORTED;
-		this_device->volume_offline = volume_offline & 0xff;
+		if (volume_offline == HPSA_LV_FAILED) {
+			rc = HPSA_LV_FAILED;
+			dev_err(&h->pdev->dev,
+				"%s: LV failed, device will be skipped.\n",
+				__func__);
+			goto bail_out;
+		}
 	} else {
 		this_device->raid_level = RAID_UNKNOWN;
 		this_device->offload_config = 0;
@@ -4379,8 +4377,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
 			goto out;
 		}
 		if (rc) {
-			dev_warn(&h->pdev->dev,
-				"Inquiry failed, skipping device.\n");
+			h->drv_req_rescan = 1;
 			continue;
 		}
 
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index a584cdf..5961705 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -156,6 +156,7 @@
 #define CFGTBL_BusType_Fibre2G  0x00000200l
 
 /* VPD Inquiry types */
+#define HPSA_INQUIRY_FAILED		0x02
 #define HPSA_VPD_SUPPORTED_PAGES        0x00
 #define HPSA_VPD_LV_DEVICE_ID           0x83
 #define HPSA_VPD_LV_DEVICE_GEOMETRY     0xC1
@@ -166,6 +167,7 @@
 /* Logical volume states */
 #define HPSA_VPD_LV_STATUS_UNSUPPORTED			0xff
 #define HPSA_LV_OK                                      0x0
+#define HPSA_LV_FAILED					0x01
 #define HPSA_LV_NOT_AVAILABLE				0x0b
 #define HPSA_LV_UNDERGOING_ERASE			0x0F
 #define HPSA_LV_UNDERGOING_RPI				0x12

[PATCH V3 2/3] hpsa: limit outstanding rescans

[Don Brace]

avoid rescan storms. No need to queue another
if one is pending.

Reviewed-by: Scott Benesh <scott.benesh@microsemi.com>
Reviewed-by: Scott Teel <scott.teel@microsemi.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com> review
Signed-off-by: Don Brace <don.brace@microsemi.com>
---
 drivers/scsi/hpsa.c |   16 +++++++++++++++-
 drivers/scsi/hpsa.h |    1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 90b76c4..0a8ac68 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -5555,7 +5555,7 @@ static void hpsa_scan_complete(struct ctlr_info *h)
 
 	spin_lock_irqsave(&h->scan_lock, flags);
 	h->scan_finished = 1;
-	wake_up_all(&h->scan_wait_queue);
+	wake_up(&h->scan_wait_queue);
 	spin_unlock_irqrestore(&h->scan_lock, flags);
 }
 
@@ -5573,11 +5573,23 @@ static void hpsa_scan_start(struct Scsi_Host *sh)
 	if (unlikely(lockup_detected(h)))
 		return hpsa_scan_complete(h);
 
+	/*
+	 * If a scan is already waiting to run, no need to add another
+	 */
+	spin_lock_irqsave(&h->scan_lock, flags);
+	if (h->scan_waiting) {
+		spin_unlock_irqrestore(&h->scan_lock, flags);
+		return;
+	}
+
+	spin_unlock_irqrestore(&h->scan_lock, flags);
+
 	/* wait until any scan already in progress is finished. */
 	while (1) {
 		spin_lock_irqsave(&h->scan_lock, flags);
 		if (h->scan_finished)
 			break;
+		h->scan_waiting = 1;
 		spin_unlock_irqrestore(&h->scan_lock, flags);
 		wait_event(h->scan_wait_queue, h->scan_finished);
 		/* Note: We don't need to worry about a race between this
@@ -5587,6 +5599,7 @@ static void hpsa_scan_start(struct Scsi_Host *sh)
 		 */
 	}
 	h->scan_finished = 0; /* mark scan as in progress */
+	h->scan_waiting = 0;
 	spin_unlock_irqrestore(&h->scan_lock, flags);
 
 	if (unlikely(lockup_detected(h)))
@@ -8789,6 +8802,7 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	init_waitqueue_head(&h->event_sync_wait_queue);
 	mutex_init(&h->reset_mutex);
 	h->scan_finished = 1; /* no scan currently in progress */
+	h->scan_waiting = 0;
 
 	pci_set_drvdata(pdev, h);
 	h->ndevices = 0;
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index bf6cdc1..6f04f2a 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -201,6 +201,7 @@ struct ctlr_info {
 	dma_addr_t		errinfo_pool_dhandle;
 	unsigned long  		*cmd_pool_bits;
 	int			scan_finished;
+	u8			scan_waiting : 1;
 	spinlock_t		scan_lock;
 	wait_queue_head_t	scan_wait_queue;
 

Re: [PATCH V3 2/3] hpsa: limit outstanding rescans

[James Bottomley]

On Fri, 2017-03-10 at 14:35 -0600, Don Brace wrote:
> avoid rescan storms. No need to queue another
> if one is pending.
> 
> Reviewed-by: Scott Benesh <scott.benesh@microsemi.com>
> Reviewed-by: Scott Teel <scott.teel@microsemi.com>
> Reviewed-by: Tomas Henzl <thenzl@redhat.com> review

Guys, what is this additional "review" for?  I just noticed because my
reply scripts barfed on it.  Please don't add extra junk after an email
address in a tag line.

Thanks,

James

RE: [PATCH V3 2/3] hpsa: limit outstanding rescans

[Don Brace]

> -----Original Message-----
> From: James Bottomley [mailto:jejb@linux.vnet.ibm.com]
> Sent: Wednesday, March 15, 2017 12:24 PM
> To: Don Brace <don.brace@microsemi.com>; joseph.szczypek@hpe.com;
> Gerry Morong <gerry.morong@microsemi.com>; John Hall
> <John.Hall@microsemi.com>; Kevin Barnett
> <kevin.barnett@microsemi.com>; Mahesh Rajashekhara
> <mahesh.rajashekhara@microsemi.com>; Bader Ali - Saleh
> <bader.alisaleh@microsemi.com>; hch@infradead.org; Scott Teel
> <scott.teel@microsemi.com>; Viswas G <viswas.g@microsemi.com>; Justin
> Lindley <justin.lindley@microsemi.com>; Scott Benesh
> <scott.benesh@microsemi.com>; POSWALD@suse.com
> Cc: linux-scsi@vger.kernel.org
> Subject: Re: [PATCH V3 2/3] hpsa: limit outstanding rescans
> 
> EXTERNAL EMAIL
> 
> 
> On Fri, 2017-03-10 at 14:35 -0600, Don Brace wrote:
> > avoid rescan storms. No need to queue another
> > if one is pending.
> >
> > Reviewed-by: Scott Benesh <scott.benesh@microsemi.com>
> > Reviewed-by: Scott Teel <scott.teel@microsemi.com>
> > Reviewed-by: Tomas Henzl <thenzl@redhat.com> review
> 
> Guys, what is this additional "review" for?  I just noticed because my
> reply scripts barfed on it.  Please don't add extra junk after an email
> address in a tag line.
> 
> Thanks,
> 
> James

My bad. I'll send up a V4.
Sorry about that.

Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation

Re: [PATCH V3 2/3] hpsa: limit outstanding rescans

[Martin K. Petersen]

Don Brace <don.brace@microsemi.com> writes:

> My bad. I'll send up a V4.  Sorry about that.

I already fixed it up and James pulled the new tree. So we're all set.

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH V3 3/3] hpsa: do not timeout reset operations

[Don Brace]

resets can take longer than DEFAULT_TIMEOUT

Reviewed-by: Scott Benesh <scott.benesh@microsemi.com>
Reviewed-by: Scott Teel <scott.teel@microsemi.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com> review
Signed-off-by: Don Brace <don.brace@microsemi.com>
---
 drivers/scsi/hpsa.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0a8ac68..0d0be77 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2956,7 +2956,7 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr,
 	/* fill_cmd can't fail here, no data buffer to map. */
 	(void) fill_cmd(c, reset_type, h, NULL, 0, 0,
 			scsi3addr, TYPE_MSG);
-	rc = hpsa_scsi_do_simple_cmd(h, c, reply_queue, DEFAULT_TIMEOUT);
+	rc = hpsa_scsi_do_simple_cmd(h, c, reply_queue, NO_TIMEOUT);
 	if (rc) {
 		dev_warn(&h->pdev->dev, "Failed to send reset command\n");
 		goto out;

Re: [PATCH V3 0/3] hpsa updates

[Martin K. Petersen]

>>>>> "Don" == Don Brace <don.brace@microsemi.com> writes:

Don> These patches are based on Linus's tree The changes are:
Don>  - add in a new offline volume status
Don>  - limit the number of outstanding rescan operation
Don>  - do not timeout reset operations

Applied to 4.11/scsi-fixes.

Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering