On 16/11/2021 03:10, chenxiang wrote:
From: Xiang Chen chenxiang66@hisilicon.com
Need to add spin_lock/unlock when accessing asd_sas_port->phy_list
Please say why
, so add them in function hisi_sas_refresh_port_id(). But it has a risk that list mutates while dropping the lock
nit: it would be nice to mention why we drop the lock
at the same time in function hisi_sas_send_ata_reset_each_phy(), so read asd_sas_port->phy_mask instead of accessing asd_sas_port->phy_list to avoid the risk.
Signed-off-by: Xiang Chen chenxiang66@hisilicon.com
With issues addressed: Acked-by: John Garry john.garry@huawei.com
drivers/scsi/hisi_sas/hisi_sas_main.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index 305d6282845a..44c888e0afd7 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -1427,11 +1427,13 @@ static void hisi_sas_refresh_port_id(struct hisi_hba *hisi_hba) sas_port = device->port; port = to_hisi_sas_port(sas_port);
spin_lock(&sas_port->phy_list_lock);list_for_each_entry(sas_phy, &sas_port->phy_list, port_phy_el) if (state & BIT(sas_phy->id)) { phy = sas_phy->lldd_phy; break; }
spin_unlock(&sas_port->phy_list_lock);if (phy) { port->id = phy->port_id;
@@ -1508,22 +1510,25 @@ static void hisi_sas_send_ata_reset_each_phy(struct hisi_hba *hisi_hba, struct ata_link *link; u8 fis[20] = {0}; u32 state;
int i;
state = hisi_hba->hw->get_phys_state(hisi_hba);
- list_for_each_entry(sas_phy, &sas_port->phy_list, port_phy_el) {
for (i = 0; i < hisi_hba->n_phy; i++) { if (!(state & BIT(sas_phy->id))) continue;
if (!(sas_port->phy_mask & BIT(i)))continue;ata_for_each_link(link, ap, EDGE) { int pmp = sata_srst_pmp(link);
tmf_task.phy_id = sas_phy->id;
tmf_task.phy_id = i; hisi_sas_fill_ata_reset_cmd(link->device, 1, pmp, fis); rc = hisi_sas_exec_internal_tmf_task(device, fis, s, &tmf_task); if (rc != TMF_RESP_FUNC_COMPLETE) { dev_err(dev, "phy%d ata reset failed rc=%d\n",
sas_phy->id, rc);
i, rc); break; }