* Re: [REPOST][PATCH 1/6] mpt fusion - fibre channel target
@ 2006-06-15 18:05 Eric Moore
2006-06-15 21:57 ` Michael Reed
0 siblings, 1 reply; 2+ messages in thread
From: Eric Moore @ 2006-06-15 18:05 UTC (permalink / raw)
To: James.Bottomley, mdr, linux-scsi
On Thursday, June 15, 2006 11:50 AM, Michael Reed wrote:
> 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
diff -uarpN b/drivers/message/fusion/mptbase.c a/drivers/message/fusion/mptbase.c
--- b/drivers/message/fusion/mptbase.c 2006-06-13 15:56:02.000000000 -0600
+++ a/drivers/message/fusion/mptbase.c 2006-06-15 11:58:37.000000000 -0600
@@ -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, 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) {
@@ -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.h a/drivers/message/fusion/mptbase.h
--- b/drivers/message/fusion/mptbase.h 2006-06-13 15:56:02.000000000 -0600
+++ a/drivers/message/fusion/mptbase.h 2006-06-15 11:48:58.000000000 -0600
@@ -602,6 +602,7 @@ typedef struct _MPT_ADAPTER
FCPortPage0_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;
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [REPOST][PATCH 1/6] mpt fusion - fibre channel target
2006-06-15 18:05 [REPOST][PATCH 1/6] mpt fusion - fibre channel target Eric Moore
@ 2006-06-15 21:57 ` Michael Reed
0 siblings, 0 replies; 2+ messages in thread
From: Michael Reed @ 2006-06-15 21:57 UTC (permalink / raw)
To: Eric Moore; +Cc: James.Bottomley, linux-scsi
Eric Moore wrote:
> On Thursday, June 15, 2006 11:50 AM, Michael Reed wrote:
>
>> 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.
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
>
> Eric Moore
> LSI Logic
>
>
>
> diff -uarpN b/drivers/message/fusion/mptbase.c a/drivers/message/fusion/mptbase.c
> --- b/drivers/message/fusion/mptbase.c 2006-06-13 15:56:02.000000000 -0600
> +++ a/drivers/message/fusion/mptbase.c 2006-06-15 11:58:37.000000000 -0600
> @@ -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, 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) {
> @@ -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.h a/drivers/message/fusion/mptbase.h
> --- b/drivers/message/fusion/mptbase.h 2006-06-13 15:56:02.000000000 -0600
> +++ a/drivers/message/fusion/mptbase.h 2006-06-15 11:48:58.000000000 -0600
> @@ -602,6 +602,7 @@ typedef struct _MPT_ADAPTER
> FCPortPage0_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;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2006-06-15 21:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-15 18:05 [REPOST][PATCH 1/6] mpt fusion - fibre channel target Eric Moore
2006-06-15 21:57 ` Michael Reed
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox