Skip to contentSkip to conversation

mpt2sas: Infinite loop can occur if MPI2_IOCSTATUS_CONFIG_INVALID_PAGE is not returned

PATCH In review Sreekanth Reddy 2 participants 4 messages Jul 25, 2013 – Aug 26, 2013 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 5/8 mpt2sas: Infinite loop can occur if MPI2_IOCSTATUS_CONFIG_INVALID_PAGE is not returned

Infinite loop can occur if IOCStatus is not equal to MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value in the while loops in functions _scsih_search_responding_sas_devices, _scsih_search_responding_raid_devices and _scsih_search_responding_expanders

So, Instead of checking for MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value, in this patch code is modified to check for IOCStatus not equals to MPI2_IOCSTATUS_SUCCESS to break the while loop.

---
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)
drivers/scsi/mpt2sas/mpt2sas_scsih.c +4 -12
index 6c86355..b7f8229 100644
@@ -6753,7 +6753,7 @@ _scsih_search_responding_sas_devices(struct MPT2SAS_ADAPTER *ioc)
 	    handle))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
+		if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
 			break;
 		handle = le16_to_cpu(sas_device_pg0.DevHandle);
 		device_info = le32_to_cpu(sas_device_pg0.DeviceInfo);
@@ -6862,7 +6862,7 @@ _scsih_search_responding_raid_devices(struct MPT2SAS_ADAPTER *ioc)
 	    &volume_pg1, MPI2_RAID_VOLUME_PGAD_FORM_GET_NEXT_HANDLE, handle))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
+		if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
 			break;
 		handle = le16_to_cpu(volume_pg1.DevHandle);
 
@@ -6887,7 +6887,7 @@ _scsih_search_responding_raid_devices(struct MPT2SAS_ADAPTER *ioc)
 		    phys_disk_num))) {
 			ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 			    MPI2_IOCSTATUS_MASK;
-			if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
+			if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
 				break;
 			phys_disk_num = pd_pg0.PhysDiskNum;
 			handle = le16_to_cpu(pd_pg0.DevHandle);
@@ -6967,7 +6967,7 @@ _scsih_search_responding_expanders(struct MPT2SAS_ADAPTER *ioc)
 
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
+		if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
 			break;
 
 		handle = le16_to_cpu(expander_pg0.DevHandle);
@@ -7109,8 +7109,6 @@ _scsih_scan_for_devices_after_reset(struct MPT2SAS_ADAPTER *ioc)
 	    MPI2_SAS_EXPAND_PGAD_FORM_GET_NEXT_HNDL, handle))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
-			break;
 		if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
 			printk(MPT2SAS_INFO_FMT "\tbreak from expander scan: "
 				"ioc_status(0x%04x), loginfo(0x%08x)\n",
@@ -7153,8 +7151,6 @@ _scsih_scan_for_devices_after_reset(struct MPT2SAS_ADAPTER *ioc)
 	    phys_disk_num))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
-			break;
 		if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
 			printk(MPT2SAS_INFO_FMT "\tbreak from phys disk scan:"
 				"ioc_status(0x%04x), loginfo(0x%08x)\n",
@@ -7219,8 +7215,6 @@ _scsih_scan_for_devices_after_reset(struct MPT2SAS_ADAPTER *ioc)
 	    &volume_pg1, MPI2_RAID_VOLUME_PGAD_FORM_GET_NEXT_HANDLE, handle))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
-			break;
 		if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
 			printk(MPT2SAS_INFO_FMT "\tbreak from volume scan: "
 				"ioc_status(0x%04x), loginfo(0x%08x)\n",
@@ -7278,8 +7272,6 @@ _scsih_scan_for_devices_after_reset(struct MPT2SAS_ADAPTER *ioc)
 	    handle))) {
 		ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
 		    MPI2_IOCSTATUS_MASK;
-		if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
-			break;
 		if (ioc_status != MPI2_IOCSTATUS_SUCCESS) {
 			printk(MPT2SAS_INFO_FMT "\tbreak from end device scan:"
 				" ioc_status(0x%04x), loginfo(0x%08x)\n",
Signature · 1 line
1.7.3
maintainer
#1 lore raw
Quoted 6 lines · Sreekanth Reddy

Infinite loop can occur if IOCStatus is not equal to MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value in the while loops in functions _scsih_search_responding_sas_devices, _scsih_search_responding_raid_devices and _scsih_search_responding_expanders

Quoted 6 lines

So, Instead of checking for MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value, in this patch code is modified to check for IOCStatus not equals to MPI2_IOCSTATUS_SUCCESS to break the while loop.

Patches like this aren't acceptable any more: Greg will find this and reject it from Stable.

If there's a bug in the driver, it needs to be in a separate patch set for the fixes branch of SCSI to go upstream now rather than waiting for the merge window. Only patches for the fixes branch can be tagged for stable.

James

maintainer
#2 lore raw
Quoted 14 lines · James Bottomley
Quoted 6 lines · Sreekanth Reddy

Infinite loop can occur if IOCStatus is not equal to MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value in the while loops in functions _scsih_search_responding_sas_devices, _scsih_search_responding_raid_devices and _scsih_search_responding_expanders

Quoted 6 lines

So, Instead of checking for MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value, in this patch code is modified to check for IOCStatus not equals to MPI2_IOCSTATUS_SUCCESS to break the while loop.

Quoted 7 lines

Patches like this aren't acceptable any more: Greg will find this and reject it from Stable.

If there's a bug in the driver, it needs to be in a separate patch set for the fixes branch of SCSI to go upstream now rather than waiting for the merge window. Only patches for the fixes branch can be tagged for stable.

Ping on this, please. What do you want me to do? I can strip the cc:
stable from the series and send it to the misc branch or you can resubmit as two series: one for fixes with the stable tags and the rest for misc.

James

#3 lore raw

Hi James,

Sorry for the delay.

we support the first method i.e. "strip the cc: stable from this patch and send it to the misc branch".

Thanks,
Sreekanth.

Quoted 38 lines

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Monday, August 26, 2013 2:11 PM
To: Reddy, Sreekanth
Cc: linux-scsi; Prakash, Sathya; Nandigama, Nagalakshmi
Subject: Re: [PATCH 5/8][SCSI] mpt2sas: Infinite loop can occur if MPI2_IOCSTATUS_CONFIG_INVALID_PAGE is not returned

Quoted 14 lines · James Bottomley
Quoted 6 lines · Sreekanth Reddy

Infinite loop can occur if IOCStatus is not equal to MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value in the while loops in functions _scsih_search_responding_sas_devices, _scsih_search_responding_raid_devices and _scsih_search_responding_expanders

Quoted 6 lines

So, Instead of checking for MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value, in this patch code is modified to check for IOCStatus not equals to MPI2_IOCSTATUS_SUCCESS to break the while loop.

Quoted 7 lines

Patches like this aren't acceptable any more: Greg will find this and reject it from Stable.

If there's a bug in the driver, it needs to be in a separate patch set for the fixes branch of SCSI to go upstream now rather than waiting for the merge window. Only patches for the fixes branch can be tagged for stable.

Ping on this, please. What do you want me to do? I can strip the cc:
stable from the series and send it to the misc branch or you can resubmit as two series: one for fixes with the stable tags and the rest for misc.

James