[PATCH 2/3] - mpt fusion -

[PATCH 2/3] - mpt fusion -

[Moore, Eric]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

Bug fix - Due to a recent change in scsi_transport_sas,
in the function sas_rphy_add(), its possible to have the
same target mapped to two different device nodes, 
meaning /dev/sda at id=0 and /dev/sdb at id=1, are remapped
in mptsas to the same device at id=foo.

I'll explain the problem.
The sas drivers have a rphy to rphy relationship.  An rphy
created on a hba phy points to the expander phy.  Then the expander
phy creates a rphy pointing back to the same hba rphy. The expander
phy which contains the rphy pointing back to the hba phy, contains
the attributes device_type = SAS_END_DEVICE, and target_port_protocals =
0.
Hence the recent change in sas_rphy_add() make it drop down to
scsi_scan_target, whereas before there was sepecial handling to 
prevent this case from happening.  With this change, the mptsas slave
entry 
points are called to enumerate the rphy that points back to the hba phy.
The target id in our data structures is the same as a real target device
on another phy.  Thus we end up mapping the same target to two different
rphys.
To fix this, I added a check in mptsas_slave_alloc to make sure the
requested rphy is mapped to a true end device.

JEJB: Pls apply to scsi-rc-fixes tree.

Signed-off-by: Eric Moore <Eric.Moore@lsil.com>

[-- Attachment #2: duplicate_targets.patch --]
[-- Type: application/octet-stream, Size: 1988 bytes --]

diff -uarN b/drivers/message/fusion/mptsas.c a/drivers/message/fusion/mptsas.c
--- b/drivers/message/fusion/mptsas.c	2006-04-13 15:08:27.000000000 -0600
+++ a/drivers/message/fusion/mptsas.c	2006-04-13 15:11:15.000000000 -0600
@@ -298,6 +298,26 @@
 	return rc;
 }
 
+/*
+ * Returns true if there is a scsi end device
+ */
+static inline int
+mptsas_is_end_device(struct mptsas_devinfo * attached)
+{
+	if ((attached->handle) &&
+	    (attached->device_info &
+	    MPI_SAS_DEVICE_INFO_END_DEVICE) &&
+	    ((attached->device_info &
+	    MPI_SAS_DEVICE_INFO_SSP_TARGET) |
+	    (attached->device_info &
+	    MPI_SAS_DEVICE_INFO_STP_TARGET) |
+	    (attached->device_info &
+	    MPI_SAS_DEVICE_INFO_SATA_DEVICE)))
+		return 1;
+	else
+		return 0;
+}
+
 static int
 mptsas_sas_enclosure_pg0(MPT_ADAPTER *ioc, struct mptsas_enclosure *enclosure,
 		u32 form, u32 form_specific)
@@ -429,6 +449,9 @@
 		for (i = 0; i < p->num_phys; i++) {
 			if (p->phy_info[i].attached.sas_address ==
 					rphy->identify.sas_address) {
+				if (!mptsas_is_end_device(
+					&p->phy_info[i].attached))
+					goto out_no_dev;
 				target_id = p->phy_info[i].attached.id;
 				vtarget->bus_id = p->phy_info[i].attached.channel;
 				vdev->lun = sdev->lun;
@@ -448,6 +471,7 @@
 			}
 		}
 	}
+ out_no_dev:
 	mutex_unlock(&hd->ioc->sas_topology_mutex);
 
 	kfree(vdev);
@@ -1068,26 +1092,6 @@
 	return error;
 }
 
-/*
- * Returns true if there is a scsi end device
- */
-static inline int
-mptsas_is_end_device(struct mptsas_devinfo * attached)
-{
-	if ((attached->handle) &&
-	    (attached->device_info &
-	    MPI_SAS_DEVICE_INFO_END_DEVICE) &&
-	    ((attached->device_info &
-	    MPI_SAS_DEVICE_INFO_SSP_TARGET) |
-	    (attached->device_info &
-	    MPI_SAS_DEVICE_INFO_STP_TARGET) |
-	    (attached->device_info &
-	    MPI_SAS_DEVICE_INFO_SATA_DEVICE)))
-		return 1;
-	else
-		return 0;
-}
-
 static void
 mptsas_parse_device_info(struct sas_identify *identify,
 		struct mptsas_devinfo *device_info)

Re: [PATCH 2/3] - mpt fusion -

[Christoph Hellwig]

