[PATCH v2 0/4] mpt3sas driver Enhancements and

[PATCH v2 0/4] mpt3sas driver Enhancements and

[Chaitra P B]

Here is the change list:
Posting 4 patches for mpt3sas driver enhancement and defect fixes.
  * Handle cable event for notifying degraded speed.
  * Performance improvement for Crusader.
  * Fix Firmware fault state 0x2100 during heavy 4K RR
    FIO stress test.
  * Updated driver version to 15.100.00.00

Chaitra P B (4):
  mpt3sas: Added print to notify cable running at a degraded
    speed.
  mpt3sas: Fix for Crusader to achieve product targets with SAS
    devices.
  mpt3sas: Fix Firmware fault state 0x2100 during heavy 4K RR        
    FIO stress test.
  mpt3sas: Updating driver version to v15.100.00.00

 drivers/scsi/mpt3sas/mpi/mpi2_ioc.h  |    2 +
 drivers/scsi/mpt3sas/mpt3sas_base.c  |   20 +++++++++++++
 drivers/scsi/mpt3sas/mpt3sas_base.h  |    7 ++--
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |    4 ++-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   51 ++++++++++++++++++++++++++++------
 5 files changed, 71 insertions(+), 13 deletions(-)

[PATCH v2 1/4] mpt3sas: Added print to notify cable running at a degraded speed.

[Chaitra P B]

