[SCSI] libsas: fix port->dev_list locking
authorDan Williams <dan.j.williams@intel.com>
Thu, 22 Sep 2011 05:05:34 +0000 (22:05 -0700)
committerJames Bottomley <JBottomley@Parallels.com>
Sun, 16 Oct 2011 15:54:02 +0000 (10:54 -0500)
port->dev_list maintains a list of devices attached to a given sas root port.
It needs to be mutated under a lock as contexts outside of the
single-threaded-libsas-workqueue access the list via sas_find_dev_by_rphy().
Fixup locations where the list was being mutated without a lock.

This is a follow-up to commit 5911e963 "[SCSI] libsas: remove expander
from dev list on error", where Luben noted [1]:

    > 2/ We have unlocked list manipulations in sas_ex_discover_end_dev(),
    > sas_unregister_common_dev(), and sas_ex_discover_end_dev()

    Yes, I can see that and that is very unfortunate.

[1]: http://marc.info/?l=linux-scsi&m=131480962006471&w=2

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
drivers/scsi/libsas/sas_discover.c
drivers/scsi/libsas/sas_expander.c
include/scsi/libsas.h

index f583193..54a5199 100644 (file)
@@ -219,17 +219,20 @@ out_err2:
 
 /* ---------- Device registration and unregistration ---------- */
 
-static inline void sas_unregister_common_dev(struct domain_device *dev)
+static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_device *dev)
 {
        sas_notify_lldd_dev_gone(dev);
        if (!dev->parent)
                dev->port->port_dev = NULL;
        else
                list_del_init(&dev->siblings);
+
+       spin_lock_irq(&port->dev_list_lock);
        list_del_init(&dev->dev_list_node);
+       spin_unlock_irq(&port->dev_list_lock);
 }
 
-void sas_unregister_dev(struct domain_device *dev)
+void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
 {
        if (dev->rphy) {
                sas_remove_children(&dev->rphy->dev);
@@ -241,15 +244,15 @@ void sas_unregister_dev(struct domain_device *dev)
                kfree(dev->ex_dev.ex_phy);
                dev->ex_dev.ex_phy = NULL;
        }
-       sas_unregister_common_dev(dev);
+       sas_unregister_common_dev(port, dev);
 }
 
 void sas_unregister_domain_devices(struct asd_sas_port *port)
 {
        struct domain_device *dev, *n;
 
-       list_for_each_entry_safe_reverse(dev,n,&port->dev_list,dev_list_node)
-               sas_unregister_dev(dev);
+       list_for_each_entry_safe_reverse(dev, n, &port->dev_list, dev_list_node)
+               sas_unregister_dev(port, dev);
 
        port->port->rphy = NULL;
 
index b172f17..88bbe8e 100644 (file)
@@ -754,7 +754,10 @@ static struct domain_device *sas_ex_discover_end_dev(
  out_list_del:
        sas_rphy_free(child->rphy);
        child->rphy = NULL;
+
+       spin_lock_irq(&parent->port->dev_list_lock);
        list_del(&child->dev_list_node);
+       spin_unlock_irq(&parent->port->dev_list_lock);
  out_free:
        sas_port_delete(phy->port);
  out_err:
@@ -1739,7 +1742,7 @@ out:
        return res;
 }
 
-static void sas_unregister_ex_tree(struct domain_device *dev)
+static void sas_unregister_ex_tree(struct asd_sas_port *port, struct domain_device *dev)
 {
        struct expander_device *ex = &dev->ex_dev;
        struct domain_device *child, *n;
@@ -1748,11 +1751,11 @@ static void sas_unregister_ex_tree(struct domain_device *dev)
                child->gone = 1;
                if (child->dev_type == EDGE_DEV ||
                    child->dev_type == FANOUT_DEV)
-                       sas_unregister_ex_tree(child);
+                       sas_unregister_ex_tree(port, child);
                else
-                       sas_unregister_dev(child);
+                       sas_unregister_dev(port, child);
        }
-       sas_unregister_dev(dev);
+       sas_unregister_dev(port, dev);
 }
 
 static void sas_unregister_devs_sas_addr(struct domain_device *parent,
@@ -1769,9 +1772,9 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
                                child->gone = 1;
                                if (child->dev_type == EDGE_DEV ||
                                    child->dev_type == FANOUT_DEV)
-                                       sas_unregister_ex_tree(child);
+                                       sas_unregister_ex_tree(parent->port, child);
                                else
-                                       sas_unregister_dev(child);
+                                       sas_unregister_dev(parent->port, child);
                                break;
                        }
                }
index c4b7cd0..6a308d4 100644 (file)
@@ -656,7 +656,7 @@ int  sas_discover_event(struct asd_sas_port *, enum discover_event ev);
 int  sas_discover_sata(struct domain_device *);
 int  sas_discover_end_dev(struct domain_device *);
 
-void sas_unregister_dev(struct domain_device *);
+void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *);
 
 void sas_init_dev(struct domain_device *);