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

[SCSI] libsas: fix sas_find_local_phy(), take phy references

In the direct-attached case this routine returns the phy on which this
device was first discovered. Which is broken if we want to support
wide-targets, as this phy reference can become stale even though the
port is still active.

In the expander-attached case this routine tries to lookup the phy by
scanning the attached sas addresses of the parent expander, and BUG_ONs
if it can't find it. However since eh and the libsas workqueue run
independently we can still be attempting device recovery via eh after
libsas has recorded the device as detached. This is even easier to hit
now that eh is blocked while device domain rediscovery takes place, and
that libata is fed more timed out commands increasing the chances that
it will try to recover the ata device.

Arrange for dev->phy to always point to a last known good phy, it may be
stale after the port is torn down, but it will catch up for wide port
reconfigurations, and never be NULL.

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
f41a0c44 3a9c5560

+116 -44
+6 -3
drivers/scsi/aic94xx/aic94xx_tmf.c
··· 181 181 int asd_I_T_nexus_reset(struct domain_device *dev) 182 182 { 183 183 int res, tmp_res, i; 184 - struct sas_phy *phy = sas_find_local_phy(dev); 184 + struct sas_phy *phy = sas_get_local_phy(dev); 185 185 /* Standard mandates link reset for ATA (type 0) and 186 186 * hard reset for SSP (type 1) */ 187 187 int reset_type = (dev->dev_type == SATA_DEV || ··· 201 201 for (i = 0 ; i < 3; i++) { 202 202 tmp_res = asd_clear_nexus_I_T(dev, NEXUS_PHASE_RESUME); 203 203 if (tmp_res == TC_RESUME) 204 - return res; 204 + goto out; 205 205 msleep(500); 206 206 } 207 207 ··· 211 211 dev_printk(KERN_ERR, &phy->dev, 212 212 "Failed to resume nexus after reset 0x%x\n", tmp_res); 213 213 214 - return TMF_RESP_FUNC_FAILED; 214 + res = TMF_RESP_FUNC_FAILED; 215 + out: 216 + sas_put_local_phy(phy); 217 + return res; 215 218 } 216 219 217 220 static int asd_clear_nexus_I_T_L(struct domain_device *dev, u8 *lun)
+5 -4
drivers/scsi/isci/task.c
··· 1332 1332 static int isci_reset_device(struct isci_host *ihost, 1333 1333 struct isci_remote_device *idev) 1334 1334 { 1335 - struct sas_phy *phy = sas_find_local_phy(idev->domain_dev); 1335 + struct sas_phy *phy = sas_get_local_phy(idev->domain_dev); 1336 1336 enum sci_status status; 1337 1337 unsigned long flags; 1338 1338 int rc; ··· 1347 1347 dev_dbg(&ihost->pdev->dev, 1348 1348 "%s: sci_remote_device_reset(%p) returned %d!\n", 1349 1349 __func__, idev, status); 1350 - 1351 - return TMF_RESP_FUNC_FAILED; 1350 + rc = TMF_RESP_FUNC_FAILED; 1351 + goto out; 1352 1352 } 1353 1353 spin_unlock_irqrestore(&ihost->scic_lock, flags); 1354 1354 ··· 1369 1369 } 1370 1370 1371 1371 dev_dbg(&ihost->pdev->dev, "%s: idev %p complete.\n", __func__, idev); 1372 - 1372 + out: 1373 + sas_put_local_phy(phy); 1373 1374 return rc; 1374 1375 } 1375 1376
+5 -2
drivers/scsi/libsas/sas_ata.c
··· 284 284 struct ata_port *ap = link->ap; 285 285 struct domain_device *dev = ap->private_data; 286 286 struct domain_device *ex_dev = dev->parent; 287 - struct sas_phy *phy = sas_find_local_phy(dev); 287 + struct sas_phy *phy = sas_get_local_phy(dev); 288 288 289 289 res = sas_get_phy_attached_sas_addr(ex_dev, phy->number, addr); 290 + sas_put_local_phy(phy); 290 291 /* break the wait early if the expander is unreachable, 291 292 * otherwise keep polling 292 293 */ ··· 320 319 unsigned long deadline) 321 320 { 322 321 int ret = 0, res; 322 + struct sas_phy *phy; 323 323 struct ata_port *ap = link->ap; 324 324 int (*check_ready)(struct ata_link *link); 325 325 struct domain_device *dev = ap->private_data; 326 - struct sas_phy *phy = sas_find_local_phy(dev); 327 326 struct sas_internal *i = dev_to_sas_internal(dev); 328 327 329 328 res = i->dft->lldd_I_T_nexus_reset(dev); ··· 331 330 if (res != TMF_RESP_FUNC_COMPLETE) 332 331 SAS_DPRINTK("%s: Unable to reset ata device?\n", __func__); 333 332 333 + phy = sas_get_local_phy(dev); 334 334 if (scsi_is_sas_phy_local(phy)) 335 335 check_ready = local_ata_check_ready; 336 336 else 337 337 check_ready = smp_ata_check_ready; 338 + sas_put_local_phy(phy); 338 339 339 340 ret = ata_wait_after_reset(link, deadline, check_ready); 340 341 if (ret && ret != -EAGAIN)
+24
drivers/scsi/libsas/sas_discover.c
··· 147 147 memset(port->disc.eeds_a, 0, SAS_ADDR_SIZE); 148 148 memset(port->disc.eeds_b, 0, SAS_ADDR_SIZE); 149 149 port->disc.max_level = 0; 150 + sas_device_set_phy(dev, port->port); 150 151 151 152 dev->rphy = rphy; 152 153 ··· 235 234 if (dev->parent) 236 235 sas_put_device(dev->parent); 237 236 237 + sas_port_put_phy(dev->phy); 238 + dev->phy = NULL; 239 + 238 240 /* remove the phys and ports, everything else should be gone */ 239 241 if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV) 240 242 kfree(dev->ex_dev.ex_phy); ··· 310 306 311 307 port->port->rphy = NULL; 312 308 309 + } 310 + 311 + void sas_device_set_phy(struct domain_device *dev, struct sas_port *port) 312 + { 313 + struct sas_ha_struct *ha; 314 + struct sas_phy *new_phy; 315 + 316 + if (!dev) 317 + return; 318 + 319 + ha = dev->port->ha; 320 + new_phy = sas_port_get_phy(port); 321 + 322 + /* pin and record last seen phy */ 323 + spin_lock_irq(&ha->phy_port_lock); 324 + if (new_phy) { 325 + sas_port_put_phy(dev->phy); 326 + dev->phy = new_phy; 327 + } 328 + spin_unlock_irq(&ha->phy_port_lock); 313 329 } 314 330 315 331 /* ---------- Discovery and Revalidation ---------- */
+4 -1
drivers/scsi/libsas/sas_expander.c
··· 723 723 } 724 724 } 725 725 sas_ex_get_linkrate(parent, child, phy); 726 + sas_device_set_phy(child, phy->port); 726 727 727 728 #ifdef CONFIG_SCSI_SAS_ATA 728 729 if ((phy->attached_tproto & SAS_PROTOCOL_STP) || phy->attached_sata_dev) { ··· 1811 1810 { 1812 1811 struct expander_device *ex_dev = &parent->ex_dev; 1813 1812 struct ex_phy *phy = &ex_dev->ex_phy[phy_id]; 1814 - struct domain_device *child, *n; 1813 + struct domain_device *child, *n, *found = NULL; 1815 1814 if (last) { 1816 1815 list_for_each_entry_safe(child, n, 1817 1816 &ex_dev->children, siblings) { ··· 1823 1822 sas_unregister_ex_tree(parent->port, child); 1824 1823 else 1825 1824 sas_unregister_dev(parent->port, child); 1825 + found = child; 1826 1826 break; 1827 1827 } 1828 1828 } ··· 1832 1830 memset(phy->attached_sas_addr, 0, SAS_ADDR_SIZE); 1833 1831 if (phy->port) { 1834 1832 sas_port_delete_phy(phy->port, phy->phy); 1833 + sas_device_set_phy(found, phy->port); 1835 1834 if (phy->port->num_phys == 0) 1836 1835 sas_port_delete(phy->port); 1837 1836 phy->port = NULL;
+1
drivers/scsi/libsas/sas_internal.h
··· 87 87 enum phy_func phy_func, struct sas_phy_linkrates *); 88 88 int sas_smp_get_phy_events(struct sas_phy *phy); 89 89 90 + void sas_device_set_phy(struct domain_device *dev, struct sas_port *port); 90 91 struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy); 91 92 struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id); 92 93 int sas_get_phy_attached_sas_addr(struct domain_device *dev, int phy_id,
+3 -4
drivers/scsi/libsas/sas_port.c
··· 108 108 port->num_phys++; 109 109 port->phy_mask |= (1U << phy->id); 110 110 111 - if (!port->phy) 112 - port->phy = phy->phy; 113 - 114 111 if (*(u64 *)port->attached_sas_addr == 0) { 115 112 port->class = phy->class; 116 113 memcpy(port->attached_sas_addr, phy->attached_sas_addr, ··· 172 175 sas_unregister_domain_devices(port); 173 176 sas_port_delete(port->port); 174 177 port->port = NULL; 175 - } else 178 + } else { 176 179 sas_port_delete_phy(port->port, phy->phy); 180 + sas_device_set_phy(dev, port->port); 181 + } 177 182 178 183 if (si->dft->lldd_port_deformed) 179 184 si->dft->lldd_port_deformed(phy);
+18 -20
drivers/scsi/libsas/sas_scsi_host.c
··· 439 439 return res; 440 440 } 441 441 442 - /* Find the sas_phy that's attached to this device */ 443 - struct sas_phy *sas_find_local_phy(struct domain_device *dev) 442 + /* take a reference on the last known good phy for this device */ 443 + struct sas_phy *sas_get_local_phy(struct domain_device *dev) 444 444 { 445 - struct domain_device *pdev = dev->parent; 446 - struct ex_phy *exphy = NULL; 447 - int i; 445 + struct sas_ha_struct *ha = dev->port->ha; 446 + struct sas_phy *phy; 447 + unsigned long flags; 448 448 449 - /* Directly attached device */ 450 - if (!pdev) 451 - return dev->port->phy; 449 + /* a published domain device always has a valid phy, it may be 450 + * stale, but it is never NULL 451 + */ 452 + BUG_ON(!dev->phy); 452 453 453 - /* Otherwise look in the expander */ 454 - for (i = 0; i < pdev->ex_dev.num_phys; i++) 455 - if (!memcmp(dev->sas_addr, 456 - pdev->ex_dev.ex_phy[i].attached_sas_addr, 457 - SAS_ADDR_SIZE)) { 458 - exphy = &pdev->ex_dev.ex_phy[i]; 459 - break; 460 - } 454 + spin_lock_irqsave(&ha->phy_port_lock, flags); 455 + phy = dev->phy; 456 + get_device(&phy->dev); 457 + spin_unlock_irqrestore(&ha->phy_port_lock, flags); 461 458 462 - BUG_ON(!exphy); 463 - return exphy->phy; 459 + return phy; 464 460 } 465 - EXPORT_SYMBOL_GPL(sas_find_local_phy); 461 + EXPORT_SYMBOL_GPL(sas_get_local_phy); 466 462 467 463 /* Attempt to send a LUN reset message to a device */ 468 464 int sas_eh_device_reset_handler(struct scsi_cmnd *cmd) ··· 485 489 int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd) 486 490 { 487 491 struct domain_device *dev = cmd_to_domain_dev(cmd); 488 - struct sas_phy *phy = sas_find_local_phy(dev); 492 + struct sas_phy *phy = sas_get_local_phy(dev); 489 493 int res; 490 494 491 495 res = sas_phy_reset(phy, 1); ··· 493 497 SAS_DPRINTK("Bus reset of %s failed 0x%x\n", 494 498 kobject_name(&phy->dev.kobj), 495 499 res); 500 + sas_put_local_phy(phy); 501 + 496 502 if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE) 497 503 return SUCCESS; 498 504
+2 -1
drivers/scsi/mvsas/mv_sas.c
··· 1474 1474 static int mvs_debug_I_T_nexus_reset(struct domain_device *dev) 1475 1475 { 1476 1476 int rc; 1477 - struct sas_phy *phy = sas_find_local_phy(dev); 1477 + struct sas_phy *phy = sas_get_local_phy(dev); 1478 1478 int reset_type = (dev->dev_type == SATA_DEV || 1479 1479 (dev->tproto & SAS_PROTOCOL_STP)) ? 0 : 1; 1480 1480 rc = sas_phy_reset(phy, reset_type); 1481 + sas_put_local_phy(phy); 1481 1482 msleep(2000); 1482 1483 return rc; 1483 1484 }
+12 -7
drivers/scsi/pm8001/pm8001_sas.c
··· 967 967 968 968 pm8001_dev = dev->lldd_dev; 969 969 pm8001_ha = pm8001_find_ha_by_dev(dev); 970 - phy = sas_find_local_phy(dev); 970 + phy = sas_get_local_phy(dev); 971 971 972 972 if (dev_is_sata(dev)) { 973 973 DECLARE_COMPLETION_ONSTACK(completion_setstate); 974 - if (scsi_is_sas_phy_local(phy)) 975 - return 0; 974 + if (scsi_is_sas_phy_local(phy)) { 975 + rc = 0; 976 + goto out; 977 + } 976 978 rc = sas_phy_reset(phy, 1); 977 979 msleep(2000); 978 980 rc = pm8001_exec_internal_task_abort(pm8001_ha, pm8001_dev , ··· 983 981 rc = PM8001_CHIP_DISP->set_dev_state_req(pm8001_ha, 984 982 pm8001_dev, 0x01); 985 983 wait_for_completion(&completion_setstate); 986 - } else{ 987 - rc = sas_phy_reset(phy, 1); 988 - msleep(2000); 984 + } else { 985 + rc = sas_phy_reset(phy, 1); 986 + msleep(2000); 989 987 } 990 988 PM8001_EH_DBG(pm8001_ha, pm8001_printk(" for device[%x]:rc=%d\n", 991 989 pm8001_dev->device_id, rc)); 990 + out: 991 + sas_put_local_phy(phy); 992 992 return rc; 993 993 } 994 994 ··· 1002 998 struct pm8001_device *pm8001_dev = dev->lldd_dev; 1003 999 struct pm8001_hba_info *pm8001_ha = pm8001_find_ha_by_dev(dev); 1004 1000 if (dev_is_sata(dev)) { 1005 - struct sas_phy *phy = sas_find_local_phy(dev); 1001 + struct sas_phy *phy = sas_get_local_phy(dev); 1006 1002 rc = pm8001_exec_internal_task_abort(pm8001_ha, pm8001_dev , 1007 1003 dev, 1, 0); 1008 1004 rc = sas_phy_reset(phy, 1); 1005 + sas_put_local_phy(phy); 1009 1006 rc = PM8001_CHIP_DISP->set_dev_state_req(pm8001_ha, 1010 1007 pm8001_dev, 0x01); 1011 1008 msleep(2000);
+23
drivers/scsi/scsi_transport_sas.c
··· 1060 1060 EXPORT_SYMBOL(scsi_is_sas_port); 1061 1061 1062 1062 /** 1063 + * sas_port_get_phy - try to take a reference on a port member 1064 + * @port: port to check 1065 + */ 1066 + struct sas_phy *sas_port_get_phy(struct sas_port *port) 1067 + { 1068 + struct sas_phy *phy; 1069 + 1070 + mutex_lock(&port->phy_list_mutex); 1071 + if (list_empty(&port->phy_list)) 1072 + phy = NULL; 1073 + else { 1074 + struct list_head *ent = port->phy_list.next; 1075 + 1076 + phy = list_entry(ent, typeof(*phy), port_siblings); 1077 + get_device(&phy->dev); 1078 + } 1079 + mutex_unlock(&port->phy_list_mutex); 1080 + 1081 + return phy; 1082 + } 1083 + EXPORT_SYMBOL(sas_port_get_phy); 1084 + 1085 + /** 1063 1086 * sas_port_add_phy - add another phy to a port to form a wide port 1064 1087 * @port: port to add the phy to 1065 1088 * @phy: phy to add
+7 -2
include/scsi/libsas.h
··· 192 192 struct domain_device *parent; 193 193 struct list_head siblings; /* devices on the same level */ 194 194 struct asd_sas_port *port; /* shortcut to root of the tree */ 195 + struct sas_phy *phy; 195 196 196 197 struct list_head dev_list_node; 197 198 struct list_head disco_list_node; /* awaiting probe or destruct */ ··· 244 243 struct list_head destroy_list; 245 244 enum sas_linkrate linkrate; 246 245 247 - struct sas_phy *phy; 248 246 struct work_struct work; 249 247 250 248 /* public: */ ··· 427 427 static inline unsigned int to_sas_gpio_od(int device, int bit) 428 428 { 429 429 return 3 * device + bit; 430 + } 431 + 432 + static inline void sas_put_local_phy(struct sas_phy *phy) 433 + { 434 + put_device(&phy->dev); 430 435 } 431 436 432 437 #ifdef CONFIG_SCSI_SAS_HOST_SMP ··· 689 684 690 685 extern void sas_ssp_task_response(struct device *dev, struct sas_task *task, 691 686 struct ssp_response_iu *iu); 692 - struct sas_phy *sas_find_local_phy(struct domain_device *dev); 687 + struct sas_phy *sas_get_local_phy(struct domain_device *dev); 693 688 694 689 int sas_request_addr(struct Scsi_Host *shost, u8 *addr); 695 690
+6
include/scsi/scsi_transport_sas.h
··· 209 209 void sas_port_delete_phy(struct sas_port *, struct sas_phy *); 210 210 void sas_port_mark_backlink(struct sas_port *); 211 211 int scsi_is_sas_port(const struct device *); 212 + struct sas_phy *sas_port_get_phy(struct sas_port *port); 213 + static inline void sas_port_put_phy(struct sas_phy *phy) 214 + { 215 + if (phy) 216 + put_device(&phy->dev); 217 + } 212 218 213 219 extern struct scsi_transport_template * 214 220 sas_attach_transport(struct sas_function_template *);