Re: Linux enclosure services, hot swap issues

Re: Linux enclosure services, hot swap issues

[James Bottomley]

cc to linux-scsi added

On Thu, 2009-07-30 at 12:30 -0700, Chris Ptacek wrote:
> Hello,
> We are attempting to use the enclosure services (ses.c and enclosure.c) 
> with Xyratex shelves (note we may have the same/similar issues with the 
> IBM enclosure shelves) and have been running tests performing hot 
> swapping of drives and seeing issues.  There appear to be two similar 
> issues.
> 
> 1. When we pull a drive the drive information in the enclosure (slot, 
> device link, etc) is not cleaned up and released.  It appears that 
> ses_intf_remove() is being called however as the device is not an 
> enclosure it just returns and does nothing.  This leaves a stale device 
> link and other information within the sysfs information for that 
> enclosure slot.
> 
> 2. When we re-add a drive to the system the drive gets assigned a new 
> port and number.  At the moment we are unsure if this may be caused by 
> refcounts on the old drive never being fully decremented.  However as 
> the drive has a new port name the stale link in the sysfs enclosure slot 
> is no longer pointing to the drive. 
> It also appears that when adding the drive the ses_intf_add() function 
> checks to see if the device is in an enclosure by examining the parent.  
> However this appears to always fail.  On boot when the actual enclosure 
> is added it manages to walk all the drives and add them, however on some 
> systems it appears that the boot ordering may cause only some subset of 
> drives to appear.
> 
> Before issue, the device in slot 15 of enclosure looks as follows
> /sys/block/sde/device/enclosure_device:15/device ->
> ../../../../devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/port-2:0:2/end_device-2:0:2/target2:0:2/2:0:2:0
> 
> NOTE: under the expander-2:0 it shows as "port-2:0:2"
> If we look at this directory it shows following...
> 
> -bash-3.2# ls 
> /sys/devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/
> phy-2:0:10 phy-2:0:16 phy-2:0:22 phy-2:0:28 phy-2:0:34 phy-2:0:40 
> phy-2:0:9 port-2:0:13 port-2:0:19 port-2:0:24 port-2:0:7 uevent
> phy-2:0:11 phy-2:0:17 phy-2:0:23 phy-2:0:29 phy-2:0:35 phy-2:0:41 
> port-2:0:0 port-2:0:14 port-2:0:2 port-2:0:25 port-2:0:8
> phy-2:0:12 phy-2:0:18 phy-2:0:24 phy-2:0:30 phy-2:0:36 phy-2:0:42 
> port-2:0:1 port-2:0:15 port-2:0:20 port-2:0:3 port-2:0:9
> phy-2:0:13 phy-2:0:19 phy-2:0:25 phy-2:0:31 phy-2:0:37 phy-2:0:43 
> port-2:0:10 port-2:0:16 port-2:0:21 port-2:0:4 power
> phy-2:0:14 phy-2:0:20 phy-2:0:26 phy-2:0:32 phy-2:0:38 phy-2:0:44 
> port-2:0:11 port-2:0:17 port-2:0:22 port-2:0:5 sas_device:expander-2:0
> phy-2:0:15 phy-2:0:21 phy-2:0:27 phy-2:0:33 phy-2:0:39 phy-2:0:8 
> port-2:0:12 port-2:0:18 port-2:0:23 port-2:0:6 sas_expander:expander-2:0
> 
> === REMOVE AND INSERT DRIVE =====
> 
> However, if we then remove the drive and insert it again the above 
> relationship breaks down. The link that we follow above is stale and 
> still points at "port-2:0:2".
> /sys/block/sde/device/enclosure_device:15/device ->
> ../../../../devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/port-2:0:2/end_device-2:0:2/target2:0:2/2:0:2:0
> 
> Yet, if we look at that expander directory we find that this port no 
> longer exists and a new one was added now as "port-2:0:26".
> 
> -bash-3.2# ls 
> /sys/devices/pci0000\:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/
> phy-2:0:10 phy-2:0:16 phy-2:0:22 phy-2:0:28 phy-2:0:34 phy-2:0:40 
> phy-2:0:9 port-2:0:13 port-2:0:19 port-2:0:25 port-2:0:7 uevent
> phy-2:0:11 phy-2:0:17 phy-2:0:23 phy-2:0:29 phy-2:0:35 phy-2:0:41 
> port-2:0:0 port-2:0:14 port-2:0:20 port-2:0:26 port-2:0:8
> phy-2:0:12 phy-2:0:18 phy-2:0:24 phy-2:0:30 phy-2:0:36 phy-2:0:42 
> port-2:0:1 port-2:0:15 port-2:0:21 port-2:0:3 port-2:0:9
> phy-2:0:13 phy-2:0:19 phy-2:0:25 phy-2:0:31 phy-2:0:37 phy-2:0:43 
> port-2:0:10 port-2:0:16 port-2:0:22 port-2:0:4 power
> phy-2:0:14 phy-2:0:20 phy-2:0:26 phy-2:0:32 phy-2:0:38 phy-2:0:44 
> port-2:0:11 port-2:0:17 port-2:0:23 port-2:0:5 sas_device:expander-2:0
> phy-2:0:15 phy-2:0:21 phy-2:0:27 phy-2:0:33 phy-2:0:39 phy-2:0:8 
> port-2:0:12 port-2:0:18 port-2:0:24 port-2:0:6 sas_expander:expander-2:0
> 
> 
> When adding the drive we are printing out the names and the parents. 
> 
> Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] 976773168 512-byte 
> hardware sectors: (500 GB/465 GiB)
> Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] Write Protect is off
> Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] Write cache: 
> disabled, read cache: enabled, supports DPO and FUA
> Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: Attached scsi generic sg33 
> type 0
> ## In ses_intf_add we are printing the name of the device passed in:
> ##  printk("%s : %s\n", __func__, dev_name(cdev));
> Jul 30 11:29:53 sweng72 kernel: ses_intf_add : 2:0:51:0
> Jul 30 11:29:53 sweng72 kernel: device: 'sdad': device_add
> ## In enclosure_add we are printing the name of the host passed in and 
> the parentage:
> ##   printk("%s : %s (%p)\n", __func__, dev_name(dev), dev);
> ## Then per enclosure
> ##   printk("%s : edev %s parent %s \n", __func__, 
> dev_name(&edev->edev), dev_name(edev->edev.parent));
> ##   pdev = edev->edev.parent;
> ##   while(pdev != NULL)
> ##   {
> ##        printk("%s :         parent %s (%p)\n", __func__, 
> dev_name(pdev), pdev);
> ##        pdev = pdev->parent;
> ##    }
> Jul 30 11:29:53 sweng72 kernel: enclosure_find : host2 (ffff8804cb804178)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 0:3:0:0 parent 0:3:0:0
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 0:3:0:0 
> (ffff8804c9d63928)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> target0:3:0 (ffff8804c9d62828)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host0 
> (ffff8804ca3d6978)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:04:00.0 (ffff8804cb867880)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:00:03.0 (ffff8804cb802880)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> pci0000:00 (ffff8804cb800e00)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 2:0:24:0 parent 
> 2:0:24:0
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 2:0:24:0 
> (ffff8804c98f5128)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> target2:0:24 (ffff8804c9916428)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> end_device-2:0:25 (ffff8804c9914000)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> port-2:0:25 (ffff8804c9914800)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> expander-2:0 (ffff8804c9c0b838)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent port-2:0 
> (ffff8804c9c0d400)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host2 
> (ffff8804cb804178)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:07:00.0 (ffff8804cb86d880)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:00:06.0 (ffff8804cb803080)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> pci0000:00 (ffff8804cb800e00)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 2:0:49:0 parent 
> 2:0:49:0
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 2:0:49:0 
> (ffff8804c9a85928)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> target2:0:49 (ffff8804c9a82828)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> end_device-2:1:25 (ffff8804c9a81400)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> port-2:1:25 (ffff8804c9a81c00)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> expander-2:1 (ffff8804c9d11838)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent port-2:1 
> (ffff8804ca1d3400)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host2 
> (ffff8804cb804178)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:07:00.0 (ffff8804cb86d880)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> 0000:00:06.0 (ffff8804cb803080)
> Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> pci0000:00 (ffff8804cb800e00)
> 
> Note these enclosures are double cabled, we have tried without it with 
> the same results. 
> If we examine the parentage of the enclosures the host2 entry is way 
> down the list, not the direct parent of the device passed in.  This 
> causes no enclosure to be found and no links, etc are handled for the 
> drive that was added.
> 
> We were wondering if you may have any input on these issues and their 
> expected operation?

