Skip to contentSkip to conversation

mpt fusion - fibre channel target discovery prematurely terminates

PATCH In review Michael Reed 2 participants 4 messages May 24, 2006 – Jun 15, 2006

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

PATCH 1/6 mpt fusion - fibre channel target discovery prematurely terminates

Reset processing sets ioc->active to zero. This causes target discovery to be unable to read configuration pages which can result in fewer than the total number of targets being discovered. This patch skips target discovery if ioc->active is zero. The driver reschedules target discovery once ioc->active becomes non-zero.

This situation occurs when the lsiutil program is being used to reset the board.

(Logic change from previously posted 01-mptfc_eagain.patch.)

Reset processing sets ioc->active to zero. This causes target discovery to be unable to read configuration pages which can result in fewer than the total number of targets being discovered. This patch skips target discovery if ioc->active is zero. The driver reschedules target discovery once ioc->active becomes non-zero.

This situation occurs when the lsiutil program is being used to reset the board.

drivers/message/fusion/mptfc.c +9 -0
@@ -639,6 +639,14 @@
 	struct mptfc_rport_info *ri;
 
 	do {
+		/*
+		 * if the ioc isn't active, cannot fetch config pages
+		 * if it becomes active, work count will increment and
+		 * another pass will occur or work will be rescheduled
+		 */
+		if (ioc->active == 0)
+			goto check_work_remaining;
+
 		/* start by tagging all ports as missing */
 		list_for_each_entry(ri, &ioc->fc_rports, list) {
 			if (ri->flags & MPT_RPORT_INFO_FLAGS_REGISTERED) {
@@ -676,6 +684,7 @@
 			}
 		}
 
+ check_work_remaining:
 		/*
 		 * allow multiple passes as target state
 		 * might have changed during scan
#1 lore raw
Quoted 5 lines · Michael Reed

Reset processing sets ioc->active to zero. This causes target discovery to be unable to read configuration pages which can result in fewer than the total number of targets being discovered. This patch skips target discovery if ioc->active is zero. The driver reschedules target discovery once ioc->active becomes non-zero.

This doesn't quite solve all cases, though, does it? mpt_config() returns -EAGAIN for both ioc->active set to zero *and* if we run out of frames.

Quoted 4 lines

This situation occurs when the lsiutil program is being used to reset the board.

(Logic change from previously posted 01-mptfc_eagain.patch.)

I was still thinking of something that made mpt_config() never return -EAGAIN. How does the following work out? It's my first pass at sorting out the frame and active logic in mptbase:

James

drivers/message/fusion/mptbase.c +19 -10
index a300840..585f42f 100644
@@ -121,6 +121,11 @@ static int	mpt_base_index = -1;
 static int	last_drv_idx = -1;
 
 static DECLARE_WAIT_QUEUE_HEAD(mpt_waitq);
+/* This waitq is used for mpt_get_msg_frame failures which may be
+ * caused either by the ioc->active being zero or by the adapter being
+ * out of frames.  Receiving this event doesn't guarantee that a frame
+ * is available, so you must check */
+static DECLARE_WAIT_QUEUE_HEAD(mpt_config_waitq);
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
@@ -722,6 +727,13 @@ mpt_device_driver_deregister(int cb_idx)
 	MptDeviceDriverHandlers[cb_idx] = NULL;
 }
 
+static void
+mpt_ioc_activate(MPT_ADAPTER *ioc)
+{
+	ioc->active = 1;
+	wake_up(&mpt_config_waitq);
+}
+
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
@@ -835,6 +847,7 @@ #endif
 	mf_dma_addr = (ioc->req_frames_low_dma + req_offset) | ioc->RequestNB[req_idx];
 	dsgprintk((MYIOC_s_INFO_FMT "mf_dma_addr=%x req_idx=%d RequestNB=%x\n", ioc->name, mf_dma_addr, req_idx, ioc->RequestNB[req_idx]));
 	CHIPREG_WRITE32(&ioc->chip->RequestFifo, mf_dma_addr);
