mpt2sas: Wait for port enable to get complete before reporting devices to OS.

PATCH In review Kashyap, Desai 3 participants 6 messages Aug 4, 2010 – Oct 12, 2010 SCSI

Share & tools

Kashyap, Desai

author

2010-08-04 09:11 UTC #0 lore raw

PATCH 1/3 mpt2sas: Wait for port enable to get complete before reporting devices to OS.

Add support in driver so it will wait for discovery to complete before returning from the send_port_enable routine. There are some cases where firmware is doing discovery after port enable completes. When it does this, the driver will not have all the devices in the sas_device_init_list list prior to calling the sort routine for reporting boot devices to OS.

This patch is required so the driver is waiting for all the events to be processed in the hot plug worker thread prior to reporting them to the OS.

(1) The driver will wait for port enable to complete
(2) Wait for the first discovery stop event
(3) Wait an additional 500ms in case there are additional discoveries
(4) process device list -> reporting devcies to OS

Signed-off-by Kashyap Desai <[email protected]>

---

drivers/scsi/mpt2sas/mpt2sas_base.c +9 -0

index 57bcd5c..be160e0 100644
@@ -3113,6 +3113,9 @@ _base_send_port_enable(struct MPT2SAS_ADAPTER *ioc, int sleep_flag)
 	mpi_request->VF_ID = 0; /* TODO */
 	mpi_request->VP_ID = 0;
 
+	if (ioc->wait_for_port_enable_to_complete)
+		init_completion(&ioc->port_enable_done);
+
 	mpt2sas_base_put_smid_default(ioc, smid);
 	init_completion(&ioc->base_cmds.done);
 	timeleft = wait_for_completion_timeout(&ioc->base_cmds.done,

@@ -3138,6 +3141,12 @@ _base_send_port_enable(struct MPT2SAS_ADAPTER *ioc, int sleep_flag)
 		    " (ioc_state=0x%x)\n", ioc->name, __func__, ioc_state);
 		r = -EFAULT;
 	}
