From mboxrd@z Thu Jan 1 00:00:00 1970 From: mdr@sgi.com Subject: [PATCH] fusion - race between mptfc_register_dev and mptfc_target_alloc Date: Mon, 01 May 2006 13:07:04 -0500 Message-ID: <44564E48.Mail3RH11AK33@sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Return-path: Received: from omx2-ext.sgi.com ([192.48.171.19]:43432 "EHLO omx2.sgi.com") by vger.kernel.org with ESMTP id S932190AbWEASHJ (ORCPT ); Mon, 1 May 2006 14:07:09 -0400 Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: linux-scsi@vger.kernel.org Cc: James.Bottomley@SteelEye.com, Eric.Moore@lsil.com, jeremy@sgi.com, gwh@sgi.com, mdr@sgi.com 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 Signed-off-by: Eric Moore --- 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;