+	wake_up(&mpt_config_waitq);
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -1582,7 +1595,7 @@ mpt_resume(struct pci_dev *pdev)
 
 	/* enable interrupts */
 	CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-	ioc->active = 1;
+	mpt_ioc_activate(ioc);
 
 	printk(MYIOC_s_INFO_FMT
 		"pci-resume: ioc-state=0x%x,doorbell=0x%x\n",
@@ -1682,7 +1695,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 				dprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 						ioc->alt_ioc->name));
 				CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-				ioc->alt_ioc->active = 1;
+				mpt_ioc_activate(ioc->alt_ioc);
 			}
 
 		} else {
@@ -1798,7 +1811,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 	if (ret == 0) {
 		/* Enable! (reply interrupt) */
 		CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->active = 1;
+		mpt_ioc_activate(ioc);
 	}
 
 	if (reset_alt_ioc_active && ioc->alt_ioc) {
@@ -1806,7 +1819,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 		dinitprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 				ioc->alt_ioc->name));
 		CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->alt_ioc->active = 1;
+		mpt_ioc_activate(ioc->alt_ioc);
 	}
 
 	/*  Enable MPT base driver management of EventNotification
@@ -4069,7 +4082,6 @@ #endif
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -4181,7 +4193,6 @@ GetLanConfigPages(MPT_ADAPTER *ioc)
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -4493,7 +4504,6 @@ mptbase_raid_process_event_data(MPT_ADAP
  *	Returns: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -5156,7 +5166,6 @@ SendEventAck(MPT_ADAPTER *ioc, EventNoti
  *
  *	Returns 0 for success
  *	-EPERM if not allowed due to ISR context
- *	-EAGAIN if no msg frames currently available
  *	-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -5182,10 +5191,10 @@ mpt_config(MPT_ADAPTER *ioc, CONFIGPARMS
 
 	/* Get and Populate a free Frame
 	 */
-	if ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	while ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
 		dcprintk((MYIOC_s_WARN_FMT "mpt_config: no msg frames!\n",
 				ioc->name));
-		return -EAGAIN;
+		wait_event(mpt_config_waitq, pCfg->wait_done);
 	}
 	pReq = (Config_t *)mf;
 	pReq->Action = pCfg->action;
#2 lore raw
Quoted 4 lines · James Bottomley

I was still thinking of something that made mpt_config() never return -EAGAIN. How does the following work out? It's my first pass at sorting out the frame and active logic in mptbase:

pCfg->wait_done needs to be initialized to zero before sleeping.

It takes seven to ten seconds for the ioc->active value to become non-zero and the wake up to occur.

It appears that the order of execution during "lsiutil 99" testing has changed. Once the driver marks the fibre targets missing, they never return. Hmmm.... Until this is resolved, this patch cannot be applied. I'll see if I can sort out how the execution order has changed.

Another comment, for every i/o, this patch introduces overhead. spinlock and checking a list which is empty 99.99[...]% of the time. I'll have to perform an iop measurement to see if there is a measurable impact on system performance.

Mike

Quoted 124 lines

James

drivers/message/fusion/mptbase.c +19 -10
index a300840..585f42f 100644
@@ -121,6 +121,11 @@ static int	mpt_base_index = -1;
 static int	last_drv_idx = -1;
 
 static DECLARE_WAIT_QUEUE_HEAD(mpt_waitq);
+/* This waitq is used for mpt_get_msg_frame failures which may be
+ * caused either by the ioc->active being zero or by the adapter being
+ * out of frames.  Receiving this event doesn't guarantee that a frame
+ * is available, so you must check */
+static DECLARE_WAIT_QUEUE_HEAD(mpt_config_waitq);
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
@@ -722,6 +727,13 @@ mpt_device_driver_deregister(int cb_idx)
 	MptDeviceDriverHandlers[cb_idx] = NULL;
 }
 
