diff options
author | Dan Williams <dan.j.williams@intel.com> | 2011-09-22 01:05:34 -0400 |
---|---|---|
committer | James Bottomley <JBottomley@Parallels.com> | 2011-10-16 11:54:02 -0400 |
commit | 1a34c0640137eed8dabdac3a68a7a84116ac9e0d (patch) | |
tree | fbe141523906f19c10c80bbdbdd4a69446c5fa44 | |
parent | 29f366e8a99fdced4c0b5417a478d7539adc66d3 (diff) |
[SCSI] libsas: fix port->dev_list locking
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>
-rw-r--r-- | drivers/scsi/libsas/sas_discover.c | 13 | ||||
-rw-r--r-- | drivers/scsi/libsas/sas_expander.c | 15 | ||||
-rw-r--r-- | include/scsi/libsas.h | 2 |
3 files changed, 18 insertions, 12 deletions
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c index f5831930df9b..54a5199ceb56 100644 --- a/drivers/scsi/libsas/sas_discover.c +++ b/drivers/scsi/libsas/sas_discover.c | |||
@@ -219,17 +219,20 @@ out_err2: | |||
219 | 219 | ||
220 | /* ---------- Device registration and unregistration ---------- */ | 220 | /* ---------- Device registration and unregistration ---------- */ |
221 | 221 | ||
222 | static inline void sas_unregister_common_dev(struct domain_device *dev) | 222 | static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_device *dev) |
223 | { | 223 | { |
224 | sas_notify_lldd_dev_gone(dev); | 224 | sas_notify_lldd_dev_gone(dev); |
225 | if (!dev->parent) | 225 | if (!dev->parent) |
226 | dev->port->port_dev = NULL; | 226 | dev->port->port_dev = NULL; |
227 | else | 227 | else |
228 | list_del_init(&dev->siblings); | 228 | list_del_init(&dev->siblings); |
229 | |||
230 | spin_lock_irq(&port->dev_list_lock); | ||
229 | list_del_init(&dev->dev_list_node); | 231 | list_del_init(&dev->dev_list_node); |
232 | spin_unlock_irq(&port->dev_list_lock); | ||
230 | } | 233 | } |
231 | 234 | ||
232 | void sas_unregister_dev(struct domain_device *dev) | 235 | void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev) |
233 | { | 236 | { |
234 | if (dev->rphy) { | 237 | if (dev->rphy) { |
235 | sas_remove_children(&dev->rphy->dev); | 238 | sas_remove_children(&dev->rphy->dev); |
@@ -241,15 +244,15 @@ void sas_unregister_dev(struct domain_device *dev) | |||
241 | kfree(dev->ex_dev.ex_phy); | 244 | kfree(dev->ex_dev.ex_phy); |
242 | dev->ex_dev.ex_phy = NULL; | 245 | dev->ex_dev.ex_phy = NULL; |
243 | } | 246 | } |
244 | sas_unregister_common_dev(dev); | 247 | sas_unregister_common_dev(port, dev); |
245 | } | 248 | } |
246 | 249 | ||
247 | void sas_unregister_domain_devices(struct asd_sas_port *port) | 250 | void sas_unregister_domain_devices(struct asd_sas_port *port) |
248 | { | 251 | { |
249 | struct domain_device *dev, *n; | 252 | struct domain_device *dev, *n; |
250 | 253 | ||
251 | list_for_each_entry_safe_reverse(dev,n,&port->dev_list,dev_list_node) | 254 | list_for_each_entry_safe_reverse(dev, n, &port->dev_list, dev_list_node) |
252 | sas_unregister_dev(dev); | 255 | sas_unregister_dev(port, dev); |
253 | 256 | ||
254 | port->port->rphy = NULL; | 257 | port->port->rphy = NULL; |
255 | 258 | ||
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index b172f1773081..88bbe8e1e844 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c | |||
@@ -754,7 +754,10 @@ static struct domain_device *sas_ex_discover_end_dev( | |||
754 | out_list_del: | 754 | out_list_del: |
755 | sas_rphy_free(child->rphy); | 755 | sas_rphy_free(child->rphy); |
756 | child->rphy = NULL; | 756 | child->rphy = NULL; |
757 | |||
758 | spin_lock_irq(&parent->port->dev_list_lock); | ||
757 | list_del(&child->dev_list_node); | 759 | list_del(&child->dev_list_node); |
760 | spin_unlock_irq(&parent->port->dev_list_lock); | ||
758 | out_free: | 761 | out_free: |
759 | sas_port_delete(phy->port); | 762 | sas_port_delete(phy->port); |
760 | out_err: | 763 | out_err: |
@@ -1739,7 +1742,7 @@ out: | |||
1739 | return res; | 1742 | return res; |
1740 | } | 1743 | } |
1741 | 1744 | ||
1742 | static void sas_unregister_ex_tree(struct domain_device *dev) | 1745 | static void sas_unregister_ex_tree(struct asd_sas_port *port, struct domain_device *dev) |
1743 | { | 1746 | { |
1744 | struct expander_device *ex = &dev->ex_dev; | 1747 | struct expander_device *ex = &dev->ex_dev; |
1745 | struct domain_device *child, *n; | 1748 | struct domain_device *child, *n; |
@@ -1748,11 +1751,11 @@ static void sas_unregister_ex_tree(struct domain_device *dev) | |||
1748 | child->gone = 1; | 1751 | child->gone = 1; |
1749 | if (child->dev_type == EDGE_DEV || | 1752 | if (child->dev_type == EDGE_DEV || |
1750 | child->dev_type == FANOUT_DEV) | 1753 | child->dev_type == FANOUT_DEV) |
1751 | sas_unregister_ex_tree(child); | 1754 | sas_unregister_ex_tree(port, child); |
1752 | else | 1755 | else |
1753 | sas_unregister_dev(child); | 1756 | sas_unregister_dev(port, child); |
1754 | } | 1757 | } |
1755 | sas_unregister_dev(dev); | 1758 | sas_unregister_dev(port, dev); |
1756 | } | 1759 | } |
1757 | 1760 | ||
1758 | static void sas_unregister_devs_sas_addr(struct domain_device *parent, | 1761 | 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, | |||
1769 | child->gone = 1; | 1772 | child->gone = 1; |
1770 | if (child->dev_type == EDGE_DEV || | 1773 | if (child->dev_type == EDGE_DEV || |
1771 | child->dev_type == FANOUT_DEV) | 1774 | child->dev_type == FANOUT_DEV) |
1772 | sas_unregister_ex_tree(child); | 1775 | sas_unregister_ex_tree(parent->port, child); |
1773 | else | 1776 | else |
1774 | sas_unregister_dev(child); | 1777 | sas_unregister_dev(parent->port, child); |
1775 | break; | 1778 | break; |
1776 | } | 1779 | } |
1777 | } | 1780 | } |
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h index c4b7cd0b85e5..6a308d42d98f 100644 --- a/include/scsi/libsas.h +++ b/include/scsi/libsas.h | |||
@@ -656,7 +656,7 @@ int sas_discover_event(struct asd_sas_port *, enum discover_event ev); | |||
656 | int sas_discover_sata(struct domain_device *); | 656 | int sas_discover_sata(struct domain_device *); |
657 | int sas_discover_end_dev(struct domain_device *); | 657 | int sas_discover_end_dev(struct domain_device *); |
658 | 658 | ||
659 | void sas_unregister_dev(struct domain_device *); | 659 | void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *); |
660 | 660 | ||
661 | void sas_init_dev(struct domain_device *); | 661 | void sas_init_dev(struct domain_device *); |
662 | 662 | ||