Driver processes the event MPI26_EVENT_ACTIVE_CABLE_DEGRADED
when a cable is present and is running at a degraded speed
(below the SAS3 12 Gb/s rate). Prints added
to inform the user that the cable is not running at
optimal speed.

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpi/mpi2_ioc.h  |    2 ++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   20 ++++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
index 8bae305..af4be40 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_ioc.h
@@ -624,6 +624,8 @@ typedef struct _MPI26_EVENT_DATA_ACTIVE_CABLE_EXCEPT {
 
 /* defines for ReasonCode field */
 #define MPI26_EVENT_ACTIVE_CABLE_INSUFFICIENT_POWER     (0x00)
+#define MPI26_EVENT_ACTIVE_CABLE_PRESENT                (0x01)
+#define MPI26_EVENT_ACTIVE_CABLE_DEGRADED               (0x02)
 
 /*Hard Reset Received Event data */
 
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 75f3fce..0f71f4a 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -8028,15 +8028,23 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
 	case MPI2_EVENT_ACTIVE_CABLE_EXCEPTION:
 		ActiveCableEventData =
 		    (Mpi26EventDataActiveCableExcept_t *) mpi_reply->EventData;
-		if (ActiveCableEventData->ReasonCode ==
-				MPI26_EVENT_ACTIVE_CABLE_INSUFFICIENT_POWER) {
-			pr_info(MPT3SAS_FMT "Currently an active cable with ReceptacleID %d",
+		switch (ActiveCableEventData->ReasonCode) {
+		case MPI26_EVENT_ACTIVE_CABLE_INSUFFICIENT_POWER:
+			pr_info(MPT3SAS_FMT "Currently an active cable with ReceptacleID %d\n",
 			    ioc->name, ActiveCableEventData->ReceptacleID);
-			pr_info("cannot be powered and devices connected to this active cable");
-			pr_info("will not be seen. This active cable");
-			pr_info("requires %d mW of power",
+			pr_info(" cannot be powered and devices connected to\n");
+			pr_info(" this active cable will not be seen. This\n");
+			pr_info(" cable requires %d mW of power\n",
 			    ActiveCableEventData->ActiveCablePowerRequirement);
+			break;
+
+		case MPI26_EVENT_ACTIVE_CABLE_DEGRADED:
+			pr_info(MPT3SAS_FMT "Currently a cable with ReceptacleID %d",
+			    ioc->name, ActiveCableEventData->ReceptacleID);
+			pr_info(" is not running at optimal speed(12 Gb/s)\n");
+			break;
 		}
+
 		break;
 
 	default: /* ignore the rest */
-- 
1.7.1

Re: [PATCH v2 1/4] mpt3sas: Added print to notify cable running at a degraded speed.

[Johannes Thumshirn]

On Fri, Jan 20, 2017 at 08:12:10PM +0530, Chaitra P B wrote:
> Driver processes the event MPI26_EVENT_ACTIVE_CABLE_DEGRADED
> when a cable is present and is running at a degraded speed
> (below the SAS3 12 Gb/s rate). Prints added
> to inform the user that the cable is not running at
> optimal speed.
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> ---


Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 1/4] mpt3sas: Added print to notify cable running at a degraded speed.

[Joe Perches]

On Fri, 2017-01-20 at 20:12 +0530, Chaitra P B wrote:
> Driver processes the event MPI26_EVENT_ACTIVE_CABLE_DEGRADED
> when a cable is present and is running at a degraded speed
> (below the SAS3 12 Gb/s rate). Prints added
> to inform the user that the cable is not running at
> optimal speed.
[]
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
[]
> @@ -8028,15 +8028,23 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
>  	case MPI2_EVENT_ACTIVE_CABLE_EXCEPTION:
>  		ActiveCableEventData =
>  		    (Mpi26EventDataActiveCableExcept_t *) mpi_reply->EventData;
> -		if (ActiveCableEventData->ReasonCode ==
> -				MPI26_EVENT_ACTIVE_CABLE_INSUFFICIENT_POWER) {
> -			pr_info(MPT3SAS_FMT "Currently an active cable with ReceptacleID %d",
> +		switch (ActiveCableEventData->ReasonCode) {
> +		case MPI26_EVENT_ACTIVE_CABLE_INSUFFICIENT_POWER:
> +			pr_info(MPT3SAS_FMT "Currently an active cable with ReceptacleID %d\n",
>  			    ioc->name, ActiveCableEventData->ReceptacleID);
> -			pr_info("cannot be powered and devices connected to this active cable");
> -			pr_info("will not be seen. This active cable");
> -			pr_info("requires %d mW of power",
> +			pr_info(" cannot be powered and devices connected to\n");
> +			pr_info(" this active cable will not be seen. This\n");
> +			pr_info(" cable requires %d mW of power\n",

Can you please use more intelligible logging
where sentences are not broken across multiple
lines of output?  Something like:

			pr_notice(MPT3SAS_FMT "Receptacle ID %d: This active cable requires %d mW of power\n",
				  ioc->name, ActiveCableEventData->ReceptacleID,
				  ActiveCableEventData->ActiveCablePowerRequirement);
			pr_notice(MPT3SAS_FMT "Receptacle ID %d: Devices connected to this active cable will not be seen\n",
				  ioc->name, ActiveCableEventData->ReceptacleID);

I believe MPT3SAS_FMT is unnecessary obfuscation and
it should just be replaced by "%s: " everywhere.

Re: [PATCH v2 1/4] mpt3sas: Added print to notify cable running at a degraded speed.

[Joe Perches]

On Fri, 2017-01-20 at 09:55 -0800, Joe Perches wrote:
> I believe MPT3SAS_FMT is unnecessary obfuscation and
> it should just be replaced by "%s: " everywhere.

Here's a trivial command that could be used one day:

$ git grep --name-only MPT3SAS_FMT -- "*.c" | \
  xargs perl -p -i -e 'local $/; while (<>) { s/\bMPT3SAS_FMT\s*"/"%s: /g; print; }'

[PATCH v2 2/4] mpt3sas: Fix for Crusader to achieve product targets with SAS devices.

[Chaitra P B]

Small glitch/degraded performance in Crusader is improved with SAS
drives by removing unnecessary spinlocks while clearing scsi command
in drivers internal lookup table.

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  |    1 +
 drivers/scsi/mpt3sas/mpt3sas_base.h  |    1 +
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |    4 +++-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   31 ++++++++++++++++++++++++++++---
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f00ef88..722fab9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -5522,6 +5522,7 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
 		goto out_free_resources;
 
 	ioc->non_operational_loop = 0;
+	ioc->got_task_abort_from_ioctl = 0;
 	return 0;
 
  out_free_resources:
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index dcb33f4..83cfa16 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1000,6 +1000,7 @@ struct MPT3SAS_ADAPTER {
 	u8		broadcast_aen_busy;
 	u16		broadcast_aen_pending;
 	u8		shost_recovery;
+	u8		got_task_abort_from_ioctl;
 
 	struct mutex	reset_in_progress_mutex;
 	spinlock_t	ioc_reset_in_progress_lock;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index 95f0f24..02fe1c4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -826,16 +826,18 @@ _ctl_do_mpt_command(struct MPT3SAS_ADAPTER *ioc, struct mpt3_ioctl_command karg,
 			"TASK_MGMT: handle(0x%04x), task_type(0x%02x)\n",
 			ioc->name,
 		    le16_to_cpu(tm_request->DevHandle), tm_request->TaskType));
-
+		ioc->got_task_abort_from_ioctl = 1;
 		if (tm_request->TaskType ==
 		    MPI2_SCSITASKMGMT_TASKTYPE_ABORT_TASK ||
 		    tm_request->TaskType ==
 		    MPI2_SCSITASKMGMT_TASKTYPE_QUERY_TASK) {
 			if (_ctl_set_task_mid(ioc, &karg, tm_request)) {
 				mpt3sas_base_free_smid(ioc, smid);
+				ioc->got_task_abort_from_ioctl = 0;
 				goto out;
 			}
 		}
+		ioc->got_task_abort_from_ioctl = 0;
 
 		if (test_bit(device_handle, ioc->device_remove_in_progress)) {
 			dtmprintk(ioc, pr_info(MPT3SAS_FMT
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 0f71f4a..217e0fe 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1074,6 +1074,26 @@ _scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 }
 
 /**
+ * _scsih_scsi_lookup_get_clear_without_lock - returns scmd entry without
+ *						holding any lock.
+ * @ioc: per adapter object
+ * @smid: system request message index
+ *
+ * Returns the smid stored scmd pointer.
+ * Then will dereference the stored scmd pointer.
+ */
+static inline struct scsi_cmnd *
+_scsih_scsi_lookup_get_clear_without_lock(struct MPT3SAS_ADAPTER *ioc,
+		u16 smid)
+{
+	struct scsi_cmnd *scmd = NULL;
+
+	swap(scmd, ioc->scsi_lookup[smid - 1].scmd);
+
+	return scmd;
+}
+
+/**
  * _scsih_scsi_lookup_get_clear - returns scmd entry
  * @ioc: per adapter object
  * @smid: system request message index
@@ -1088,8 +1108,7 @@ _scsih_scsi_lookup_get_clear(struct MPT3SAS_ADAPTER *ioc, u16 smid)
 	struct scsi_cmnd *scmd;
 
 	spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
-	scmd = ioc->scsi_lookup[smid - 1].scmd;
-	ioc->scsi_lookup[smid - 1].scmd = NULL;
+	scmd = _scsih_scsi_lookup_get_clear_without_lock(ioc, smid);
 	spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
 
 	return scmd;
@@ -4659,7 +4678,13 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
 	unsigned long flags;
 
 	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
-	scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
+
+	if (ioc->broadcast_aen_busy || ioc->pci_error_recovery ||
+			ioc->got_task_abort_from_ioctl)
+		scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
+	else
+		scmd = _scsih_scsi_lookup_get_clear_without_lock(ioc, smid);
+
 	if (scmd == NULL)
 		return 1;
 
-- 
1.7.1

Re: [PATCH v2 2/4] mpt3sas: Fix for Crusader to achieve product targets with SAS devices.

[Johannes Thumshirn]

On Fri, Jan 20, 2017 at 08:12:11PM +0530, Chaitra P B wrote:
> Small glitch/degraded performance in Crusader is improved with SAS
> drives by removing unnecessary spinlocks while clearing scsi command
> in drivers internal lookup table.
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> ---

Whats wrong with creating a __scsih_scsi_lookup_get_clear() and calling
it with the scsi_lookup_lock held from _scsih_scsi_lookup_get_clear()
instead of duplicating the code?

It's quite common in the linux kernel to have two functions func and __func,
where func is 

func()
{
	spin_lock();
	__func();
	spin_unlock();
}

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 2/4] mpt3sas: Fix for Crusader to achieve product targets with SAS devices.

[Sreekanth Reddy]

On Fri, Jan 20, 2017 at 8:30 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Fri, Jan 20, 2017 at 08:12:11PM +0530, Chaitra P B wrote:
>> Small glitch/degraded performance in Crusader is improved with SAS
>> drives by removing unnecessary spinlocks while clearing scsi command
>> in drivers internal lookup table.
>>
>> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
>> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
>> ---
>
> Whats wrong with creating a __scsih_scsi_lookup_get_clear() and calling
> it with the scsi_lookup_lock held from _scsih_scsi_lookup_get_clear()
> instead of duplicating the code?
>
> It's quite common in the linux kernel to have two functions func and __func,
> where func is
>
> func()
> {
>         spin_lock();
>         __func();
>         spin_unlock();
> }
>

Sure, We will resend this path with this change.

Thanks,
Sreekanth

> --
> Johannes Thumshirn                                          Storage
> jthumshirn@suse.de                                +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 3/4] mpt3sas: Fix Firmware fault state 0x2100 during heavy 4K RR FIO stress test.

[Chaitra P B]

Due existence of loop in the IO path our HBA will receive heavy IOs and
also as driver is not updating the Reply Post Host Index frequently, So
there will be a high chance that our Firmware unable to find any free entry
in the Reply Post Descriptor Queue (i.e. Queue overflow occurs) and can
observe 0x2100 firmware fault.
So to fix this, we have defined a thresh hold value. After continuously
processing this thresh hold number of reply descriptors driver will update
the Reply Descriptor Host Index so that this thresh hold number of reply
descriptors entries will be freed and these entries will be available for
firmware and we won't observe this Firmware fault. We have defined this
threshold value as 1/3rd of the hba queue depth.

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 722fab9..a3fe1fb 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1040,6 +1040,25 @@ _base_interrupt(int irq, void *bus_id)
 		    reply_q->reply_post_free[reply_q->reply_post_host_index].
 		    Default.ReplyFlags & MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
 		completed_cmds++;
+		/* Update the reply post host index after continuously
+		 * processing the threshold number of Reply Descriptors.
+		 * So that FW can find enough entries to post the Reply
+		 * Descriptors in the reply descriptor post queue.
+		 */
+		if (completed_cmds > ioc->hba_queue_depth/3) {
+			if (ioc->combined_reply_queue) {
+				writel(reply_q->reply_post_host_index |
+						((msix_index  & 7) <<
+						 MPI2_RPHI_MSIX_INDEX_SHIFT),
+				    ioc->replyPostRegisterIndex[msix_index/8]);
+			} else {
+				writel(reply_q->reply_post_host_index |
+						(msix_index <<
+						 MPI2_RPHI_MSIX_INDEX_SHIFT),
+						&ioc->chip->ReplyPostHostIndex);
+			}
+			completed_cmds = 1;
+		}
 		if (request_desript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
 			goto out;
 		if (!reply_q->reply_post_host_index)
-- 
1.7.1

Re: [PATCH v2 3/4] mpt3sas: Fix Firmware fault state 0x2100 during heavy 4K RR FIO stress test.

[Johannes Thumshirn]

On Fri, Jan 20, 2017 at 08:12:12PM +0530, Chaitra P B wrote:
> Due existence of loop in the IO path our HBA will receive heavy IOs and
> also as driver is not updating the Reply Post Host Index frequently, So
> there will be a high chance that our Firmware unable to find any free entry
> in the Reply Post Descriptor Queue (i.e. Queue overflow occurs) and can
> observe 0x2100 firmware fault.
> So to fix this, we have defined a thresh hold value. After continuously
> processing this thresh hold number of reply descriptors driver will update
> the Reply Descriptor Host Index so that this thresh hold number of reply
> descriptors entries will be freed and these entries will be available for
> firmware and we won't observe this Firmware fault. We have defined this
> threshold value as 1/3rd of the hba queue depth.
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 722fab9..a3fe1fb 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -1040,6 +1040,25 @@ _base_interrupt(int irq, void *bus_id)
>  		    reply_q->reply_post_free[reply_q->reply_post_host_index].
>  		    Default.ReplyFlags & MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
>  		completed_cmds++;
> +		/* Update the reply post host index after continuously
> +		 * processing the threshold number of Reply Descriptors.
> +		 * So that FW can find enough entries to post the Reply
> +		 * Descriptors in the reply descriptor post queue.
> +		 */
> +		if (completed_cmds > ioc->hba_queue_depth/3) {
> +			if (ioc->combined_reply_queue) {
> +				writel(reply_q->reply_post_host_index |
> +						((msix_index  & 7) <<
> +						 MPI2_RPHI_MSIX_INDEX_SHIFT),
> +				    ioc->replyPostRegisterIndex[msix_index/8]);
> +			} else {
> +				writel(reply_q->reply_post_host_index |
> +						(msix_index <<
> +						 MPI2_RPHI_MSIX_INDEX_SHIFT),
> +						&ioc->chip->ReplyPostHostIndex);
> +			}
> +			completed_cmds = 1;
> +		}
>  		if (request_desript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
>  			goto out;
>  		if (!reply_q->reply_post_host_index)

Do I understand it correctly that you fill the HBA's internal queue up to a
3rd and then kick it to start processing?

Thanks,
	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 3/4] mpt3sas: Fix Firmware fault state 0x2100 during heavy 4K RR FIO stress test.

[Sreekanth Reddy]

On Fri, Jan 20, 2017 at 8:40 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Fri, Jan 20, 2017 at 08:12:12PM +0530, Chaitra P B wrote:
>> Due existence of loop in the IO path our HBA will receive heavy IOs and
>> also as driver is not updating the Reply Post Host Index frequently, So
>> there will be a high chance that our Firmware unable to find any free entry
>> in the Reply Post Descriptor Queue (i.e. Queue overflow occurs) and can
>> observe 0x2100 firmware fault.
>> So to fix this, we have defined a thresh hold value. After continuously
>> processing this thresh hold number of reply descriptors driver will update
>> the Reply Descriptor Host Index so that this thresh hold number of reply
>> descriptors entries will be freed and these entries will be available for
>> firmware and we won't observe this Firmware fault. We have defined this
>> threshold value as 1/3rd of the hba queue depth.
>>
>> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
>> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
>> ---
>>  drivers/scsi/mpt3sas/mpt3sas_base.c |   19 +++++++++++++++++++
>>  1 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> index 722fab9..a3fe1fb 100644
>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
>> @@ -1040,6 +1040,25 @@ _base_interrupt(int irq, void *bus_id)
>>                   reply_q->reply_post_free[reply_q->reply_post_host_index].
>>                   Default.ReplyFlags & MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
>>               completed_cmds++;
>> +             /* Update the reply post host index after continuously
>> +              * processing the threshold number of Reply Descriptors.
>> +              * So that FW can find enough entries to post the Reply
>> +              * Descriptors in the reply descriptor post queue.
>> +              */
>> +             if (completed_cmds > ioc->hba_queue_depth/3) {
>> +                     if (ioc->combined_reply_queue) {
>> +                             writel(reply_q->reply_post_host_index |
>> +                                             ((msix_index  & 7) <<
>> +                                              MPI2_RPHI_MSIX_INDEX_SHIFT),
>> +                                 ioc->replyPostRegisterIndex[msix_index/8]);
>> +                     } else {
>> +                             writel(reply_q->reply_post_host_index |
>> +                                             (msix_index <<
>> +                                              MPI2_RPHI_MSIX_INDEX_SHIFT),
>> +                                             &ioc->chip->ReplyPostHostIndex);
>> +                     }
>> +                     completed_cmds = 1;
>> +             }
>>               if (request_desript_type == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
>>                       goto out;
>>               if (!reply_q->reply_post_host_index)
>
> Do I understand it correctly that you fill the HBA's internal queue up to a
> 3rd and then kick it to start processing?

No, driver will continuously process the reply descriptors from Reply
Descriptor Post Queue (RDPQ) but will update it's Host Index (tail
index) with the firmware after continuously processing 1/3rd of the
HBA queue depth number of descriptors instead of updating it's host
index only at after it see unused descriptor entry. So that firmware
can always get enough free descriptors entries to post reply
descriptors and won't see any 0x2100 fault which will occur if
firmware doesn't find any free descriptor entry in the  RDPQ queue.

Thanks,
Sreekanth

>
> Thanks,
>         Johannes
> --
> Johannes Thumshirn                                          Storage
> jthumshirn@suse.de                                +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 4/4] mpt3sas: Updating driver version to v15.100.00.00

[Chaitra P B]

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 83cfa16..4ab634f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -73,9 +73,9 @@
 #define MPT3SAS_DRIVER_NAME		"mpt3sas"
 #define MPT3SAS_AUTHOR "Avago Technologies <MPT-FusionLinux.pdl@avagotech.com>"
 #define MPT3SAS_DESCRIPTION	"LSI MPT Fusion SAS 3.0 Device Driver"
-#define MPT3SAS_DRIVER_VERSION		"14.101.00.00"
-#define MPT3SAS_MAJOR_VERSION		14
-#define MPT3SAS_MINOR_VERSION		101
+#define MPT3SAS_DRIVER_VERSION		"15.100.00.00"
+#define MPT3SAS_MAJOR_VERSION		15
+#define MPT3SAS_MINOR_VERSION		100
 #define MPT3SAS_BUILD_VERSION		0
 #define MPT3SAS_RELEASE_VERSION	00
 
-- 
1.7.1

Re: [PATCH v2 4/4] mpt3sas: Updating driver version to v15.100.00.00

[Johannes Thumshirn]

On Fri, Jan 20, 2017 at 08:12:13PM +0530, Chaitra P B wrote:
> 
> Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
> Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850