+
+	/* this is waiting for the discovery to complete */
+	if (ioc->wait_for_port_enable_to_complete && ioc_state == 0) {
+		wait_for_completion(&ioc->port_enable_done);
+		msleep(500); /* in case there are additional discoveries */
+	}
  out:
 	ioc->base_cmds.status = MPT2_CMD_NOT_USED;
 	printk(MPT2SAS_INFO_FMT "port enable: %s\n",

drivers/scsi/mpt2sas/mpt2sas_base.h +1 -0

index 0ebef0c..3c1b088 100644
@@ -620,6 +620,7 @@ struct MPT2SAS_ADAPTER {
 	u8		remove_host;
 	u8		pci_error_recovery;
 	u8		wait_for_port_enable_to_complete;
+	struct completion	port_enable_done;
 
 	u8		msix_enable;
 	u16		msix_vector_count;

drivers/scsi/mpt2sas/mpt2sas_scsih.c +4 -0

index 6273abd..0e82863 100644
@@ -5125,6 +5125,10 @@ _scsih_sas_discovery_event(struct MPT2SAS_ADAPTER *ioc,
 	if (event_data->ReasonCode == MPI2_EVENT_SAS_DISC_RC_STARTED &&
 	    !ioc->sas_hba.num_phys)
 		_scsih_sas_host_add(ioc);
+
+	if (ioc->wait_for_port_enable_to_complete &&
+	    event_data->ReasonCode == MPI2_EVENT_SAS_DISC_RC_COMPLETED)
+		complete(&ioc->port_enable_done);
 }
 
 /**

James Bottomley

maintainer

2010-09-05 16:35 UTC #1 lore raw

Quoted 7 lines · Desai

Add support in driver so it will wait for discovery to complete before returning from the send_port_enable routine. There are some cases where firmware is doing discovery after port enable completes. When it does this, the driver will not have all the devices in the sas_device_init_list list prior to calling the sort routine for reporting boot devices to OS.

Quoted 8 lines

This patch is required so the driver is waiting for all the events to be processed in the hot plug worker thread prior to reporting them to the OS.
(1) The driver will wait for port enable to complete
(2) Wait for the first discovery stop event
(3) Wait an additional 500ms in case there are additional discoveries
(4) process device list -> reporting devcies to OS

Why is there any need to wait and sort at all? If you just report the devices as hotplug sees them, boot will be much faster (especially for root devices).

James

Moore, Eric

2010-09-09 23:08 UTC #2 lore raw

Quoted 14 lines · James Bottomley

Quoted 5 lines · Desai
Add support in driver so it will wait for discovery to complete before returning from the send_port_enable routine. There are some cases where firmware is doing discovery after port enable completes. When it does this, the driver will not have all the devices in the sas_device_init_list list prior to calling the sort routine for
reporting
Quoted 4 lines
boot devices to OS.
This patch is required so the driver is waiting for all the events to be processed in the hot plug worker thread prior to reporting them to
the
Quoted 2 lines
OS.

Quoted 9 lines

Quoted 4 lines
(1) The driver will wait for port enable to complete
(2) Wait for the first discovery stop event
(3) Wait an additional 500ms in case there are additional discoveries
(4) process device list -> reporting devcies to OS
Why is there any need to wait and sort at all? If you just report the devices as hotplug sees them, boot will be much faster (especially for root devices).

IMO, this patch is really needed. Without it you probably will not be able to boot after a fresh installation of OS when there are multiple drives. The issue is Red Hat or SuSE is going to place the new OS on the first reported drive, e.g. /dev/sda. This boot drive information is passed from the BIOS to the driver via a configuration page called bios page 2. The driver needs to wait for all firmware to complete discovery, then search for boot device as indicated in the config page, then report it first. Without the patch, there is no guarantee which drive or raid volume is getting reported first. In other words the device driver could of reported bios drive located at 0x83 as /dev/sda, instead of 0x80, then when you reboot, the BIOS is not finding boot partition. The end user can select the boot device by going into the BIOS configuration utility; e.g. control C.

Eric

James Bottomley

maintainer

2010-09-11 14:03 UTC #3 lore raw

Quoted 20 lines · Eric

Quoted 19 lines · James Bottomley
Quoted 5 lines · Desai
Add support in driver so it will wait for discovery to complete before returning from the send_port_enable routine. There are some cases where firmware is doing discovery after port enable completes. When it does this, the driver will not have all the devices in the sas_device_init_list list prior to calling the sort routine for
reporting
Quoted 4 lines
boot devices to OS.
This patch is required so the driver is waiting for all the events to be processed in the hot plug worker thread prior to reporting them to
the
Quoted 6 lines
OS.
(1) The driver will wait for port enable to complete
(2) Wait for the first discovery stop event
(3) Wait an additional 500ms in case there are additional discoveries
(4) process device list -> reporting devcies to OS

Quoted 8 lines

Quoted 4 lines
Why is there any need to wait and sort at all? If you just report the devices as hotplug sees them, boot will be much faster (especially for root devices).
IMO, this patch is really needed. Without it you probably will not be able to boot after a fresh installation of OS when there are multiple drives.

But that's an installer problem, not a kernel one.

Quoted 4 lines

The issue is Red Hat or SuSE is going to place the new OS on the
first reported drive, e.g. /dev/sda. This boot drive information is passed from the BIOS to the driver via a configuration page called bios page 2.

I think both installers already take this into account: grub finds it when it builds the mapping table for the first time.

Quoted 8 lines

The driver needs to wait for all firmware to complete discovery,
then search for boot device as indicated in the config page, then report it first. Without the patch, there is no guarantee which drive or raid volume is getting reported first. In other words the device driver could of reported bios drive located at 0x83 as /dev/sda, instead of 0x80, then when you reboot, the BIOS is not finding boot partition. The end user can select the boot device by going into the BIOS configuration utility; e.g. control C.

But you don't need it to be reported first. The default install on both SLES and RHEL (and Fedora and openSUSE) is by label. So once we get the boot drive from the mapping, we build its label into the grub config and fstab files and we always run from it, regardless of the order it appears in the bootup.

James

Moore, Eric

2010-09-14 00:45 UTC #4 lore raw

Quoted 6 lines · James Bottomley

Quoted 4 lines
IMO, this patch is really needed. Without it you probably will not be able to boot after a fresh installation of OS when there are multiple drives.
But that's an installer problem, not a kernel one.

Probably I'm not explaining it clearly. I pretty sure this is a device driver issue.

The installation needs to occur on the same drive which BIOS is planning to booting to.

Installer typically use the first reported drive to put the OS on. Since LSI controller firmware does discovery, there is no guarantee which hard drive is going to be reported first. What I'm trying to say is discovery is random, and drives can be reported in any order. If you have 100 hard drives in an enclosure, then the first reported drive can be any one of the 100 drives. If you made a mistake an didn't get put OS onto the same drive BIOS is booting to, then your screwed the next time you boot up.

The boot drive info is passed from BIOS to driver via the bios configuration page I mentioned in the previous email. The device driver makes sure the first hard drive reported is going to the drive your booting to. Thus you need to wait for discovery to complete in order to find the correct drive, which is what this patch is all about.

Quoted 8 lines

Quoted 5 lines
The issue is Red Hat or SuSE is going to place the new OS on the
first reported drive, e.g. /dev/sda. This boot drive information is passed from the BIOS to the driver via a configuration page called bios page 2.
I think both installers already take this into account: grub finds it when it builds the mapping table for the first time.

Yes if BIOS boots to the correct drive containing the OS and grub, then I agree with everything your saying.

Quoted 11 lines

Quoted 4 lines
The driver needs to wait for all firmware to complete discovery,
then search for boot device as indicated in the config page, then report it first. Without the patch, there is no guarantee which
drive
Quoted 5 lines
or raid volume is getting reported first. In other words the device driver could of reported bios drive located at 0x83 as /dev/sda, instead of 0x80, then when you reboot, the BIOS is not finding boot partition. The end user can select the boot device by going into the BIOS configuration utility; e.g. control C.

Quoted 9 lines

But you don't need it to be reported first. The default install on both
SLES and RHEL (and Fedora and openSUSE) is by label. So once we get the
boot drive from the mapping, we build its label into the grub config and
fstab files and we always run from it, regardless of the order it appears in the bootup.

Yes if BIOS boots to the correct drive containing the OS, then I agree that device labels, ids, path, and uuid all work.

Desai, Kashyap

author

2010-10-12 15:15 UTC #5 lore raw

James,

We had internal discussion again after your input. Here is summary of discussion and we are agreed not to push this patchset for upstream. Please reject this submission. I have provided summary just for information point of view.

Patch1: Wait for discovery to complete.

"This patch is only required when fresh installation is happening. Next reboot onwards that part of code does not play any role" That is what a concern of James. James suggested do not wait for whole topology to discover. Let's start boot ASAP possible once grub is detected by BIOS.

Issue solve by this patch: While installation user cannot identify end-device on which they want to install OS.

Pros of this patch :
It solves installation problem where we have huge topology attached to the system.
It will report /dev/sda which is marked as "preferred boot device in BIOS page8". This way user can always make sure /dev/sda is a device where they want to install OS.

Cons of this patch :
After Installation, code provided in this patch is not required. There is a no need to always provide /dev/sda scsi device name to preferred boot device.
Default RHEL and SLES installation is based on LABEL. (This is part of udev). Using this patch driver will wait for discovery to be completed by firmware later it will report prefer boot device first to the SML.

Workaround:
Without this patch customer can still solve real-time installation issue. They can opt for reduced topology when going for OS installation. e.a if they are 100 drivers in topology, they can disconnect 99 drivers for time being and only connect 1 driver on which they want to install OS. Once OS installation is done, connect those 99 devices back to topology.

Patch 2: Actual enumeration logic.

This patch is again an agreement of same /dev/sdX for preferred boot device, which is already known to upstream and only solution, is udev. LSI agreed with James comment. Attached is james last comment for reference.

Thanks, Kashyap

Quoted 80 lines

-----Original Message-----
From: Moore, Eric
Sent: Tuesday, September 14, 2010 6:15 AM
To: James Bottomley
Cc: Desai, Kashyap; [email protected]; Prakash, Sathya
Subject: RE: [PATCH 1/3] mpt2sas: Wait for port enable to get complete before reporting devices to OS.
Quoted 2 lines · James Bottomley
Quoted 2 lines
IMO, this patch is really needed. Without it you probably will not
be
Quoted 1 line
Quoted 1 line
able to boot after a fresh installation of OS when there are
multiple
Quoted 3 lines
Quoted 1 line
drives.
But that's an installer problem, not a kernel one.
Probably I'm not explaining it clearly. I pretty sure this is a device driver issue.
The installation needs to occur on the same drive which BIOS is planning to booting to.
Installer typically use the first reported drive to put the OS on. Since LSI controller firmware does discovery, there is no guarantee which hard drive is going to be reported first. What I'm trying to say
is discovery is random, and drives can be reported in any order. If
you have 100 hard drives in an enclosure, then the first reported drive can be any one of the 100 drives. If you made a mistake an didn't get put OS onto the same drive BIOS is booting to, then your screwed the next time you boot up.
The boot drive info is passed from BIOS to driver via the bios configuration page I mentioned in the previous email. The device driver makes sure the first hard drive reported is going to the drive your booting to. Thus you need to wait for discovery to complete in order to find the correct drive, which is what this patch is all about.
Quoted 3 lines
Quoted 3 lines
The issue is Red Hat or SuSE is going to place the new OS on the
first reported drive, e.g. /dev/sda. This boot drive information
is
Quoted 5 lines
Quoted 2 lines
passed from the BIOS to the driver via a configuration page called bios page 2.
I think both installers already take this into account: grub finds it when it builds the mapping table for the first time.
Yes if BIOS boots to the correct drive containing the OS and grub, then I agree with everything your saying.
Quoted 6 lines
Quoted 4 lines
The driver needs to wait for all firmware to complete discovery,
then search for boot device as indicated in the config page, then report it first. Without the patch, there is no guarantee which
drive
Quoted 1 line
or raid volume is getting reported first. In other words the
device
Quoted 3 lines
Quoted 3 lines
driver could of reported bios drive located at 0x83 as /dev/sda, instead of 0x80, then when you reboot, the BIOS is not finding boot partition. The end user can select the boot device by going into
the
Quoted 11 lines
Quoted 1 line
BIOS configuration utility; e.g. control C.
But you don't need it to be reported first. The default install on both
SLES and RHEL (and Fedora and openSUSE) is by label. So once we get the
boot drive from the mapping, we build its label into the grub config and
fstab files and we always run from it, regardless of the order it appears in the bootup.
Yes if BIOS boots to the correct drive containing the OS, then I agree that device labels, ids, path, and uuid all work.

Quoted 64 lines · Desai

Adding sorting support to driver so that during driver load time it will sort devices using enclosure/slot mapping algorithm prior to reporting devices to OS. This is only done when ioc page 8 says to sort enclosure/slot mode. The driver will handle sorting by enclosure handle and slot when available. If that is not available, then it will sort by parent device handle and phy number. For persistent device mapping mode the driver will not sort.
This patch is for sorting devices. The customer desire is for devices to be reported to OS to be consistent manner across reboots and across multiple systems with similar topology. Current design in the driver is to report devices in the order of firmware discovery, which can change due to variations in device spin up time. The suggested change in the driver is to prepare a sorted list prior to reporting devices to the OS. This algorithm will be based on Enclosure/Slot order algorithm.
For this algorithm to be implemented in the controller firmware, the firmware needs memory space to account for several hundreds of devices that could potentially be supported. This specific version of the controller hardware does not have the necessary memory to implement this logic. Hence the logic is implemented in the driver.
Here is overview of this patch:
(a) Check IOC Page 8. If the MPI2_IOCPAGE8_FLAGS_ENCLOSURE_SLOT_MAPPING bit is not set in the flags field, then don't sort devices. When the flag is cleared, then devices are added to list in the order of controller discovery.
(b) If the Enclosure/Slot mapping bit is set, then proceed to sort devices as follows.
This is the Enclosure/SlotID mapping algorithm:
(1)If the list is empty, add the first device.
(2)If the new device has a Enclosure Handle set to zero, then proceed to the Expander /Phy Number mapping algorithm (see next section).
(3)Traverse through the list. If the Enclosure Handle is zero, loop to the next device. If we made it thru the entire list and didn't insert the new device and then proceed to the Expander /Phy Number mapping algorithm. (see next section)
(4)If Enclosure Handle less than the searched Enclosure Handle, then insert new device before the searched device.
(5)If the Enclosure Handle is the same as the searched device Enclosure Handle, then check the Slot ID. If the Slot ID is smaller than the searched Slot ID, then insert new device before the searched device.
(6)If both Enclosure Handle and Slot ID are the same as searched devices, then check Phy Number. If the PhyNumber is smaller than the searched device PhyNumber, then insert new device before the searched device.
(7)If we hit the last entry in the list, then append new device to the tail.
This is the Expander /Phy Number mapping algorithm:
(1)Traverse through the list from the start. If the Parent Handle is less than the search Parent Handle, then insert new device before the searched device. Please note that the Parent Handle is parent of the attached device, it would be either an host controller or expander.

Quoted 6 lines

(2)If the Parent Handle is the same as the searched device Parent Handle, then check PhyNumber. If the PhyNumber is less than the searched PhyNumber, then insert the new device before the searched device.
(3)If we made it through the entire list without adding the new device, then append new device to tail of the list.

This really doesn't look like a good idea. If you want slot mapping why not just add a udev rule to get it (or use ses)? Trying to wait and sort in the driver is fragile and also a bit pointless.

James