Re: [PATCH v3 1/3] scsi: hisi_sas: Enable force phy when SATA disk directly connected

yangxingui
2025-02-24T13:12:27Z

Hi, John

On 2025/2/24 20:21, John Garry wrote:
> On 24/02/2025 09:36, yangxingui wrote:
>>>
>>>
>>> So do you mean that all IO to this disk will error? If yes, then this
>>> is good.
>> Yes, IO error or IO result does not meet expectations. As shown in the
>> log below, due to an abnormal port ID, the SNs of the two disks read
>> are the same.
>
> Do you mean that this is mainline kernel behaviour, below:
Yes
>
>>
>> [448000.504979] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1
>> link_rate=10(sata)
>> [448000.505070] sas: phy-10:1 added to port-10:1, phy_mask:0x2
>> (5000000000000a01)
>> [448000.505247] sas: DOING DISCOVERY on port 1, pid:2239187
>> [448000.505255] hisi_sas_v3_hw 0000:d4:02.0: dev[2:5] found
>> [448000.505274] sas: Enter sas_scsi_recover_host busy: 0 failed: 0
>> [448000.505295] sas: ata31: end_device-10:0: dev error handler
>> [448000.505299] sas: ata32: end_device-10:1: dev error handler
>> [448001.300517] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy1
>> phy_state=0x1   // phy1's hw port id released
>> [448001.300522] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy1 down
>> [448001.436187] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2
>> link_rate=10(sata) // phy2 occupies the hardware port ID of phy1
>> [448001.608766] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy1
>> link_rate=10(sata) // phy1 was assigned a new hardware port ID
>> [448001.775605] ata32.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133
>> [448002.159364] sas: phy-10:2 added to port-10:2, phy_mask:0x4
>> (5000000000000a02)
>> [448002.159575] sas: DOING DISCOVERY on port 2, pid:2239187
>> [448002.159581] hisi_sas_v3_hw 0000:d4:02.0: dev[3:5] found
>> [448002.159602] sas: Enter sas_scsi_recover_host busy: 0 failed: 0
>> [448002.159623] sas: ata31: end_device-10:0: dev error handler
>> [448002.159633] sas: ata32: end_device-10:1: dev error handler
>> [448002.159636] sas: ata33: end_device-10:2: dev error handler
>> [448002.393349] hisi_sas_v3_hw 0000:d4:02.0: phydown: phy2 phy_state=0x3
>> [448002.393354] hisi_sas_v3_hw 0000:d4:02.0: ignore flutter phy2 down
>> [448002.684937] hisi_sas_v3_hw 0000:d4:02.0: phyup: phy2
>> link_rate=10(sata)
>> [448002.851639] ata33.00: ATA-11: WUH721816ALE6L4, PCGAW660, max UDMA/133
>> [448002.851644] ata33.00: 31251759104 sectors, multi 0: LBA48 NCQ
>> (depth 32)
>>
>>>
>>> But I still don't like the handling in this patch. If we get a phy
>>> up, then the directly-attached disk ideally should be gone already,
>>> so should not have to do this handling.
>> There is no problem when the disk is removed. The current problem is
>> that multiple phy up at the same time. When one of the phys up and
>> enters error handler to execute hardreset, the phy will down and then
>> up. other phy up will probably occupy the hw port id of the previous
>> phy which do hardreset in EH.
>
> Could you do this work (itct update) in lldd_ata_check_ready CB?

It's a good idea only for sata disks, but the current problem is not
only the scenario of connecting the sata disk. This phenomenon
occasionally occurs when the SAS disk is connected after the controller
is reset. The following is the log of the stress test recurrence after
incorporating the current repair patch. Although we called
hisi_sas_refresh_port_id() on controller reset.

[ 5387.235015] hisi_sas_v3_hw 0000:74:02.0: I_T nexus reset: internal
abort (-5)
[ 5387.242126] sas: clear nexus ha
[ 5387.245283] hisi_sas_v3_hw 0000:74:02.0: controller resetting...
[ 5388.908489] hisi_sas_v3_hw 0000:74:02.0: phyup: phy5 link_rate=10(sata)
[ 5388.915090] hisi_sas_v3_hw 0000:74:02.0: phyup: phy6 link_rate=10(sata)
[ 5388.934505] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata)
[ 5388.941009] hisi_sas_v3_hw 0000:74:02.0: phyup: phy1 link_rate=9(sata)
[ 5388.950976] hisi_sas_v3_hw 0000:74:02.0: phyup: phy4 link_rate=11
[ 5388.957048] hisi_sas_v3_hw 0000:74:02.0: phyup: phy7 link_rate=11
[ 5388.980097] hisi_sas_v3_hw 0000:74:02.0: phyup: phy2 link_rate=11
[ 5388.986169] hisi_sas_v3_hw 0000:74:02.0: phyup: phy3 link_rate=11 //
phy3 attached a sas disk.
[ 5389.065103] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not
attach device
[ 5389.072409] sas: executing TMF task failed 5000c500ae49c8f1 (-70)
[ 5389.078492] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not
attach device
[ 5389.085780] sas: executing TMF task failed 5000c500ae49c8f1 (-70)
[ 5389.091861] hisi_sas_v3_hw 0000:74:02.0: task prep: SAS port1 not
attach device
[ 5389.099146] sas: executing TMF task failed 5000c500ae49c8f1 (-70)
[ 5389.107419] hisi_sas_v3_hw 0000:74:02.0: controller reset complete
// controller reset finished
[ 5389.113686] hisi_sas_v3_hw 0000:74:02.0: phydown: phy0 phy_state=0xfe
[ 5389.120099] hisi_sas_v3_hw 0000:74:02.0: ignore flutter phy0 down
[ 5389.136399] hisi_sas_v3_hw 0000:74:02.0: phy3's hw port id changed
from 1 to 7
[ 5389.308114] hisi_sas_v3_hw 0000:74:02.0: phyup: phy0 link_rate=9(sata)

Thanks,
Xingui