The problems are basically because ses has no hotplug code (it doesn't
expect the configuration to change).  It shouldn't be too hard to add
via the SCSI interface function, though; I'll take a look.

James

Re: Linux enclosure services, hot swap issues

[James Bottomley]

On Thu, 2009-07-30 at 20:05 +0000, James Bottomley wrote:
> cc to linux-scsi added
> 
> On Thu, 2009-07-30 at 12:30 -0700, Chris Ptacek wrote:
> > Hello,
> > We are attempting to use the enclosure services (ses.c and enclosure.c) 
> > with Xyratex shelves (note we may have the same/similar issues with the 
> > IBM enclosure shelves) and have been running tests performing hot 
> > swapping of drives and seeing issues.  There appear to be two similar 
> > issues.
> > 
> > 1. When we pull a drive the drive information in the enclosure (slot, 
> > device link, etc) is not cleaned up and released.  It appears that 
> > ses_intf_remove() is being called however as the device is not an 
> > enclosure it just returns and does nothing.  This leaves a stale device 
> > link and other information within the sysfs information for that 
> > enclosure slot.
> > 
> > 2. When we re-add a drive to the system the drive gets assigned a new 
> > port and number.  At the moment we are unsure if this may be caused by 
> > refcounts on the old drive never being fully decremented.  However as 
> > the drive has a new port name the stale link in the sysfs enclosure slot 
> > is no longer pointing to the drive. 
> > It also appears that when adding the drive the ses_intf_add() function 
> > checks to see if the device is in an enclosure by examining the parent.  
> > However this appears to always fail.  On boot when the actual enclosure 
> > is added it manages to walk all the drives and add them, however on some 
> > systems it appears that the boot ordering may cause only some subset of 
> > drives to appear.
> > 
> > Before issue, the device in slot 15 of enclosure looks as follows
> > /sys/block/sde/device/enclosure_device:15/device ->
> > ../../../../devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/port-2:0:2/end_device-2:0:2/target2:0:2/2:0:2:0
> > 
> > NOTE: under the expander-2:0 it shows as "port-2:0:2"
> > If we look at this directory it shows following...
> > 
> > -bash-3.2# ls 
> > /sys/devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/
> > phy-2:0:10 phy-2:0:16 phy-2:0:22 phy-2:0:28 phy-2:0:34 phy-2:0:40 
> > phy-2:0:9 port-2:0:13 port-2:0:19 port-2:0:24 port-2:0:7 uevent
> > phy-2:0:11 phy-2:0:17 phy-2:0:23 phy-2:0:29 phy-2:0:35 phy-2:0:41 
> > port-2:0:0 port-2:0:14 port-2:0:2 port-2:0:25 port-2:0:8
> > phy-2:0:12 phy-2:0:18 phy-2:0:24 phy-2:0:30 phy-2:0:36 phy-2:0:42 
> > port-2:0:1 port-2:0:15 port-2:0:20 port-2:0:3 port-2:0:9
> > phy-2:0:13 phy-2:0:19 phy-2:0:25 phy-2:0:31 phy-2:0:37 phy-2:0:43 
> > port-2:0:10 port-2:0:16 port-2:0:21 port-2:0:4 power
> > phy-2:0:14 phy-2:0:20 phy-2:0:26 phy-2:0:32 phy-2:0:38 phy-2:0:44 
> > port-2:0:11 port-2:0:17 port-2:0:22 port-2:0:5 sas_device:expander-2:0
> > phy-2:0:15 phy-2:0:21 phy-2:0:27 phy-2:0:33 phy-2:0:39 phy-2:0:8 
> > port-2:0:12 port-2:0:18 port-2:0:23 port-2:0:6 sas_expander:expander-2:0
> > 
> > === REMOVE AND INSERT DRIVE =====
> > 
> > However, if we then remove the drive and insert it again the above 
> > relationship breaks down. The link that we follow above is stale and 
> > still points at "port-2:0:2".
> > /sys/block/sde/device/enclosure_device:15/device ->
> > ../../../../devices/pci0000:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/port-2:0:2/end_device-2:0:2/target2:0:2/2:0:2:0
> > 
> > Yet, if we look at that expander directory we find that this port no 
> > longer exists and a new one was added now as "port-2:0:26".
> > 
> > -bash-3.2# ls 
> > /sys/devices/pci0000\:00/0000:00:06.0/0000:07:00.0/host2/port-2:0/expander-2:0/
> > phy-2:0:10 phy-2:0:16 phy-2:0:22 phy-2:0:28 phy-2:0:34 phy-2:0:40 
> > phy-2:0:9 port-2:0:13 port-2:0:19 port-2:0:25 port-2:0:7 uevent
> > phy-2:0:11 phy-2:0:17 phy-2:0:23 phy-2:0:29 phy-2:0:35 phy-2:0:41 
> > port-2:0:0 port-2:0:14 port-2:0:20 port-2:0:26 port-2:0:8
> > phy-2:0:12 phy-2:0:18 phy-2:0:24 phy-2:0:30 phy-2:0:36 phy-2:0:42 
> > port-2:0:1 port-2:0:15 port-2:0:21 port-2:0:3 port-2:0:9
> > phy-2:0:13 phy-2:0:19 phy-2:0:25 phy-2:0:31 phy-2:0:37 phy-2:0:43 
> > port-2:0:10 port-2:0:16 port-2:0:22 port-2:0:4 power
> > phy-2:0:14 phy-2:0:20 phy-2:0:26 phy-2:0:32 phy-2:0:38 phy-2:0:44 
> > port-2:0:11 port-2:0:17 port-2:0:23 port-2:0:5 sas_device:expander-2:0
> > phy-2:0:15 phy-2:0:21 phy-2:0:27 phy-2:0:33 phy-2:0:39 phy-2:0:8 
> > port-2:0:12 port-2:0:18 port-2:0:24 port-2:0:6 sas_expander:expander-2:0
> > 
> > 
> > When adding the drive we are printing out the names and the parents. 
> > 
> > Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] 976773168 512-byte 
> > hardware sectors: (500 GB/465 GiB)
> > Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] Write Protect is off
> > Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: [sdad] Write cache: 
> > disabled, read cache: enabled, supports DPO and FUA
> > Jul 30 11:29:53 sweng72 kernel: sd 2:0:51:0: Attached scsi generic sg33 
> > type 0
> > ## In ses_intf_add we are printing the name of the device passed in:
> > ##  printk("%s : %s\n", __func__, dev_name(cdev));
> > Jul 30 11:29:53 sweng72 kernel: ses_intf_add : 2:0:51:0
> > Jul 30 11:29:53 sweng72 kernel: device: 'sdad': device_add
> > ## In enclosure_add we are printing the name of the host passed in and 
> > the parentage:
> > ##   printk("%s : %s (%p)\n", __func__, dev_name(dev), dev);
> > ## Then per enclosure
> > ##   printk("%s : edev %s parent %s \n", __func__, 
> > dev_name(&edev->edev), dev_name(edev->edev.parent));
> > ##   pdev = edev->edev.parent;
> > ##   while(pdev != NULL)
> > ##   {
> > ##        printk("%s :         parent %s (%p)\n", __func__, 
> > dev_name(pdev), pdev);
> > ##        pdev = pdev->parent;
> > ##    }
> > Jul 30 11:29:53 sweng72 kernel: enclosure_find : host2 (ffff8804cb804178)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 0:3:0:0 parent 0:3:0:0
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 0:3:0:0 
> > (ffff8804c9d63928)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > target0:3:0 (ffff8804c9d62828)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host0 
> > (ffff8804ca3d6978)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:04:00.0 (ffff8804cb867880)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:00:03.0 (ffff8804cb802880)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > pci0000:00 (ffff8804cb800e00)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 2:0:24:0 parent 
> > 2:0:24:0
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 2:0:24:0 
> > (ffff8804c98f5128)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > target2:0:24 (ffff8804c9916428)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > end_device-2:0:25 (ffff8804c9914000)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > port-2:0:25 (ffff8804c9914800)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > expander-2:0 (ffff8804c9c0b838)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent port-2:0 
> > (ffff8804c9c0d400)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host2 
> > (ffff8804cb804178)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:07:00.0 (ffff8804cb86d880)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:00:06.0 (ffff8804cb803080)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > pci0000:00 (ffff8804cb800e00)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find : edev 2:0:49:0 parent 
> > 2:0:49:0
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 2:0:49:0 
> > (ffff8804c9a85928)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > target2:0:49 (ffff8804c9a82828)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > end_device-2:1:25 (ffff8804c9a81400)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > port-2:1:25 (ffff8804c9a81c00)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > expander-2:1 (ffff8804c9d11838)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent port-2:1 
> > (ffff8804ca1d3400)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent host2 
> > (ffff8804cb804178)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:07:00.0 (ffff8804cb86d880)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > 0000:00:06.0 (ffff8804cb803080)
> > Jul 30 11:29:54 sweng72 kernel: enclosure_find :         parent 
> > pci0000:00 (ffff8804cb800e00)
> > 
> > Note these enclosures are double cabled, we have tried without it with 
> > the same results. 
> > If we examine the parentage of the enclosures the host2 entry is way 
> > down the list, not the direct parent of the device passed in.  This 
> > causes no enclosure to be found and no links, etc are handled for the 
> > drive that was added.
> > 
> > We were wondering if you may have any input on these issues and their 
> > expected operation?
> 
> The problems are basically because ses has no hotplug code (it doesn't
> expect the configuration to change).  It shouldn't be too hard to add
> via the SCSI interface function, though; I'll take a look.

