public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] scsi/mptsas: add RAID devices in ascending ID order
@ 2006-09-25 16:14 Moore, Eric
  2006-09-26 12:12 ` Martin Wilck
  0 siblings, 1 reply; 6+ messages in thread
From: Moore, Eric @ 2006-09-25 16:14 UTC (permalink / raw)
  To: Martin Wilck; +Cc: linux-scsi, mpt_linux_developer

On Monday, September 25, 2006 3:59 AM, Martin Wilck wrote: 
> > Ascending ID ordering does not address all cases. 
> > There is no guarantee which target ids are 
> > assigned by firmware to volumes when
> > volumes are created.  
> 
> That's what my patch was supposed to handle: No matter what 
> the ordering 
> of IDs by firmware is, the devices are registered in 
> ascending ID order.

You patch will call scsi_add_device, from low to hi target 
ids, right?  Well if someone creates a volume that
is issued target id=8, installs the operating system.
Then tommorrow they create a new volume that is issued
target id=7, then what your patch will do is reverse
the order, right?  If so, a panic would occur
unless they assigned device names/lables.

> 
> > I've made similar 
> > ordering proposals before to this mailing list. 
> > All been rejected.
> 
> Do you have a thread reference for me? I'd like to understand the 
> arguments better. I find this strange because scanning in ascending 
> order has always been the common case in drivers as well as 
> in the mid 
> layer, AFAIK.

I was working with Christoph Hellwig and James Bottomley last
year implementing raid support in mptsas, and they rejected it
code that ordered raid ids.

BTW, the scsi mid layer is not scanning devices that use
transport layers, beside spi.  Meaning that lld's that are
fibre channel, iscsi, and sas hba will report the know devices
to transports, and the transport assign its own unique mapping.
For example, mptsas and mptfc are using transport, and we
don't call scsi_scan_host.  

 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] scsi/mptsas: add RAID devices in ascending ID order
@ 2006-09-26 18:33 Moore, Eric
  0 siblings, 0 replies; 6+ messages in thread
From: Moore, Eric @ 2006-09-26 18:33 UTC (permalink / raw)
  To: Martin Wilck; +Cc: linux-scsi, mpt_linux_developer

Martin Wilck wrote: 

> 
> Yes, if the order of VolumeIDs assigned by the firmware is indeed 
> random. That's not what was observed in our test lab, though.
> 

Yes, our firmware does persistent target mapping, and if you fill
up your table, you can create a volume with a large target id.  The
target id assigned usually will map to the least significant id
of one of your hidden disk ids, and that hidden disk id will be moved
to another id much larger during creation of the volume.  Then if you
end up creating a volume at a later date, that uses smaller target ids,
then
my case I presented in yesterdays email would occur.

Eric 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] scsi/mptsas: add RAID devices in ascending ID order
@ 2006-09-22 18:17 Moore, Eric
  2006-09-25  9:59 ` Martin Wilck
  0 siblings, 1 reply; 6+ messages in thread
From: Moore, Eric @ 2006-09-22 18:17 UTC (permalink / raw)
  To: martin.wilck; +Cc: linux-scsi, mpt_linux_developer

On Friday, September 22, 2006 8:58 AM, Martin Wilck wrote: 

> 
> From: Martin Wilck <Martin.Wilck@fujitsu-siemens.com>
> 
>     [SCSI] mptsas: add RAID devices in ascending ID order
>     
>     This patch fixes the order in which mptsas registers
>     RAID devices with the SCSI mid layer. The VolumeID field
>     is not in ascending order. This leads to situations where
>     e.g. device 2:8:2:0 is registered before 2:8:0:0.
>     
>     That may lead to an unbootable system e.g. if a user installs
>     on a two-disk RAID1 array and adds another 2-disk RAID1 later
>     (the new array will be detected as sda, the old one as sdb).
>     
>     The patch makes sure that arrays are added in ascending
>     VolumeID order.
> 

Rejected.  

Ascending ID ordering does not address all cases. 
There is no guarantee which target ids are 
assigned by firmware to volumes when
volumes are created.   I've made similar 
ordering proposals before to this mailing list. 
All been rejected.

I suggest that you not map your devices
to /dev/sda, but instead use persistent
device labels or names.  Most distro's
now days ship with Logical Volume Manager
that does this for you.

Here is a link on udev:

http://en.wikipedia.org/wiki/Udev


Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] scsi/mptsas: add RAID devices in ascending ID order
@ 2006-09-22 14:57 martin.wilck
  0 siblings, 0 replies; 6+ messages in thread
From: martin.wilck @ 2006-09-22 14:57 UTC (permalink / raw)
  To: Eric.Moore; +Cc: martin.wilck, linux-scsi, mpt_linux_developer

From: Martin Wilck <Martin.Wilck@fujitsu-siemens.com>

    [SCSI] mptsas: add RAID devices in ascending ID order
    
    This patch fixes the order in which mptsas registers
    RAID devices with the SCSI mid layer. The VolumeID field
    is not in ascending order. This leads to situations where
    e.g. device 2:8:2:0 is registered before 2:8:0:0.
    
    That may lead to an unbootable system e.g. if a user installs
    on a two-disk RAID1 array and adds another 2-disk RAID1 later
    (the new array will be detected as sda, the old one as sdb).
    
    The patch makes sure that arrays are added in ascending
    VolumeID order.

Signed-off-by: Martin Wilck <Martin.Wilck@fujitsu-siemens.com>

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index b752a47..3f01f03 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1962,6 +1962,7 @@ mptsas_scan_sas_topology(MPT_ADAPTER *io
 {
 	u32 handle = 0xFFFF;
 	int i;
+	int last_added_id;
 
 	mutex_lock(&ioc->sas_discovery_mutex);
 	mptsas_probe_hba_phys(ioc);
@@ -1974,9 +1975,24 @@ mptsas_scan_sas_topology(MPT_ADAPTER *io
 		goto out;
 	if (!ioc->raid_data.pIocPg2->NumActiveVolumes)
 		goto out;
-	for (i=0; i<ioc->raid_data.pIocPg2->NumActiveVolumes; i++) {
+
+	/* Add logical volumes in the usual SCSI scan sequence (ascending VolumeID) */
+	for (last_added_id = -1;;) {
+		int idx = -1;
+		int smallest_id = 256; /* VolumeID is u8 */
+
+		for (i=0; i<ioc->raid_data.pIocPg2->NumActiveVolumes; i++) {
+			int id = ioc->raid_data.pIocPg2->RaidVolume[i].VolumeID;
+			if (id > last_added_id && id < smallest_id) {
+				smallest_id = id;
+				idx = i;
+			}
+		}
+		if (idx == -1)
+			break;
+		last_added_id = smallest_id;
 		scsi_add_device(ioc->sh, MPTSAS_RAID_CHANNEL,
-		    ioc->raid_data.pIocPg2->RaidVolume[i].VolumeID, 0);
+		    ioc->raid_data.pIocPg2->RaidVolume[idx].VolumeID, 0);
 	}
  out:
 	mutex_unlock(&ioc->sas_discovery_mutex);

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-09-26 18:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-25 16:14 [PATCH] scsi/mptsas: add RAID devices in ascending ID order Moore, Eric
2006-09-26 12:12 ` Martin Wilck
  -- strict thread matches above, loose matches on Subject: below --
2006-09-26 18:33 Moore, Eric
2006-09-22 18:17 Moore, Eric
2006-09-25  9:59 ` Martin Wilck
2006-09-22 14:57 martin.wilck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox