Skip to contentSkip to conversation

qla4xxx: bug fixes

PATCH In review David C Somayajulu 2 participants 3 messages Jan 22, 2007 – Jan 29, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 qla4xxx: bug fixes

The included patch fixes the following issues:
1. qla3xxx/qla4xxx co-existence issue which can result in a lockup when qla3xxx driver is unloaded, or when ifdown; ifup is performed on one of the interfaces correponding to qla3xxx. This is because qla4xxx HBA supports one ethernet and iscsi interfaces per port. Both iscsi and ethernet interfaces share the same state machine. The problem has to do with synchronizing access to the state machine in the event of a reset
2. mutex_lock() is sometimes not followed by mutex_unlock() prior to invoking a msleep() in qla4xxx_mailbox_command()

Thanks
David Somayajulu

---
 drivers/scsi/qla4xxx/ql4_def.h     |    1 -
 drivers/scsi/qla4xxx/ql4_glbl.h    |    1 +
 drivers/scsi/qla4xxx/ql4_init.c    |   18 +++++-----
 drivers/scsi/qla4xxx/ql4_isr.c     |    4 +-
 drivers/scsi/qla4xxx/ql4_mbx.c     |   35 ++++++++++++--------
 drivers/scsi/qla4xxx/ql4_os.c      |   64 ++++++++++++++++++++++--------------
 drivers/scsi/qla4xxx/ql4_version.h |    2 +
 7 files changed, 73 insertions(+), 52 deletions(-)
drivers/scsi/qla4xxx/ql4_def.h +0 -1
index 4249e52..6f4cf2d 100644
@@ -418,7 +418,6 @@ #define MEM_ALIGN_VALUE \
 	 * concurrently.
 	 */
 	struct mutex  mbox_sem;
-	wait_queue_head_t mailbox_wait_queue;
 
 	/* temporary mailbox status registers */
 	volatile uint8_t mbox_status_count;
drivers/scsi/qla4xxx/ql4_glbl.h +1 -0
index 2122967..e021eb5 100644
@@ -76,4 +76,5 @@ int qla4xxx_process_ddb_changed(struct s
 extern int ql4xextended_error_logging;
 extern int ql4xdiscoverywait;
 extern int ql4xdontresethba;
+extern int ql4_mod_unload;
 #endif /* _QLA4x_GBL_H */
drivers/scsi/qla4xxx/ql4_init.c +9 -9
index cc210f2..b907b06 100644
@@ -958,25 +958,25 @@ static int qla4xxx_start_firmware_from_f
 	return status;
 }
 
-int ql4xxx_lock_drvr_wait(struct scsi_qla_host *a)
+int ql4xxx_lock_drvr_wait(struct scsi_qla_host *ha)
 {
-#define QL4_LOCK_DRVR_WAIT	300
-#define QL4_LOCK_DRVR_SLEEP	100
+#define QL4_LOCK_DRVR_WAIT	30
+#define QL4_LOCK_DRVR_SLEEP	1
 
 	int drvr_wait = QL4_LOCK_DRVR_WAIT;
 	while (drvr_wait) {
-		if (ql4xxx_lock_drvr(a) == 0) {
-			msleep(QL4_LOCK_DRVR_SLEEP);
+		if (ql4xxx_lock_drvr(ha) == 0) {
+			ssleep(QL4_LOCK_DRVR_SLEEP);
 			if (drvr_wait) {
 				DEBUG2(printk("scsi%ld: %s: Waiting for "
-					      "Global Init Semaphore...n",
-					      a->host_no,
-					      __func__));
+					      "Global Init Semaphore(%d)...n",
+					      ha->host_no,
+					      __func__, drvr_wait));
 			}
 			drvr_wait -= QL4_LOCK_DRVR_SLEEP;
 		} else {
 			DEBUG2(printk("scsi%ld: %s: Global Init Semaphore "
-				      "acquired.n", a->host_no, __func__));
+				      "acquired.n", ha->host_no, __func__));
 			return QLA_SUCCESS;
 		}
 	}