Actually, there turned out to be three separate issues:

     1. The way we handle enclosures in hot add doesn't contemplate that
        there may be more than one per host
     2. The hot remove for components simply isn't plumbed in
     3. We also need to update the enclosure pages so that we get the
        new mappings

I've got patches for each of these issues; at the end of the series,
hotplug in a dual enclosure device system works pretty well for me.

James

[PATCH 1/3] ses: fix hotplug with multiple devices and expanders

[James Bottomley]

In a situation either with expanders or with multiple enclosure
devices, hot add doesn't always work.  This is because we try to find
a single enclosure device attached to the host.  Fix this by looping
over all enclosure devices attached to the host and also by making the
find loop recognise that the enclosure devices may be expander remote
(i.e. not parented by the host).
---
 drivers/misc/enclosure.c  |   44 ++++++++++++++++++++++++++++++++------------
 drivers/scsi/ses.c        |   10 ++++++----
 include/linux/enclosure.h |    3 ++-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 348443b..789d121 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -33,24 +33,44 @@ static DEFINE_MUTEX(container_list_lock);
 static struct class enclosure_class;
 
 /**
- * enclosure_find - find an enclosure given a device
- * @dev:	the device to find for
+ * enclosure_find - find an enclosure given a parent device
+ * @dev:	the parent to match against
+ * @start:	Optional enclosure device to start from (NULL if none)
  *
- * Looks through the list of registered enclosures to see
- * if it can find a match for a device.  Returns NULL if no
- * enclosure is found. Obtains a reference to the enclosure class
- * device which must be released with device_put().
+ * Looks through the list of registered enclosures to find all those
+ * with @dev as a parent.  Returns NULL if no enclosure is
+ * found. @start can be used as a starting point to obtain multiple
+ * enclosures per parent (should begin with NULL and then be set to
+ * each returned enclosure device). Obtains a reference to the
+ * enclosure class device which must be released with device_put().
+ * If @start is not NULL, a reference must be taken on it which is
+ * released before returning (this allows a loop through all
+ * enclosures to exit with only the reference on the enclosure of
+ * interest held).  Note that the @dev may correspond to the actual
+ * device housing the enclosure, in which case no iteration via @start
+ * is required.
  */
