mpt fusion - fibre channel target
Subsystems (2)
Quoted 2 lines · Michael Reed
There's another problem. The routines doing the wakeup don't have access to the sleep variable so the thread sleeps forever.
Mike - I think I addressed all the concerns with this patch, however if mpt_config is re-enterant, then all bets are off. Perhaps a mutex might work. Pls review and comment.
Eric Moore
LSI Logic
drivers/message/fusion/mptbase.c +25 -9
@@ -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_mf_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 ; + ioc->mf_wait_done =1 ; +wake_up (&mpt_mf_waitq); +}/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ /**
@@ -851,15 +863,21 @@ void mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf) {unsigned long flags; +int freeq_list_empty;/* Put Request back on FreeQ! */ spin_lock_irqsave (&ioc->FreeQlock, flags); + freeq_list_empty =list_empty (&ioc->FreeQ); mf->u.frame.linkage.arg1 =0xdeadbeaf ;/* signature to know if this mf is freed */ list_add_tail (&mf->u.frame.linkage.list, &ioc->FreeQ);#ifdef MFCNT ioc->mfcnt--;#endif spin_unlock_irqrestore (&ioc->FreeQlock, flags); +if (freeq_list_empty) { + ioc->mf_wait_done =1 ; +wake_up (&mpt_mf_waitq); + } }/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
@@ -1544,7 +1562,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",
@@ -1645,7 +1663,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 {
@@ -1803,7 +1821,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3if (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) {
@@ -1811,7 +1829,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
@@ -4072,7 +4090,6 @@ WaitForDoorbellReply(MPT_ADAPTER *ioc, i* 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
@@ -4394,7 +4411,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
@@ -5057,7 +5073,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
@@ -5083,10 +5098,11 @@ 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 ; + ioc->mf_wait_done =0 ; +wait_event (mpt_mf_waitq, ioc->mf_wait_done); } pReq = (Config_t *)mf; pReq->Action = pCfg->action;
drivers/message/fusion/mptbase.h +1 -0
@@ -602,6 +602,7 @@ typedef struct _MPT_ADAPTERFCPortPage0_t fc_port_page0[2 ];struct timer_list persist_timer;/* persist table timer */ int persist_wait_done;/* persist completion flag */ +int mf_wait_done;u8 persist_reply_frame[MPT_DEFAULT_FRAME_SIZE ];/* persist reply */ LANPage0_t lan_cnfg_page0;LANPage1_t lan_cnfg_page1;
Quoted 8 lines · Eric Moore
Quoted 2 lines · Michael Reed
There's another problem. The routines doing the wakeup don't have access to the sleep variable so the thread sleeps forever.
Mike - I think I addressed all the concerns with this patch, however if
mpt_configis re-enterant, then all bets are off. Perhaps a mutex might work. Pls review and comment.
Hi Eric,
I looked at the changes and they look okay. I ran them on my system and was able to induce the ioc->active==0 sleep. Thread wakes up as expected. I'm not able to evaluate if multiple threads will be calling mpt_config() and needing to sleep at the same time. Instead of a mutex, perhaps pass the sleep variable to mpt_config()? wake up all threads sleeping? But, don't know if this is an issue.
I didn't induce out of frame sleep, so the wakeup for that is untested.
If questions remain, I'll continue looking when I return from vacation.
Mike
Quoted 143 lines
Eric Moore
LSI Logicdiff -uarpN
b/drivers/message/fusion/mptbase.ca/drivers/message/fusion/mptbase.cdrivers/message/fusion/mptbase.c +25
-9@@ -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_mf_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 ; + ioc->mf_wait_done =1 ; +wake_up (&mpt_mf_waitq); +}/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ /** @@ -851,15 +863,21 @@ void mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf) {unsigned long flags; +int freeq_list_empty;/* Put Request back on FreeQ! */ spin_lock_irqsave (&ioc->FreeQlock, flags); + freeq_list_empty =list_empty (&ioc->FreeQ); mf->u.frame.linkage.arg1 =0xdeadbeaf ;/* signature to know if this mf is freed */ list_add_tail (&mf->u.frame.linkage.list, &ioc->FreeQ);#ifdef MFCNT ioc->mfcnt--;#endif spin_unlock_irqrestore (&ioc->FreeQlock, flags); +if (freeq_list_empty) { + ioc->mf_wait_done =1 ; +wake_up (&mpt_mf_waitq); + } }/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ @@ -1544,7 +1562,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",@@ -1645,7 +1663,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 {@@ -1803,7 +1821,7 @@ mpt_do_ioc_recovery(MPT_ADAPTER *ioc, u3if (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) {@@ -1811,7 +1829,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 @@ -4072,7 +4090,6 @@ WaitForDoorbellReply(MPT_ADAPTER *ioc, i* 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@@ -4394,7 +4411,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@@ -5057,7 +5073,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 @@ -5083,10 +5098,11 @@ 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 ; + ioc->mf_wait_done =0 ; +wait_event (mpt_mf_waitq, ioc->mf_wait_done); } pReq = (Config_t *)mf; pReq->Action = pCfg->action;diff -uarpN
b/drivers/message/fusion/mptbase.ha/drivers/message/fusion/mptbase.hdrivers/message/fusion/mptbase.h +1
-0@@ -602,6 +602,7 @@ typedef struct _MPT_ADAPTERFCPortPage0_t fc_port_page0[2 ];struct timer_list persist_timer;/* persist table timer */ int persist_wait_done;/* persist completion flag */ +int mf_wait_done;u8 persist_reply_frame[MPT_DEFAULT_FRAME_SIZE ];/* persist reply */ LANPage0_t lan_cnfg_page0;LANPage1_t lan_cnfg_page1;