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

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

[Kashyap, 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.

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 <kashyap.desai@lsi.com>
---
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 57bcd5c..be160e0 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -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",
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
index 0ebef0c..3c1b088 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.h
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -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;
diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index 6273abd..0e82863 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -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);
 }
 
 /**

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

[James Bottomley]

On Wed, 2010-08-04 at 14:04 +0530, Kashyap, Desai wrote:
> 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

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

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

[Moore, Eric]

On Sunday, September 05, 2010 10:35 AM, James Bottomley wrote:
> On Wed, 2010-08-04 at 14:04 +0530, Kashyap, Desai wrote:
> > 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
> 
> 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

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

[James Bottomley]

On Thu, 2010-09-09 at 16:43 -0600, Moore, Eric wrote:
> On Sunday, September 05, 2010 10:35 AM, James Bottomley wrote:
> > On Wed, 2010-08-04 at 14:04 +0530, Kashyap, Desai wrote:
> > > 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
> > 
> > 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.

>   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.

>   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

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

[Moore, Eric]

On Saturday, September 11, 2010 8:03 AM, James Bottomley wrote:
> >
> > 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.

> 
> >   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.


> 
> >   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.
> 

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

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

[Desai, Kashyap]

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

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


> -----Original Message-----
> From: Moore, Eric
> Sent: Tuesday, September 14, 2010 6:15 AM
> To: James Bottomley
> Cc: Desai, Kashyap; linux-scsi@vger.kernel.org; Prakash, Sathya
> Subject: RE: [PATCH 1/3] mpt2sas: Wait for port enable to get complete
> before reporting devices to OS.
> 
> On Saturday, September 11, 2010 8:03 AM, James Bottomley wrote:
> > >
> > > 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.
> 
> >
> > >   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.
> 
> 
> >
> > >   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.
> >
> 
> Yes if BIOS boots to the correct drive containing the OS, then I agree
> that device labels, ids, path, and uuid  all work.


[-- Attachment #2: Type: message/rfc822, Size: 7596 bytes --]

From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: "Desai, Kashyap" <Kashyap.Desai@lsi.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>, "Moore, Eric" <Eric.Moore@lsi.com>, "Prakash, Sathya" <Sathya.Prakash@lsi.com>
Subject: Re: [PATCH 2/3] mpt2sas: Adding enclosure/slot sorting logic
Date: Sun, 5 Sep 2010 22:06:29 +0530
Message-ID: <1283704589.15944.1273.camel@mulgrave.site>

On Wed, 2010-08-04 at 14:05 +0530, Kashyap, Desai wrote:
> 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.
> 
> (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