+static void
+mpt_ioc_activate(MPT_ADAPTER *ioc)
+{
+	ioc->active = 1;
+	wake_up(&mpt_config_waitq);
+}
+
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
@@ -835,6 +847,7 @@ #endif
 	mf_dma_addr = (ioc->req_frames_low_dma + req_offset) | ioc->RequestNB[req_idx];
 	dsgprintk((MYIOC_s_INFO_FMT "mf_dma_addr=%x req_idx=%d RequestNB=%x\n", ioc->name, mf_dma_addr, req_idx, ioc->RequestNB[req_idx]));
 	CHIPREG_WRITE32(&ioc->chip->RequestFifo, mf_dma_addr);
+	wake_up(&mpt_config_waitq);
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -1582,7 +1595,7 @@ mpt_resume(struct pci_dev *pdev)
 
 	/* enable interrupts */
 	CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-	ioc->active = 1;
+	mpt_ioc_activate(ioc);
 
 	printk(MYIOC_s_INFO_FMT
 		"pci-resume: ioc-state=0x%x,doorbell=0x%x\n",
@@ -1682,7 +1695,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 				dprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 						ioc->alt_ioc->name));
 				CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-				ioc->alt_ioc->active = 1;
+				mpt_ioc_activate(ioc->alt_ioc);
 			}
 
 		} else {
@@ -1798,7 +1811,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 	if (ret == 0) {
 		/* Enable! (reply interrupt) */
 		CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->active = 1;
+		mpt_ioc_activate(ioc);
 	}
 
 	if (reset_alt_ioc_active && ioc->alt_ioc) {
@@ -1806,7 +1819,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 		dinitprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 				ioc->alt_ioc->name));
 		CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->alt_ioc->active = 1;
+		mpt_ioc_activate(ioc->alt_ioc);
 	}
 
 	/*  Enable MPT base driver management of EventNotification
@@ -4069,7 +4082,6 @@ #endif
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -4181,7 +4193,6 @@ GetLanConfigPages(MPT_ADAPTER *ioc)
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -4493,7 +4504,6 @@ mptbase_raid_process_event_data(MPT_ADAP
  *	Returns: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -5156,7 +5166,6 @@ SendEventAck(MPT_ADAPTER *ioc, EventNoti
  *
  *	Returns 0 for success
  *	-EPERM if not allowed due to ISR context
- *	-EAGAIN if no msg frames currently available
  *	-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -5182,10 +5191,10 @@ mpt_config(MPT_ADAPTER *ioc, CONFIGPARMS
 
 	/* Get and Populate a free Frame
 	 */
-	if ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	while ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
 		dcprintk((MYIOC_s_WARN_FMT "mpt_config: no msg frames!\n",
 				ioc->name));
-		return -EAGAIN;
+		wait_event(mpt_config_waitq, pCfg->wait_done);
 	}
 	pReq = (Config_t *)mf;
 	pReq->Action = pCfg->action;
#3 lore raw
Quoted 1 line · Michael Reed

James Bottomley wrote:

...snip...

Quoted 7 lines

It appears that the order of execution during "lsiutil 99" testing has changed. Once the driver marks the fibre targets missing, they never return. Hmmm.... Until this is resolved, this patch cannot be applied. I'll see if I can sort out how the execution order has changed.

I found the problem with the order of execution and it's my own doing. When I added the setup_reset_work, it can get queued while the rescan work is running, and THEN the rescan work count can be bumped. As the rescan work loops on the work count, this changes the exec order. Doh. The second rescan needs to execute after the setup reset work.

Mike

Quoted 133 lines

Mike

Quoted 113 lines

James

drivers/message/fusion/mptbase.c +17 -8
index a300840..585f42f 100644
@@ -121,6 +121,11 @@ static int	mpt_base_index = -1;
 static int	last_drv_idx = -1;
 
 static DECLARE_WAIT_QUEUE_HEAD(mpt_waitq);