-struct enclosure_device *enclosure_find(struct device *dev)
+struct enclosure_device *enclosure_find(struct device *dev,
+					struct enclosure_device *start)
 {
 	struct enclosure_device *edev;
 
 	mutex_lock(&container_list_lock);
-	list_for_each_entry(edev, &container_list, node) {
-		if (edev->edev.parent == dev) {
-			get_device(&edev->edev);
-			mutex_unlock(&container_list_lock);
-			return edev;
+	edev = list_prepare_entry(start, &container_list, node);
+	if (start)
+		put_device(&start->edev);
+
+	list_for_each_entry_continue(edev, &container_list, node) {
+		struct device *parent = edev->edev.parent;
+		/* parent might not be immediate, so iterate up to
+		 * the root of the tree if necessary */
+		while (parent) {
+			if (parent == dev) {
+				get_device(&edev->edev);
+				mutex_unlock(&container_list_lock);
+				return edev;
+			}
+			parent = parent->parent;
 		}
 	}
 	mutex_unlock(&container_list_lock);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index 4f618f4..e1b8c82 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -413,10 +413,11 @@ static int ses_intf_add(struct device *cdev,
 
 	if (!scsi_device_enclosure(sdev)) {
 		/* not an enclosure, but might be in one */
-		edev = enclosure_find(&sdev->host->shost_gendev);
-		if (edev) {
+		struct enclosure_device *prev = NULL;
+
+		while ((edev = enclosure_find(&sdev->host->shost_gendev, prev)) != NULL) {
 			ses_match_to_enclosure(edev, sdev);
-			put_device(&edev->edev);
+			prev = edev;
 		}
 		return -ENODEV;
 	}
@@ -625,7 +626,8 @@ static void ses_intf_remove(struct device *cdev,
 	if (!scsi_device_enclosure(sdev))
 		return;
 
-	edev = enclosure_find(cdev->parent);
+	/*  exact match to this enclosure */
+	edev = enclosure_find(cdev->parent, NULL);
 	if (!edev)
 		return;
 
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 4332442..d77811e 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -123,7 +123,8 @@ enclosure_component_register(struct enclosure_device *, unsigned int,
 int enclosure_add_device(struct enclosure_device *enclosure, int component,
 			 struct device *dev);
 int enclosure_remove_device(struct enclosure_device *enclosure, int component);
-struct enclosure_device *enclosure_find(struct device *dev);
+struct enclosure_device *enclosure_find(struct device *dev,
+					struct enclosure_device *start);
 int enclosure_for_each_device(int (*fn)(struct enclosure_device *, void *),
 			      void *data);
 
-- 
1.6.3.3

[PATCH 2/3] ses: add support for enclosure component hot removal

[James Bottomley]

Right at the moment, hot removal of a device within an enclosure does
nothing (because the intf_remove only copes with enclosure removal not
with component removal). Fix this by adding a function to remove the
component.  Also needed to fix the prototype of
enclosure_remove_device, since we know the device we've removed but
not the internal component number
---
 drivers/misc/enclosure.c  |   22 ++++++++++++++--------
 drivers/scsi/ses.c        |   33 ++++++++++++++++++++++++++-------
 include/linux/enclosure.h |    2 +-
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 789d121..850706a 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -332,19 +332,25 @@ EXPORT_SYMBOL_GPL(enclosure_add_device);
  * Returns zero on success or an error.
  *
  */
-int enclosure_remove_device(struct enclosure_device *edev, int component)
+int enclosure_remove_device(struct enclosure_device *edev, struct device *dev)
 {
 	struct enclosure_component *cdev;
+	int i;
 
-	if (!edev || component >= edev->components)
+	if (!edev || !dev)
 		return -EINVAL;
 
-	cdev = &edev->component[component];
-
-	device_del(&cdev->cdev);
-	put_device(cdev->dev);
-	cdev->dev = NULL;
-	return device_add(&cdev->cdev);
+	for (i = 0; i < edev->components; i++) {
+		cdev = &edev->component[i];
+		if (cdev->dev == dev) {
+			enclosure_remove_links(cdev);
+			device_del(&cdev->cdev);
+			put_device(dev);
+			cdev->dev = NULL;
+			return device_add(&cdev->cdev);
+		}
+	}
+	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(enclosure_remove_device);
 
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index e1b8c82..be593c8 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -616,18 +616,26 @@ static int ses_remove(struct device *dev)
 	return 0;
 }
 
-static void ses_intf_remove(struct device *cdev,
-			    struct class_interface *intf)
+static void ses_intf_remove_component(struct scsi_device *sdev)
+{
+	struct enclosure_device *edev, *prev = NULL;
+
+	while ((edev = enclosure_find(&sdev->host->shost_gendev, prev)) != NULL) {
+		prev = edev;
+		if (!enclosure_remove_device(edev, &sdev->sdev_gendev))
+			break;
+	}
+	if (edev)
+		put_device(&edev->edev);
+}
+
+static void ses_intf_remove_enclosure(struct scsi_device *sdev)
 {
-	struct scsi_device *sdev = to_scsi_device(cdev->parent);
 	struct enclosure_device *edev;
 	struct ses_device *ses_dev;
 
-	if (!scsi_device_enclosure(sdev))
-		return;
-
 	/*  exact match to this enclosure */
-	edev = enclosure_find(cdev->parent, NULL);
+	edev = enclosure_find(&sdev->sdev_gendev, NULL);
 	if (!edev)
 		return;
 
@@ -645,6 +653,17 @@ static void ses_intf_remove(struct device *cdev,
 	enclosure_unregister(edev);
 }
 
+static void ses_intf_remove(struct device *cdev,
+			    struct class_interface *intf)
+{
+	struct scsi_device *sdev = to_scsi_device(cdev->parent);
+
+	if (!scsi_device_enclosure(sdev))
+		ses_intf_remove_component(sdev);
+	else
+		ses_intf_remove_enclosure(sdev);
+}
+
 static struct class_interface ses_interface = {
 	.add_dev	= ses_intf_add,
 	.remove_dev	= ses_intf_remove,
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index d77811e..90d1c21 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -122,7 +122,7 @@ enclosure_component_register(struct enclosure_device *, unsigned int,
 				 enum enclosure_component_type, const char *);
 int enclosure_add_device(struct enclosure_device *enclosure, int component,
 			 struct device *dev);
-int enclosure_remove_device(struct enclosure_device *enclosure, int component);
+int enclosure_remove_device(struct enclosure_device *, struct device *);
 struct enclosure_device *enclosure_find(struct device *dev,
 					struct enclosure_device *start);
 int enclosure_for_each_device(int (*fn)(struct enclosure_device *, void *),
-- 
1.6.3.3

ses: update enclosure data on hot add

[James Bottomley]

Now that hot add works correctly, if a new device is added, we're still
operating on stale enclosure data, so fix that by updating the enclosure
diagnostic pages when we get notified of a device hot add

---
 drivers/misc/enclosure.c |    3 +
 drivers/scsi/ses.c       |  168 ++++++++++++++++++++++++++-------------------
 2 files changed, 100 insertions(+), 71 deletions(-)

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 850706a..7b03930 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -315,6 +315,9 @@ int enclosure_add_device(struct enclosure_device *edev, int component,
 
 	cdev = &edev->component[component];
 
+	if (cdev->dev == dev)
+		return -EEXIST;
+
 	if (cdev->dev)
 		enclosure_remove_links(cdev);
 
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index be593c8..55b034b 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -347,6 +347,97 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev,
 	return 0;
 }
 
+#define INIT_ALLOC_SIZE 32
+
+static void ses_enclosure_data_process(struct enclosure_device *edev,
+				       struct scsi_device *sdev,
+				       int create)
+{
+	u32 result;
+	unsigned char *buf = NULL, *type_ptr, *desc_ptr, *addl_desc_ptr = NULL;
+	int i, j, page7_len, len, components;
+	struct ses_device *ses_dev = edev->scratch;
+	int types = ses_dev->page1[10];
+	unsigned char *hdr_buf = kzalloc(INIT_ALLOC_SIZE, GFP_KERNEL);
+
+	if (!hdr_buf)
+		goto simple_populate;
+
+	/* re-read page 10 */
+	if (ses_dev->page10)
+		ses_recv_diag(sdev, 10, ses_dev->page10, ses_dev->page10_len);
+	/* Page 7 for the descriptors is optional */
+	result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
+	if (result)
+		goto simple_populate;
+
+	page7_len = len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
+	/* add 1 for trailing '\0' we'll use */
+	buf = kzalloc(len + 1, GFP_KERNEL);
+	if (!buf)
+		goto simple_populate;
+	result = ses_recv_diag(sdev, 7, buf, len);
+	if (result) {
+ simple_populate:
+		kfree(buf);
+		buf = NULL;
+		desc_ptr = NULL;
+		len = 0;
+		page7_len = 0;
+	} else {
+		desc_ptr = buf + 8;
+		len = (desc_ptr[2] << 8) + desc_ptr[3];
+		/* skip past overall descriptor */
+		desc_ptr += len + 4;
+		if (ses_dev->page10)
+			addl_desc_ptr = ses_dev->page10 + 8;
+	}
+	type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
+	components = 0;
+	for (i = 0; i < types; i++, type_ptr += 4) {
+		for (j = 0; j < type_ptr[1]; j++) {
+			char *name = NULL;
+			struct enclosure_component *ecomp;
+
+			if (desc_ptr) {
+				if (desc_ptr >= buf + page7_len) {
+					desc_ptr = NULL;
+				} else {
+					len = (desc_ptr[2] << 8) + desc_ptr[3];
+					desc_ptr += 4;
+					/* Add trailing zero - pushes into
+					 * reserved space */
+					desc_ptr[len] = '\0';
+					name = desc_ptr;
+				}
+			}
+			if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
+			    type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
+
+				if (create)
+					ecomp =	enclosure_component_register(edev,
+									     components++,
+									     type_ptr[0],
+									     name);
+				else
+					ecomp = &edev->component[components++];
+
+				if (!IS_ERR(ecomp) && addl_desc_ptr)
+					ses_process_descriptor(ecomp,
+							       addl_desc_ptr);
+			}
+			if (desc_ptr)
+				desc_ptr += len;
+
+			if (addl_desc_ptr)
+				addl_desc_ptr += addl_desc_ptr[1] + 2;
+
+		}
+	}
+	kfree(buf);
+	kfree(hdr_buf);
+}
+
 static void ses_match_to_enclosure(struct enclosure_device *edev,
 				   struct scsi_device *sdev)
 {
@@ -361,6 +452,8 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
 	if (!buf)
 		return;
 
+	ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0);
+
 	vpd_len = ((buf[2] << 8) | buf[3]) + 4;
 
 	desc = buf + 4;
@@ -395,18 +488,15 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
 	kfree(buf);
 }
 
-#define INIT_ALLOC_SIZE 32
-
 static int ses_intf_add(struct device *cdev,
 			struct class_interface *intf)
 {
 	struct scsi_device *sdev = to_scsi_device(cdev->parent);
 	struct scsi_device *tmp_sdev;
-	unsigned char *buf = NULL, *hdr_buf, *type_ptr, *desc_ptr = NULL,
-		*addl_desc_ptr = NULL;
+	unsigned char *buf = NULL, *hdr_buf, *type_ptr;
 	struct ses_device *ses_dev;
 	u32 result;
-	int i, j, types, len, page7_len = 0, components = 0;
+	int i, types, len, components = 0;
 	int err = -ENOMEM;
 	struct enclosure_device *edev;
 	struct ses_component *scomp = NULL;
@@ -501,6 +591,7 @@ static int ses_intf_add(struct device *cdev,
 		ses_dev->page10_len = len;
 		buf = NULL;
 	}
+	kfree(hdr_buf);
 
 	scomp = kzalloc(sizeof(struct ses_component) * components, GFP_KERNEL);
 	if (!scomp)
@@ -517,72 +608,7 @@ static int ses_intf_add(struct device *cdev,
 	for (i = 0; i < components; i++)
 		edev->component[i].scratch = scomp + i;
 
-	/* Page 7 for the descriptors is optional */
-	result = ses_recv_diag(sdev, 7, hdr_buf, INIT_ALLOC_SIZE);
-	if (result)
-		goto simple_populate;
-
-	page7_len = len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
-	/* add 1 for trailing '\0' we'll use */
-	buf = kzalloc(len + 1, GFP_KERNEL);
-	if (!buf)
-		goto simple_populate;
-	result = ses_recv_diag(sdev, 7, buf, len);
-	if (result) {
- simple_populate:
-		kfree(buf);
-		buf = NULL;
-		desc_ptr = NULL;
-		addl_desc_ptr = NULL;
-	} else {
-		desc_ptr = buf + 8;
-		len = (desc_ptr[2] << 8) + desc_ptr[3];
-		/* skip past overall descriptor */
-		desc_ptr += len + 4;
-		if (ses_dev->page10)
-			addl_desc_ptr = ses_dev->page10 + 8;
-	}
-	type_ptr = ses_dev->page1 + 12 + ses_dev->page1[11];
-	components = 0;
-	for (i = 0; i < types; i++, type_ptr += 4) {
-		for (j = 0; j < type_ptr[1]; j++) {
-			char *name = NULL;
-			struct enclosure_component *ecomp;
-
-			if (desc_ptr) {
-				if (desc_ptr >= buf + page7_len) {
-					desc_ptr = NULL;
-				} else {
-					len = (desc_ptr[2] << 8) + desc_ptr[3];
-					desc_ptr += 4;
-					/* Add trailing zero - pushes into
-					 * reserved space */
-					desc_ptr[len] = '\0';
-					name = desc_ptr;
-				}
-			}
-			if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
-			    type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
-
-				ecomp =	enclosure_component_register(edev,
-							     components++,
-							     type_ptr[0],
-							     name);
-
-				if (!IS_ERR(ecomp) && addl_desc_ptr)
-					ses_process_descriptor(ecomp,
-							       addl_desc_ptr);
-			}
-			if (desc_ptr)
-				desc_ptr += len;
-
-			if (addl_desc_ptr)
-				addl_desc_ptr += addl_desc_ptr[1] + 2;
-
-		}
-	}
-	kfree(buf);
-	kfree(hdr_buf);
+	ses_enclosure_data_process(edev, sdev, 1);
 
 	/* see if there are any devices matching before
 	 * we found the enclosure */
-- 
1.6.3.3