On Thu, Apr 13, 2006 at 04:08:22PM -0600, Moore, Eric wrote:
> Bug fix - Due to a recent change in scsi_transport_sas,
> in the function sas_rphy_add(), its possible to have the
> same target mapped to two different device nodes, 
> meaning /dev/sda at id=0 and /dev/sdb at id=1, are remapped
> in mptsas to the same device at id=foo.
> 
> I'll explain the problem.
> The sas drivers have a rphy to rphy relationship.  An rphy
> created on a hba phy points to the expander phy.  Then the expander
> phy creates a rphy pointing back to the same hba rphy. The expander
> phy which contains the rphy pointing back to the hba phy, contains
> the attributes device_type = SAS_END_DEVICE, and target_port_protocals =
> 0.
> Hence the recent change in sas_rphy_add() make it drop down to
> scsi_scan_target, whereas before there was sepecial handling to 
> prevent this case from happening.  With this change, the mptsas slave
> entry 
> points are called to enumerate the rphy that points back to the hba phy.
> The target id in our data structures is the same as a real target device
> on another phy.  Thus we end up mapping the same target to two different
> rphys.
> To fix this, I added a check in mptsas_slave_alloc to make sure the
> requested rphy is mapped to a true end device.

This needs to be fixed in the transport class again.

Re: [PATCH 2/3] - mpt fusion -

[Moore, Eric]

On Thursday, April 13, 2006 7:04 PM, Christoph Hellwig wrote:

>> 
>> I'll explain the problem.
>> The sas drivers have a rphy to rphy relationship.  An rphy
>> created on a hba phy points to the expander phy.  Then the expander
>> phy creates a rphy pointing back to the same hba rphy. The expander
>> phy which contains the rphy pointing back to the hba phy, contains
>> the attributes device_type = SAS_END_DEVICE, and target_port_protocals =
>> 0.
>> Hence the recent change in sas_rphy_add() make it drop down to
>> scsi_scan_target, whereas before there was sepecial handling to 
>> prevent this case from happening.  With this change, the mptsas slave
>> entry 
>> points are called to enumerate the rphy that points back to the hba phy.
>> The target id in our data structures is the same as a real target device
>> on another phy.  Thus we end up mapping the same target to two different
>> rphys.
>> To fix this, I added a check in mptsas_slave_alloc to make sure the
>> requested rphy is mapped to a true end device.
> 
> This needs to be fixed in the transport class again.
> 

Christoph can you help me on this, please?
The original sas_rphy_add() function from three weeks back works for mptsas.
I'm not sure why that change was made, and whether reverting back to its orginal code
would break the other sas driver.  

Eric

Re: [PATCH 2/3] - mpt fusion -

[James Bottomley]

On Thu, 2006-04-13 at 20:14 -0600, Moore, Eric wrote:
> Christoph can you help me on this, please?
> The original sas_rphy_add() function from three weeks back works for mptsas.
> I'm not sure why that change was made, and whether reverting back to its orginal code
> would break the other sas driver.  

Does this fix the problem?

James

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 2c96fc5..8b6d65e 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -845,6 +845,8 @@ int sas_rphy_add(struct sas_rphy *rphy)
 	    (identify->target_port_protocols &
 	     (SAS_PROTOCOL_SSP|SAS_PROTOCOL_STP|SAS_PROTOCOL_SATA)))
 		rphy->scsi_target_id = sas_host->next_target_id++;
+	else if (identify->device_type == SAS_END_DEVICE)
+		rphy->scsi_target_id = -1;
 	mutex_unlock(&sas_host->lock);
 
 	if (identify->device_type == SAS_END_DEVICE &&

Re: [PATCH 2/3] - mpt fusion -

[Moore, Eric]

On Friday, April 14, 2006 8:47 AM, James Bottomley wrote:


> On Thu, 2006-04-13 at 20:14 -0600, Moore, Eric wrote:
>> Christoph can you help me on this, please?
>> The original sas_rphy_add() function from three weeks back works for mptsas.
>> I'm not sure why that change was made, and whether reverting back to its orginal code
>> would break the other sas driver.  
> 
> Does this fix the problem?
> 

I think this will fix it.  I appears to do what the previous sas_rphy_add() had done, so I'm happy.
However, today was a holiday, and I went skiing one more time. So I hadn't tested it out yet.   
I noticed that you've submitted this with your scsi bug fix's to Linus, so thanks.

Can we get that [PATCH 3/3] fusion patch in for rc2?    I had fix some bugs in the handling 
reporting of raid components when power is turned off and on to the devices.   These are critical fix's.

Eric

Re: [PATCH 2/3] - mpt fusion -

[James Bottomley]

On Fri, 2006-04-14 at 18:45 -0600, Moore, Eric wrote:
> Can we get that [PATCH 3/3] fusion patch in for rc2?    I had fix some bugs in the handling 
> reporting of raid components when power is turned off and on to the devices.   These are critical fix's.

I think so, however, it applies but won't build, so it needs someone to
look at it.

James