* [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals @ 2006-05-17 22:17 Eric Moore 2006-05-17 23:06 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Eric Moore @ 2006-05-17 22:17 UTC (permalink / raw) To: linux-scsi; +Cc: James.Bottomley All registered reset callback handlers are called during reset processing. The mptspi modules has its own reset callback handler, just recently added for issuing domain validation after host reset. If either the mptsas or mptfc driver are loaded, this callback could be called. Thus resulting in domain validation being issued for sas or fibre end devices. This patch insures domain validation is only occuring on spi end devices This is urgent bug fix. Pls apply this to scsi-rc-fixes-2.6, as well as this patch posted yesterday: http://marc.theaimsgroup.com/?l=linux-scsi&m=114782261719616&w=2 Signed-off-by: Eric Moore <Eric.Moore@lsil.com> --- rc4u/drivers/message/fusion/mptspi.c 2006-05-12 00:14:15.000000000 -0500 +++ rc4/drivers/message/fusion/mptspi.c 2006-05-17 12:23:08.924423654 -0500 @@ -825,6 +825,9 @@ rc = mptscsih_ioc_reset(ioc, reset_phase); + if (rc == 0 || ioc->bus_type != SPI) + return rc; + if (reset_phase == MPT_IOC_POST_RESET) mptspi_dv_renegotiate(hd); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals 2006-05-17 22:17 [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals Eric Moore @ 2006-05-17 23:06 ` James Bottomley 2006-05-18 14:45 ` Michael Reed 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2006-05-17 23:06 UTC (permalink / raw) To: Eric Moore; +Cc: linux-scsi On Wed, 2006-05-17 at 16:17 -0600, Eric Moore wrote: > All registered reset callback handlers are called during reset processing. > The mptspi modules has its own reset callback handler, just recently > added for issuing domain validation after host reset. If either the mptsas or > mptfc driver are loaded, this callback could be called. Thus resulting > in domain validation being issued for sas or fibre end devices. > This patch insures domain validation is only occuring on spi end devices > > This is urgent bug fix. Pls apply this to scsi-rc-fixes-2.6, > as well as this patch posted yesterday: > http://marc.theaimsgroup.com/?l=linux-scsi&m=114782261719616&w=2 That's a pretty nasty bug ... and it will bite on the other reset handlers, won't it as well? Shouldn't we be fixing it this way instead: James diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c index 9080853..a300840 100644 --- a/drivers/message/fusion/mptbase.c +++ b/drivers/message/fusion/mptbase.c @@ -1605,6 +1605,21 @@ mpt_resume(struct pci_dev *pdev) } #endif +static int +mpt_signal_reset(int index, MPT_ADAPTER *ioc, int reset_phase) +{ + if ((MptDriverClass[index] == MPTSPI_DRIVER && + ioc->bus_type != SPI) || + (MptDriverClass[index] == MPTFC_DRIVER && + ioc->bus_type != FC) || + (MptDriverClass[index] == MPTSAS_DRIVER && + ioc->bus_type != SAS)) + /* make sure we only call the relevant reset handler + * for the bus */ + return 0; + return (MptResetHandlers[index])(ioc, reset_phase); +} + /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ /* * mpt_do_ioc_recovery - Initialize or recover MPT adapter. @@ -1885,14 +1900,14 @@ #endif if ((ret == 0) && MptResetHandlers[ii]) { dprintk((MYIOC_s_INFO_FMT "Calling IOC post_reset handler #%d\n", ioc->name, ii)); - rc += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_POST_RESET); + rc += mpt_signal_reset(ii, ioc, MPT_IOC_POST_RESET); handlers++; } if (alt_ioc_ready && MptResetHandlers[ii]) { drsprintk((MYIOC_s_INFO_FMT "Calling alt-%s post_reset handler #%d\n", ioc->name, ioc->alt_ioc->name, ii)); - rc += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_POST_RESET); + rc += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_POST_RESET); handlers++; } } @@ -3267,11 +3282,11 @@ #endif if (MptResetHandlers[ii]) { dprintk((MYIOC_s_INFO_FMT "Calling IOC pre_reset handler #%d\n", ioc->name, ii)); - r += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_PRE_RESET); + r += mpt_signal_reset(ii, ioc, MPT_IOC_PRE_RESET); if (ioc->alt_ioc) { dprintk((MYIOC_s_INFO_FMT "Calling alt-%s pre_reset handler #%d\n", ioc->name, ioc->alt_ioc->name, ii)); - r += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_PRE_RESET); + r += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_PRE_RESET); } } } @@ -5706,11 +5721,11 @@ #endif if (MptResetHandlers[ii]) { dtmprintk((MYIOC_s_INFO_FMT "Calling IOC reset_setup handler #%d\n", ioc->name, ii)); - r += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_SETUP_RESET); + r += mpt_signal_reset(ii, ioc, MPT_IOC_SETUP_RESET); if (ioc->alt_ioc) { dtmprintk((MYIOC_s_INFO_FMT "Calling alt-%s setup reset handler #%d\n", ioc->name, ioc->alt_ioc->name, ii)); - r += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_SETUP_RESET); + r += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_SETUP_RESET); } } } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals 2006-05-17 23:06 ` James Bottomley @ 2006-05-18 14:45 ` Michael Reed 2006-05-18 14:54 ` James Bottomley 0 siblings, 1 reply; 6+ messages in thread From: Michael Reed @ 2006-05-18 14:45 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Moore, Eric Dean This patch looks as though it will result in not calling the reset handler for MPTBASE_DRIVER, MPTCTL_DRIVER, or MPTLAN_DRIVER. The base and control drivers aren't associated with any particular board type. The other registered reset handlers, i.e., not mptspi_ioc_reset, already properly determine if they should execute. NACK. (As If I get to make this decision!) Thanks, Mike James Bottomley wrote: > On Wed, 2006-05-17 at 16:17 -0600, Eric Moore wrote: >> All registered reset callback handlers are called during reset processing. >> The mptspi modules has its own reset callback handler, just recently >> added for issuing domain validation after host reset. If either the mptsas or >> mptfc driver are loaded, this callback could be called. Thus resulting >> in domain validation being issued for sas or fibre end devices. >> This patch insures domain validation is only occuring on spi end devices >> >> This is urgent bug fix. Pls apply this to scsi-rc-fixes-2.6, >> as well as this patch posted yesterday: >> http://marc.theaimsgroup.com/?l=linux-scsi&m=114782261719616&w=2 > > That's a pretty nasty bug ... and it will bite on the other reset > handlers, won't it as well? Shouldn't we be fixing it this way instead: > > James > > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c > index 9080853..a300840 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -1605,6 +1605,21 @@ mpt_resume(struct pci_dev *pdev) > } > #endif > > +static int > +mpt_signal_reset(int index, MPT_ADAPTER *ioc, int reset_phase) > +{ > + if ((MptDriverClass[index] == MPTSPI_DRIVER && > + ioc->bus_type != SPI) || > + (MptDriverClass[index] == MPTFC_DRIVER && > + ioc->bus_type != FC) || > + (MptDriverClass[index] == MPTSAS_DRIVER && > + ioc->bus_type != SAS)) > + /* make sure we only call the relevant reset handler > + * for the bus */ > + return 0; > + return (MptResetHandlers[index])(ioc, reset_phase); > +} > + > /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/ > /* > * mpt_do_ioc_recovery - Initialize or recover MPT adapter. > @@ -1885,14 +1900,14 @@ #endif > if ((ret == 0) && MptResetHandlers[ii]) { > dprintk((MYIOC_s_INFO_FMT "Calling IOC post_reset handler #%d\n", > ioc->name, ii)); > - rc += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_POST_RESET); > + rc += mpt_signal_reset(ii, ioc, MPT_IOC_POST_RESET); > handlers++; > } > > if (alt_ioc_ready && MptResetHandlers[ii]) { > drsprintk((MYIOC_s_INFO_FMT "Calling alt-%s post_reset handler #%d\n", > ioc->name, ioc->alt_ioc->name, ii)); > - rc += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_POST_RESET); > + rc += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_POST_RESET); > handlers++; > } > } > @@ -3267,11 +3282,11 @@ #endif > if (MptResetHandlers[ii]) { > dprintk((MYIOC_s_INFO_FMT "Calling IOC pre_reset handler #%d\n", > ioc->name, ii)); > - r += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_PRE_RESET); > + r += mpt_signal_reset(ii, ioc, MPT_IOC_PRE_RESET); > if (ioc->alt_ioc) { > dprintk((MYIOC_s_INFO_FMT "Calling alt-%s pre_reset handler #%d\n", > ioc->name, ioc->alt_ioc->name, ii)); > - r += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_PRE_RESET); > + r += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_PRE_RESET); > } > } > } > @@ -5706,11 +5721,11 @@ #endif > if (MptResetHandlers[ii]) { > dtmprintk((MYIOC_s_INFO_FMT "Calling IOC reset_setup handler #%d\n", > ioc->name, ii)); > - r += (*(MptResetHandlers[ii]))(ioc, MPT_IOC_SETUP_RESET); > + r += mpt_signal_reset(ii, ioc, MPT_IOC_SETUP_RESET); > if (ioc->alt_ioc) { > dtmprintk((MYIOC_s_INFO_FMT "Calling alt-%s setup reset handler #%d\n", > ioc->name, ioc->alt_ioc->name, ii)); > - r += (*(MptResetHandlers[ii]))(ioc->alt_ioc, MPT_IOC_SETUP_RESET); > + r += mpt_signal_reset(ii, ioc->alt_ioc, MPT_IOC_SETUP_RESET); > } > } > } > > > - > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals 2006-05-18 14:45 ` Michael Reed @ 2006-05-18 14:54 ` James Bottomley 2006-05-18 15:11 ` Michael Reed 0 siblings, 1 reply; 6+ messages in thread From: James Bottomley @ 2006-05-18 14:54 UTC (permalink / raw) To: Michael Reed; +Cc: linux-scsi, Moore, Eric Dean On Thu, 2006-05-18 at 09:45 -0500, Michael Reed wrote: > This patch looks as though it will result in not calling > the reset handler for MPTBASE_DRIVER, MPTCTL_DRIVER, or > MPTLAN_DRIVER. The base and control drivers aren't > associated with any particular board type. I don't believe it will ... what makes you think that? James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals 2006-05-18 14:54 ` James Bottomley @ 2006-05-18 15:11 ` Michael Reed 2006-05-18 15:46 ` Michael Reed 0 siblings, 1 reply; 6+ messages in thread From: Michael Reed @ 2006-05-18 15:11 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Moore, Eric Dean James Bottomley wrote: > On Thu, 2006-05-18 at 09:45 -0500, Michael Reed wrote: >> This patch looks as though it will result in not calling >> the reset handler for MPTBASE_DRIVER, MPTCTL_DRIVER, or >> MPTLAN_DRIVER. The base and control drivers aren't >> associated with any particular board type. > > I don't believe it will ... what makes you think that? Mis-reading the code. :( I'll test this patch in my config this morning. (I'll pull Eric's fix so that I know for sure it works as intended.) Mike ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals 2006-05-18 15:11 ` Michael Reed @ 2006-05-18 15:46 ` Michael Reed 0 siblings, 0 replies; 6+ messages in thread From: Michael Reed @ 2006-05-18 15:46 UTC (permalink / raw) To: Michael Reed; +Cc: James Bottomley, linux-scsi, Moore, Eric Dean Michael Reed wrote: > > I'll test this patch in my config this morning. > (I'll pull Eric's fix so that I know for sure > it works as intended.) Initial testing suggests that this patch is correct. (Got a rag for the egg?) mptctl_ioc_reset, mpt_ioc_reset are entered when resets are induced. mptfc_ioc_reset (new, coming) is entered, mptspi_ioc_reset is NOT entered. James, your patch is arguably more correct. Eric? Mike ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-05-18 15:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-05-17 22:17 [PATCH] fusion - mptspi - reset handler shouldn't be called for other bus protocals Eric Moore 2006-05-17 23:06 ` James Bottomley 2006-05-18 14:45 ` Michael Reed 2006-05-18 14:54 ` James Bottomley 2006-05-18 15:11 ` Michael Reed 2006-05-18 15:46 ` Michael Reed
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.