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

[SCSI] libsas, libata: fix start of life for a sas ata_port

This changes the ordering of initialization and probing events from:
1/ allocate rphy in PORTE_BYTES_DMAED, DISCE_REVALIDATE_DOMAIN
2/ allocate ata_port and schedule port probe in DISCE_PROBE
...to:
1/ allocate ata_port in PORTE_BYTES_DMAED, DISCE_REVALIDATE_DOMAIN
2/ allocate rphy in PORTE_BYTES_DMAED, DISCE_REVALIDATE_DOMAIN
3/ schedule port probe in DISCE_PROBE

This ordering prevents PHYE_SIGNAL_LOSS_EVENTS from sneaking in to
destrory ata devices before they have been fully initialized:

BUG: unable to handle kernel paging request at 0000000000003b10
IP: [<ffffffffa0053d7e>] sas_ata_end_eh+0x12/0x5e [libsas]
...
[<ffffffffa004d1af>] sas_unregister_common_dev+0x78/0xc9 [libsas]
[<ffffffffa004d4d4>] sas_unregister_dev+0x4f/0xad [libsas]
[<ffffffffa004d5b1>] sas_unregister_domain_devices+0x7f/0xbf [libsas]
[<ffffffffa004c487>] sas_deform_port+0x61/0x1b8 [libsas]
[<ffffffffa004bed0>] sas_phye_loss_of_signal+0x29/0x2b [libsas]

...and kills the awkward "sata domain_device briefly existing in the
domain without an ata_port" state.

Reported-by: Michal Kosciowski <michal.kosciowski@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Jeff Garzik <jgarzik@redhat.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

authored by

Dan Williams and committed by
James Bottomley
b2024459 0f3fce5c

