Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[SCSI] libsas: fix domain_device leak

Arrange for the deallocation of a struct domain_device object when it no
longer has:
1/ any children
2/ references by any scsi_targets
3/ references by a lldd

The comment about domain_device lifetime in
Documentation/scsi/libsas.txt is stale as it appears mainline never had
a version of a struct domain_device that was registered as a kobject.
We now manage domain_device reference counts on behalf of external
agents.

Reviewed-by: Jack Wang <jack_wang@usish.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

authored by

Dan Williams and committed by
James Bottomley
735f7d2f 6f4e75a4

+56 -41
-15
Documentation/scsi/libsas.txt
··· 398 398 task_done -- callback when the task has finished execution 399 399 }; 400 400 401 - When an external entity, entity other than the LLDD or the 402 - SAS Layer, wants to work with a struct domain_device, it 403 - _must_ call kobject_get() when getting a handle on the 404 - device and kobject_put() when it is done with the device. 405 - 406 - This does two things: 407 - A) implements proper kfree() for the device; 408 - B) increments/decrements the kref for all players: 409 - domain_device 410 - all domain_device's ... (if past an expander) 411 - port 412 - host adapter 413 - pci device 414 - and up the ladder, etc. 415 - 416 401 DISCOVERY 417 402 --------- 418 403
+24 -12
drivers/scsi/libsas/sas_discover.c
··· 36 36 37 37 void sas_init_dev(struct domain_device *dev) 38 38 { 39 - INIT_LIST_HEAD(&dev->siblings); 40 - INIT_LIST_HEAD(&dev->dev_list_node); 41 39 switch (dev->dev_type) { 42 40 case SAS_END_DEV: 43 41 break; ··· 71 73 struct sas_rphy *rphy; 72 74 struct domain_device *dev; 73 75 74 - dev = kzalloc(sizeof(*dev), GFP_KERNEL); 76 + dev = sas_alloc_device(); 75 77 if (!dev) 76 78 return -ENOMEM; 77 79 78 80 spin_lock_irqsave(&port->phy_list_lock, flags); 79 81 if (list_empty(&port->phy_list)) { 80 82 spin_unlock_irqrestore(&port->phy_list_lock, flags); 81 - kfree(dev); 83 + sas_put_device(dev); 82 84 return -ENODEV; 83 85 } 84 86 phy = container_of(port->phy_list.next, struct asd_sas_phy, port_phy_el); ··· 128 130 } 129 131 130 132 if (!rphy) { 131 - kfree(dev); 133 + sas_put_device(dev); 132 134 return -ENODEV; 133 135 } 134 136 rphy->identify.phy_identifier = phy->phy->identify.phy_identifier; ··· 171 173 dev_name(sas_ha->dev), 172 174 SAS_ADDR(dev->sas_addr), res); 173 175 } 176 + kref_get(&dev->kref); 174 177 } 175 178 return res; 176 179 } ··· 183 184 struct Scsi_Host *shost = sas_ha->core.shost; 184 185 struct sas_internal *i = to_sas_internal(shost->transportt); 185 186 186 - if (i->dft->lldd_dev_gone) 187 + if (i->dft->lldd_dev_gone) { 187 188 i->dft->lldd_dev_gone(dev); 189 + sas_put_device(dev); 190 + } 188 191 } 189 192 190 193 /* ---------- Common/dispatchers ---------- */ ··· 220 219 221 220 /* ---------- Device registration and unregistration ---------- */ 222 221 222 + void sas_free_device(struct kref *kref) 223 + { 224 + struct domain_device *dev = container_of(kref, typeof(*dev), kref); 225 + 226 + if (dev->parent) 227 + sas_put_device(dev->parent); 228 + 229 + /* remove the phys and ports, everything else should be gone */ 230 + if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV) 231 + kfree(dev->ex_dev.ex_phy); 232 + 233 + kfree(dev); 234 + } 235 + 223 236 static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_device *dev) 224 237 { 225 238 sas_notify_lldd_dev_gone(dev); ··· 245 230 spin_lock_irq(&port->dev_list_lock); 246 231 list_del_init(&dev->dev_list_node); 247 232 spin_unlock_irq(&port->dev_list_lock); 233 + 234 + sas_put_device(dev); 248 235 } 249 236 250 237 void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev) ··· 255 238 sas_remove_children(&dev->rphy->dev); 256 239 sas_rphy_delete(dev->rphy); 257 240 dev->rphy = NULL; 258 - } 259 - if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV) { 260 - /* remove the phys and ports, everything else should be gone */ 261 - kfree(dev->ex_dev.ex_phy); 262 - dev->ex_dev.ex_phy = NULL; 263 241 } 264 242 sas_unregister_common_dev(port, dev); 265 243 } ··· 334 322 list_del_init(&dev->dev_list_node); 335 323 spin_unlock_irq(&port->dev_list_lock); 336 324 337 - kfree(dev); /* not kobject_register-ed yet */ 325 + sas_put_device(dev); 338 326 port->port_dev = NULL; 339 327 } 340 328
+6 -4
drivers/scsi/libsas/sas_expander.c
··· 657 657 if (phy->attached_sata_host || phy->attached_sata_ps) 658 658 return NULL; 659 659 660 - child = kzalloc(sizeof(*child), GFP_KERNEL); 660 + child = sas_alloc_device(); 661 661 if (!child) 662 662 return NULL; 663 663 664 + kref_get(&parent->kref); 664 665 child->parent = parent; 665 666 child->port = parent->port; 666 667 child->iproto = phy->attached_iproto; ··· 763 762 sas_port_delete(phy->port); 764 763 out_err: 765 764 phy->port = NULL; 766 - kfree(child); 765 + sas_put_device(child); 767 766 return NULL; 768 767 } 769 768 ··· 810 809 phy->attached_phy_id); 811 810 return NULL; 812 811 } 813 - child = kzalloc(sizeof(*child), GFP_KERNEL); 812 + child = sas_alloc_device(); 814 813 if (!child) 815 814 return NULL; 816 815 ··· 836 835 child->rphy = rphy; 837 836 edev = rphy_to_expander_device(rphy); 838 837 child->dev_type = phy->attached_dev_type; 838 + kref_get(&parent->kref); 839 839 child->parent = parent; 840 840 child->port = port; 841 841 child->iproto = phy->attached_iproto; ··· 860 858 spin_lock_irq(&parent->port->dev_list_lock); 861 859 list_del(&child->dev_list_node); 862 860 spin_unlock_irq(&parent->port->dev_list_lock); 863 - kfree(child); 861 + sas_put_device(child); 864 862 return NULL; 865 863 } 866 864 list_add_tail(&child->siblings, &parent->ex_dev.children);
+19
drivers/scsi/libsas/sas_internal.h
··· 76 76 77 77 void sas_hae_reset(struct work_struct *work); 78 78 79 + void sas_free_device(struct kref *kref); 80 + 79 81 #ifdef CONFIG_SCSI_SAS_HOST_SMP 80 82 extern int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req, 81 83 struct request *rsp); ··· 161 159 sas_port_mark_backlink(ex->parent_port); 162 160 } 163 161 sas_port_add_phy(ex->parent_port, ex_phy->phy); 162 + } 163 + 164 + static inline struct domain_device *sas_alloc_device(void) 165 + { 166 + struct domain_device *dev = kzalloc(sizeof(*dev), GFP_KERNEL); 167 + 168 + if (dev) { 169 + INIT_LIST_HEAD(&dev->siblings); 170 + INIT_LIST_HEAD(&dev->dev_list_node); 171 + kref_init(&dev->kref); 172 + } 173 + return dev; 174 + } 175 + 176 + static inline void sas_put_device(struct domain_device *dev) 177 + { 178 + kref_put(&dev->kref, sas_free_device); 164 179 } 165 180 166 181 #endif /* _SAS_INTERNAL_H_ */
+6 -10
drivers/scsi/libsas/sas_scsi_host.c
··· 737 737 return found_dev; 738 738 } 739 739 740 - static inline struct domain_device *sas_find_target(struct scsi_target *starget) 741 - { 742 - struct sas_rphy *rphy = dev_to_rphy(starget->dev.parent); 743 - 744 - return sas_find_dev_by_rphy(rphy); 745 - } 746 - 747 740 int sas_target_alloc(struct scsi_target *starget) 748 741 { 749 - struct domain_device *found_dev = sas_find_target(starget); 742 + struct sas_rphy *rphy = dev_to_rphy(starget->dev.parent); 743 + struct domain_device *found_dev = sas_find_dev_by_rphy(rphy); 750 744 int res; 751 745 752 746 if (!found_dev) ··· 752 758 return res; 753 759 } 754 760 761 + kref_get(&found_dev->kref); 755 762 starget->hostdata = found_dev; 756 763 return 0; 757 764 } ··· 1042 1047 1043 1048 void sas_target_destroy(struct scsi_target *starget) 1044 1049 { 1045 - struct domain_device *found_dev = sas_find_target(starget); 1050 + struct domain_device *found_dev = starget->hostdata; 1046 1051 1047 1052 if (!found_dev) 1048 1053 return; ··· 1050 1055 if (dev_is_sata(found_dev)) 1051 1056 ata_sas_port_destroy(found_dev->sata_dev.ap); 1052 1057 1053 - return; 1058 + starget->hostdata = NULL; 1059 + sas_put_device(found_dev); 1054 1060 } 1055 1061 1056 1062 static void sas_parse_addr(u8 *sas_addr, const char *p)
+1
include/scsi/libsas.h
··· 206 206 207 207 void *lldd_dev; 208 208 int gone; 209 + struct kref kref; 209 210 }; 210 211 211 212 struct sas_discovery_event {