drivers/scsi/qla4xxx/ql4_isr.c +2 -2
index ef975e0..35b9e36 100644
@@ -433,7 +433,6 @@ static void qla4xxx_isr_decode_mailbox(s
 					readl(&ha->reg->mailbox[i]);
 
 			set_bit(AF_MBOX_COMMAND_DONE, &ha->flags);
-			wake_up(&ha->mailbox_wait_queue);
 		}
 	} else if (mbox_status >> 12 == MBOX_ASYNC_EVENT_STATUS) {
 		/* Immediately process the AENs that don't require much work.
@@ -686,7 +685,8 @@ irqreturn_t qla4xxx_intr_handler(int irq
 			       &ha->reg->ctrl_status);
 			readl(&ha->reg->ctrl_status);
 
-			set_bit(DPC_RESET_HA_INTR, &ha->dpc_flags);
+			if (!ql4_mod_unload)
+				set_bit(DPC_RESET_HA_INTR, &ha->dpc_flags);
 
 			break;
 		} else if (intr_status & INTR_PENDING) {
drivers/scsi/qla4xxx/ql4_mbx.c +21 -14
index b721dc5..d1ef229 100644
@@ -29,19 +29,31 @@ int qla4xxx_mailbox_command(struct scsi_
 	u_long wait_count;
 	uint32_t intr_status;
 	unsigned long flags = 0;
-	DECLARE_WAITQUEUE(wait, current);
-
-	mutex_lock(&ha->mbox_sem);
-
-	/* Mailbox code active */
-	set_bit(AF_MBOX_COMMAND, &ha->flags);
 
 	/* Make sure that pointers are valid */
 	if (!mbx_cmd || !mbx_sts) {
 		DEBUG2(printk("scsi%ld: %s: Invalid mbx_cmd or mbx_sts "
 			      "pointer\n", ha->host_no, __func__));
-		goto mbox_exit;
+		return status;
 	}
+	/* Mailbox code active */
+	wait_count = MBOX_TOV * 100;
+
+	while (wait_count--) {
+		mutex_lock(&ha->mbox_sem);
+		if (!test_bit(AF_MBOX_COMMAND, &ha->flags)) {
+  			set_bit(AF_MBOX_COMMAND, &ha->flags);
+  			mutex_unlock(&ha->mbox_sem);
+  			break;
+  		}
+  		mutex_unlock(&ha->mbox_sem);
+  		if (!wait_count) {
+  			DEBUG2(printk("scsi%ld: %s: mbox_sem failed\n",
+  				ha->host_no, __func__));
+  			return status;
+  		}
+  		msleep(10);
+  	}
 
 	/* To prevent overwriting mailbox registers for a command that has
 	 * not yet been serviced, check to see if a previously issued
@@ -73,8 +85,6 @@ int qla4xxx_mailbox_command(struct scsi_
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
 	/* Wait for completion */
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	add_wait_queue(&ha->mailbox_wait_queue, &wait);
 
 	/*
 	 * If we don't want status, don't wait for the mailbox command to
@@ -83,8 +93,6 @@ int qla4xxx_mailbox_command(struct scsi_
 	 */
 	if (outCount == 0) {
 		status = QLA_SUCCESS;
-		set_current_state(TASK_RUNNING);
-		remove_wait_queue(&ha->mailbox_wait_queue, &wait);
 		goto mbox_exit;
 	}
 	/* Wait for command to complete */
@@ -108,8 +116,6 @@ int qla4xxx_mailbox_command(struct scsi_
 		spin_unlock_irqrestore(&ha->hardware_lock, flags);
 		msleep(10);
 	}
-	set_current_state(TASK_RUNNING);
-	remove_wait_queue(&ha->mailbox_wait_queue, &wait);
 
 	/* Check for mailbox timeout. */
 	if (!test_bit(AF_MBOX_COMMAND_DONE, &ha->flags)) {
@@ -155,9 +161,10 @@ int qla4xxx_mailbox_command(struct scsi_
 	spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
 mbox_exit:
+	mutex_lock(&ha->mbox_sem);
 	clear_bit(AF_MBOX_COMMAND, &ha->flags);
-	clear_bit(AF_MBOX_COMMAND_DONE, &ha->flags);
 	mutex_unlock(&ha->mbox_sem);
+	clear_bit(AF_MBOX_COMMAND_DONE, &ha->flags);
 
 	return status;
 }
drivers/scsi/qla4xxx/ql4_os.c +39 -25

134 lines not on this page · raw message

drivers/scsi/qla4xxx/ql4_version.h +1 -1
index 454e19c..e5183a6 100644
@@ -5,4 +5,4 @@
  * See LICENSE.qla4xxx for copyright and licensing details.
  */
 
-#define QLA4XXX_DRIVER_VERSION	"5.00.07-k"
+#define QLA4XXX_DRIVER_VERSION	"5.00.07-k1"
Signature · 1 line
1.4.2.3
#1 lore raw
Quoted 3 lines · David C Somayajulu

The included patch fixes the following issues:
1. qla3xxx/qla4xxx co-existence issue which can result in a lockup when qla3xxx driver is unloaded, or when ifdown; ifup is performed on one of the interfaces correponding to qla3xxx. This is because qla4xxx HBA supports one ethernet and iscsi interfaces per port. Both iscsi and ethernet interfaces share the same state machine. The problem has to do with synchronizing access to the state machine in the event of a reset
2. mutex_lock() is sometimes not followed by mutex_unlock() prior to invoking a msleep() in qla4xxx_mailbox_command()

Git applymbox picked up this nasty (which I've corrected):

Applying 'qla4xxx: bug fixes'

Space in indent is followed by a tab.
.dotest/patch:122: set_bit(AF_MBOX_COMMAND,
&ha->flags);
Space in indent is followed by a tab.
.dotest/patch:123: mutex_unlock(&ha->mbox_sem);
Space in indent is followed by a tab.
.dotest/patch:124: break;
Space in indent is followed by a tab.
.dotest/patch:125: }
Space in indent is followed by a tab.
.dotest/patch:126: mutex_unlock(&ha->mbox_sem);
warning: squelched 7 whitespace errors
warning: 12 lines add trailing whitespaces.

James

#2 lore raw
Quoted 5 lines

From: James Bottomley [mailto:[email protected]]
Sent: Saturday, January 27, 2007 8:08 AM

Quoted 2 lines · David C Somayajulu

The included patch fixes the following issues:
1. qla3xxx/qla4xxx co-existence issue which can result in a lockup

when qla3xxx driver is unloaded,

Quoted 1 line

or when ifdown; ifup is performed on one of the interfaces

correponding to qla3xxx. This is because

Quoted 1 line

qla4xxx HBA supports one ethernet and iscsi interfaces per port. Both

iscsi and ethernet interfaces

Quoted 1 line

share the same state machine. The problem has to do with synchronizing

access to the state machine in

Quoted 2 lines

the event of a reset

Quoted 1 line

2. mutex_lock() is sometimes not followed by mutex_unlock() prior to

invoking a msleep() in

Quoted 20 lines

qla4xxx_mailbox_command()

Git applymbox picked up this nasty (which I've corrected):

Applying 'qla4xxx: bug fixes'

Space in indent is followed by a tab.
.dotest/patch:122: set_bit(AF_MBOX_COMMAND,
&ha->flags);
Space in indent is followed by a tab.
.dotest/patch:123: mutex_unlock(&ha->mbox_sem);
Space in indent is followed by a tab.
.dotest/patch:124: break;
Space in indent is followed by a tab.
.dotest/patch:125: }
Space in indent is followed by a tab.
.dotest/patch:126: mutex_unlock(&ha->mbox_sem);
warning: squelched 7 whitespace errors
warning: 12 lines add trailing whitespaces.

Quoted 2 lines

James

Sorry about that. Really appreciate your correcting them. Thanks
David S.