+56 -50
+21 -16
drivers/ata/libata-scsi.c
··· 3839 3839 } 3840 3840 EXPORT_SYMBOL_GPL(ata_sas_port_stop); 3841 3841 3842 - int ata_sas_async_port_init(struct ata_port *ap) 3842 + /** 3843 + * ata_sas_async_probe - simply schedule probing and return 3844 + * @ap: Port to probe 3845 + * 3846 + * For batch scheduling of probe for sas attached ata devices, assumes 3847 + * the port has already been through ata_sas_port_init() 3848 + */ 3849 + void ata_sas_async_probe(struct ata_port *ap) 3843 3850 { 3844 - int rc = ap->ops->port_start(ap); 3845 - 3846 - if (!rc) { 3847 - ap->print_id = atomic_inc_return(&ata_print_id); 3848 - __ata_port_probe(ap); 3849 - } 3850 - 3851 - return rc; 3851 + __ata_port_probe(ap); 3852 3852 } 3853 - EXPORT_SYMBOL_GPL(ata_sas_async_port_init); 3853 + EXPORT_SYMBOL_GPL(ata_sas_async_probe); 3854 + 3855 + int ata_sas_sync_probe(struct ata_port *ap) 3856 + { 3857 + return ata_port_probe(ap); 3858 + } 3859 + EXPORT_SYMBOL_GPL(ata_sas_sync_probe); 3860 + 3854 3861 3855 3862 /** 3856 3863 * ata_sas_port_init - Initialize a SATA device ··· 3874 3867 { 3875 3868 int rc = ap->ops->port_start(ap); 3876 3869 3877 - if (!rc) { 3878 - ap->print_id = atomic_inc_return(&ata_print_id); 3879 - rc = ata_port_probe(ap); 3880 - } 3881 - 3882 - return rc; 3870 + if (rc) 3871 + return rc; 3872 + ap->print_id = atomic_inc_return(&ata_print_id); 3873 + return 0; 3883 3874 } 3884 3875 EXPORT_SYMBOL_GPL(ata_sas_port_init); 3885 3876
+5 -1
drivers/scsi/ipr.c
··· 4549 4549 ENTER; 4550 4550 if (sdev->sdev_target) 4551 4551 sata_port = sdev->sdev_target->hostdata; 4552 - if (sata_port) 4552 + if (sata_port) { 4553 4553 rc = ata_sas_port_init(sata_port->ap); 4554 + if (rc == 0) 4555 + rc = ata_sas_sync_probe(sata_port->ap); 4556 + } 4557 + 4554 4558 if (rc) 4555 4559 ipr_slave_destroy(sdev); 4556 4560
+10 -23
drivers/scsi/libsas/sas_ata.c
··· 546 546 .port_ops = &sas_sata_ops 547 547 }; 548 548 549 - int sas_ata_init_host_and_port(struct domain_device *found_dev) 549 + int sas_ata_init(struct domain_device *found_dev) 550 550 { 551 551 struct sas_ha_struct *ha = found_dev->port->ha; 552 552 struct Scsi_Host *shost = ha->core.shost; 553 553 struct ata_port *ap; 554 + int rc; 554 555 555 556 ata_host_init(&found_dev->sata_dev.ata_host, 556 557 ha->dev, ··· 568 567 ap->private_data = found_dev; 569 568 ap->cbl = ATA_CBL_SATA; 570 569 ap->scsi_host = shost; 571 - /* publish initialized ata port */ 572 - smp_wmb(); 570 + rc = ata_sas_port_init(ap); 571 + if (rc) { 572 + ata_sas_port_destroy(ap); 573 + return rc; 574 + } 573 575 found_dev->sata_dev.ap = ap; 574 576 575 577 return 0; ··· 652 648 void sas_probe_sata(struct asd_sas_port *port) 653 649 { 654 650 struct domain_device *dev, *n; 655 - int err; 656 651 657 652 mutex_lock(&port->ha->disco_mutex); 658 - list_for_each_entry_safe(dev, n, &port->disco_list, disco_list_node) { 653 + list_for_each_entry(dev, &port->disco_list, disco_list_node) { 659 654 if (!dev_is_sata(dev)) 660 655 continue; 661 656 662 - err = sas_ata_init_host_and_port(dev); 663 - if (err) 664 - sas_fail_probe(dev, __func__, err); 665 - else 666 - ata_sas_async_port_init(dev->sata_dev.ap); 657 + ata_sas_async_probe(dev->sata_dev.ap); 667 658 } 668 659 mutex_unlock(&port->ha->disco_mutex); 669 660 ··· 717 718 sas_put_device(dev); 718 719 } 719 720 720 - static bool sas_ata_dev_eh_valid(struct domain_device *dev) 721 - { 722 - struct ata_port *ap; 723 - 724 - if (!dev_is_sata(dev)) 725 - return false; 726 - ap = dev->sata_dev.ap; 727 - /* consume fully initialized ata ports */ 728 - smp_rmb(); 729 - return !!ap; 730 - } 731 - 732 721 void sas_ata_strategy_handler(struct Scsi_Host *shost) 733 722 { 734 723 struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(shost); ··· 740 753 741 754 spin_lock(&port->dev_list_lock); 742 755 list_for_each_entry(dev, &port->dev_list, dev_list_node) { 743 - if (!sas_ata_dev_eh_valid(dev)) 756 + if (!dev_is_sata(dev)) 744 757 continue; 745 758 async_schedule_domain(async_sas_ata_eh, dev, &async); 746 759 }
+10 -3
drivers/scsi/libsas/sas_discover.c
··· 72 72 struct asd_sas_phy *phy; 73 73 struct sas_rphy *rphy; 74 74 struct domain_device *dev; 75 + int rc = -ENODEV; 75 76 76 77 dev = sas_alloc_device(); 77 78 if (!dev) ··· 111 110 112 111 sas_init_dev(dev); 113 112 113 + dev->port = port; 114 114 switch (dev->dev_type) { 115 - case SAS_END_DEV: 116 115 case SATA_DEV: 116 + rc = sas_ata_init(dev); 117 + if (rc) { 118 + rphy = NULL; 119 + break; 120 + } 121 + /* fall through */ 122 + case SAS_END_DEV: 117 123 rphy = sas_end_device_alloc(port->port); 118 124 break; 119 125 case EDGE_DEV: ··· 139 131 140 132 if (!rphy) { 141 133 sas_put_device(dev); 142 - return -ENODEV; 134 + return rc; 143 135 } 144 136 145 137 rphy->identify.phy_identifier = phy->phy->identify.phy_identifier; ··· 147 139 sas_fill_in_rphy(dev, rphy); 148 140 sas_hash_addr(dev->hashed_sas_addr, dev->sas_addr); 149 141 port->port_dev = dev; 150 - dev->port = port; 151 142 dev->linkrate = port->linkrate; 152 143 dev->min_linkrate = port->linkrate; 153 144 dev->max_linkrate = port->linkrate;
+6 -4
drivers/scsi/libsas/sas_expander.c
··· 790 790 if (res) 791 791 goto out_free; 792 792 793 - rphy = sas_end_device_alloc(phy->port); 794 - if (unlikely(!rphy)) 795 - goto out_free; 796 - 797 793 sas_init_dev(child); 794 + res = sas_ata_init(child); 795 + if (res) 796 + goto out_free; 797 + rphy = sas_end_device_alloc(phy->port); 798 + if (!rphy) 799 + goto out_free; 798 800 799 801 child->rphy = rphy; 800 802 get_device(&rphy->dev);
+2 -1
include/linux/libata.h
··· 996 996 extern void ata_sas_port_destroy(struct ata_port *); 997 997 extern struct ata_port *ata_sas_port_alloc(struct ata_host *, 998 998 struct ata_port_info *, struct Scsi_Host *); 999 - extern int ata_sas_async_port_init(struct ata_port *); 999 + extern void ata_sas_async_probe(struct ata_port *ap); 1000 + extern int ata_sas_sync_probe(struct ata_port *ap); 1000 1001 extern int ata_sas_port_init(struct ata_port *); 1001 1002 extern int ata_sas_port_start(struct ata_port *ap); 1002 1003 extern void ata_sas_port_stop(struct ata_port *ap);
+2 -2
include/scsi/sas_ata.h
··· 37 37 } 38 38 39 39 int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy); 40 - int sas_ata_init_host_and_port(struct domain_device *found_dev); 40 + int sas_ata_init(struct domain_device *dev); 41 41 void sas_ata_task_abort(struct sas_task *task); 42 42 void sas_ata_strategy_handler(struct Scsi_Host *shost); 43 43 void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q, ··· 52 52 { 53 53 return 0; 54 54 } 55 - static inline int sas_ata_init_host_and_port(struct domain_device *found_dev) 55 + static inline int sas_ata_init(struct domain_device *dev) 56 56 { 57 57 return 0; 58 58 }