[PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[mdr]

This patch applies to 2.6.17-rc3 after applying these patches from
scsi-rc-fixes-2.6.git.

   [SCSI] mptfusion: bug fix's for raid components adding/deleting
   [SCSI] - fusion - mptfc bug fix's to prevent deadlock situations
   [SCSI] fusion - bug fix stack overflow in mptbase

A race condition exists in mptfc between the thread registering a device
with the fc transport and the scan work generated by the transport.
This race existed prior to the application of the mptfc bug fix patch.

mptfc_register_dev() calls fc_remote_port_add() with the FC_RPORT_ROLE_TARGET
bit set in the rport ids passed to the function.  Having this bit set causes
fc_remote_port_add() to schedule a scan of the device.

This scan can execute before mptfc_register_dev() can fill in the dd_data
in the rport structure.  When this happens, mptfc_target_alloc() will fail
because dd_data is null.

Attached is a patch which fixes the problem.  The patch changes the rport ids
passed to fc_remote_port_add() to not have the TARGET bit set.  This prevents
the scan from being scheduled.  After mptfc_register_dev() fills in the rport
dd_data field, fc_remote_port_rolechg() is called, changing the role of the
rport to TARGET.  Thus, the scan is scheduled after dd_data is filled
in which prevents the failure in mptfc_target_alloc().

Based on a private email from Eric Moore, I'm providing his sign-off.

> Mike - Pls go ahead with posting this patch
> to linux-scsi@ and bugzilla 170314.   Steve Shirron
> has reveiwed this, and I believe they will be testing
> the latest drivers as well.
>
> I'm not going to have time today to post
> this, and next week I have Jury Duty.
>
> Eric


Signed-off-by: Michael Reed <mdr@sgi.com>
Signed-off-by: Eric Moore <Eric.Moore@lsil.com>


--- a/drivers/message/fusion/mptfc.c	2006-04-27 12:52:53.790656757 -0500
+++ b/drivers/message/fusion/mptfc.c	2006-04-27 13:02:47.810987260 -0500
@@ -341,9 +341,6 @@
 	rid->port_name = ((u64)pg0->WWPN.High) << 32 | (u64)pg0->WWPN.Low;
 	rid->port_id =   pg0->PortIdentifier;
 	rid->roles = FC_RPORT_ROLE_UNKNOWN;
-	rid->roles |= FC_RPORT_ROLE_FCP_TARGET;
-	if (pg0->Protocol & MPI_FC_DEVICE_PAGE0_PROT_FCP_INITIATOR)
-		rid->roles |= FC_RPORT_ROLE_FCP_INITIATOR;
 
 	return 0;
 }
@@ -357,10 +354,15 @@
 	int			new_ri = 1;
 	u64			pn, nn;
 	VirtTarget		*vtarget;
+	u32			roles = FC_RPORT_ROLE_UNKNOWN;
 
 	if (mptfc_generate_rport_ids(pg0, &rport_ids) < 0)
 		return;
 
+	roles |= FC_RPORT_ROLE_FCP_TARGET;
+	if (pg0->Protocol & MPI_FC_DEVICE_PAGE0_PROT_FCP_INITIATOR)
+		roles |= FC_RPORT_ROLE_FCP_INITIATOR;
+
 	/* scan list looking for a match */
 	list_for_each_entry(ri, &ioc->fc_rports, list) {
 		pn = (u64)ri->pg0.WWPN.High << 32 | (u64)ri->pg0.WWPN.Low;
@@ -400,8 +402,9 @@
 					vtarget->bus_id = pg0->CurrentBus;
 				}
 			}
-			/* once dd_data is filled in, commands will issue to hardware */
 			*((struct mptfc_rport_info **)rport->dd_data) = ri;
+			/* scan will be scheduled once rport becomes a target */
+			fc_remote_port_rolechg(rport,roles);
 
 			pn = (u64)ri->pg0.WWPN.High << 32 | (u64)ri->pg0.WWPN.Low;
 			nn = (u64)ri->pg0.WWNN.High << 32 | (u64)ri->pg0.WWNN.Low;

RE: [PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[Moore, Eric]

On Monday, May 01, 2006 12:07 PM, Micheal Reed wrote: 

> 
> 
> This patch applies to 2.6.17-rc3 after applying these patches from
> scsi-rc-fixes-2.6.git.
> 
>    [SCSI] mptfusion: bug fix's for raid components adding/deleting
>    [SCSI] - fusion - mptfc bug fix's to prevent deadlock situations
>    [SCSI] fusion - bug fix stack overflow in mptbase
> 

These three patchs are there in James Bottomley's rc-fixes-2.6 tree.

http://www.kernel.org/pub/linux/kernel/people/jejb/scsi-rc-fixes-2.6.dif
f

Eric

RE: [PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[James Bottomley]

On Mon, 2006-05-01 at 12:10 -0600, Moore, Eric wrote:
> On Monday, May 01, 2006 12:07 PM, Micheal Reed wrote: 
> > This patch applies to 2.6.17-rc3 after applying these patches from
> > scsi-rc-fixes-2.6.git.
> > 
> >    [SCSI] mptfusion: bug fix's for raid components adding/deleting
> >    [SCSI] - fusion - mptfc bug fix's to prevent deadlock situations
> >    [SCSI] fusion - bug fix stack overflow in mptbase

Is there an Ack for this patch?

James

Re: [PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[Michael Reed]

I think it's implicit in both the private email I included and Eric's
posting of 05/01/2006 at 13:10PM (CDT).

> Based on a private email from Eric Moore, I'm providing his sign-off.
> 
>> > Mike - Pls go ahead with posting this patch
>> > to linux-scsi@ and bugzilla 170314.   Steve Shirron
>> > has reveiwed this, and I believe they will be testing
>> > the latest drivers as well.
>> >
>> > I'm not going to have time today to post
>> > this, and next week I have Jury Duty.
>> >
>> > Eric
> 
> 

http://marc.theaimsgroup.com/?l=linux-scsi&m=114650712415619&w=2

Doesn't mean that Eric won't provide what you need....

Mike



James Bottomley wrote:
> On Mon, 2006-05-01 at 12:10 -0600, Moore, Eric wrote:
>> On Monday, May 01, 2006 12:07 PM, Micheal Reed wrote: 
>>> This patch applies to 2.6.17-rc3 after applying these patches from
>>> scsi-rc-fixes-2.6.git.
>>>
>>>    [SCSI] mptfusion: bug fix's for raid components adding/deleting
>>>    [SCSI] - fusion - mptfc bug fix's to prevent deadlock situations
>>>    [SCSI] fusion - bug fix stack overflow in mptbase
> 
> Is there an Ack for this patch?
> 
> James
> 
> 
> -
> 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
> 
> 

RE: [PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc

[Moore, Eric]

On Wednesday, May 03, 2006 12:09 PM, James Bottomley wrote:  
> 
> On Mon, 2006-05-01 at 12:10 -0600, Moore, Eric wrote:
> > On Monday, May 01, 2006 12:07 PM, Micheal Reed wrote: 
> > > This patch applies to 2.6.17-rc3 after applying these patches from
> > > scsi-rc-fixes-2.6.git.
> > > 
> > >    [SCSI] mptfusion: bug fix's for raid components adding/deleting
> > >    [SCSI] - fusion - mptfc bug fix's to prevent deadlock 
> situations
> > >    [SCSI] fusion - bug fix stack overflow in mptbase
> 
> Is there an Ack for this patch?

ACK