+/* This waitq is used for mpt_get_msg_frame failures which may be
+ * caused either by the ioc->active being zero or by the adapter being
+ * out of frames.  Receiving this event doesn't guarantee that a frame
+ * is available, so you must check */
+static DECLARE_WAIT_QUEUE_HEAD(mpt_config_waitq);
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
@@ -722,6 +727,13 @@ mpt_device_driver_deregister(int cb_idx)
 	MptDeviceDriverHandlers[cb_idx] = NULL;
 }
 
+static void
+mpt_ioc_activate(MPT_ADAPTER *ioc)
+{
+	ioc->active = 1;
+	wake_up(&mpt_config_waitq);
+}
+
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
@@ -835,6 +847,7 @@ #endif
 	mf_dma_addr = (ioc->req_frames_low_dma + req_offset) | ioc->RequestNB[req_idx];
 	dsgprintk((MYIOC_s_INFO_FMT "mf_dma_addr=%x req_idx=%d RequestNB=%x\n", ioc->name, mf_dma_addr, req_idx, ioc->RequestNB[req_idx]));
 	CHIPREG_WRITE32(&ioc->chip->RequestFifo, mf_dma_addr);
+	wake_up(&mpt_config_waitq);
 }
 
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -1582,7 +1595,7 @@ mpt_resume(struct pci_dev *pdev)
 
 	/* enable interrupts */
 	CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-	ioc->active = 1;
+	mpt_ioc_activate(ioc);
 
 	printk(MYIOC_s_INFO_FMT
 		"pci-resume: ioc-state=0x%x,doorbell=0x%x\n",
@@ -1682,7 +1695,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 				dprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 						ioc->alt_ioc->name));
 				CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-				ioc->alt_ioc->active = 1;
+				mpt_ioc_activate(ioc->alt_ioc);
 			}
 
 		} else {
@@ -1798,7 +1811,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 	if (ret == 0) {
 		/* Enable! (reply interrupt) */
 		CHIPREG_WRITE32(&ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->active = 1;
+		mpt_ioc_activate(ioc);
 	}
 
 	if (reset_alt_ioc_active && ioc->alt_ioc) {
@@ -1806,7 +1819,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3
 		dinitprintk((KERN_INFO MYNAM ": alt-%s reply irq re-enabled\n",
 				ioc->alt_ioc->name));
 		CHIPREG_WRITE32(&ioc->alt_ioc->chip->IntMask, MPI_HIM_DIM);
-		ioc->alt_ioc->active = 1;
+		mpt_ioc_activate(ioc->alt_ioc);
 	}
 
 	/*  Enable MPT base driver management of EventNotification
@@ -4069,7 +4082,6 @@ #endif
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -4181,7 +4193,6 @@ GetLanConfigPages(MPT_ADAPTER *ioc)
  *	Return: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -4493,7 +4504,6 @@ mptbase_raid_process_event_data(MPT_ADAP
  *	Returns: 0 for success
  *	-ENOMEM if no memory available
  *		-EPERM if not allowed due to ISR context
- *		-EAGAIN if no msg frames currently available
  *		-EFAULT for non-successful reply or no reply (timeout)
  */
 static int
@@ -5156,7 +5166,6 @@ SendEventAck(MPT_ADAPTER *ioc, EventNoti
  *
  *	Returns 0 for success
  *	-EPERM if not allowed due to ISR context
- *	-EAGAIN if no msg frames currently available
  *	-EFAULT for non-successful reply or no reply (timeout)
  */
 int
@@ -5182,10 +5191,10 @@ mpt_config(MPT_ADAPTER *ioc, CONFIGPARMS
 
 	/* Get and Populate a free Frame

drivers/message/fusion/mptbase.c mpt_config(MPT_ADAPTER *ioc, CONFIGPARMS · line 5200

Quoted 10 lines
drivers/message/fusion/mptbase.c +2 -2
 	 */
-	if ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	while ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
 		dcprintk((MYIOC_s_WARN_FMT "mpt_config: no msg frames!\n",
 				ioc->name));
-		return -EAGAIN;
+		wait_event(mpt_config_waitq, pCfg->wait_done);
 	}
 	pReq = (Config_t *)mf;
 	pReq->Action = pCfg->action;

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html