Re: [PATCH] scsi: libsas: Fix disk not being scanned in after being removed

From: John Garry
Date: Wed Mar 06 2024 - 12:58:29 EST


On 05/03/2024 11:25, yangxingui wrote:

It's like the change in this patch.
This doesn't work properly. the previous sas address will be compared with the currently queried sas address and the previous phy information will also be used when calling sas_unregister_devs_sas_addr() after the sas_rediscover_dev() function calls sas_get_phy_attached_dev(). Therefore, it is more appropriate to update the phy information after the device is unregistered.

ok, fine

as follows:
static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
                              bool last, int sibling)
{
    ...
       res = sas_get_phy_attached_dev(dev, phy_id, sas_addr, &type);
        switch (res) {
        case SMP_RESP_NO_PHY:
                phy->phy_state = PHY_NOT_PRESENT;
                sas_unregister_devs_sas_addr(dev, phy_id, last);
                return res;
        case SMP_RESP_PHY_VACANT:
                phy->phy_state = PHY_VACANT;
                sas_unregister_devs_sas_addr(dev, phy_id, last);
                return res;
        case SMP_RESP_FUNC_ACC:
                break;
        case -ECOMM:
                break;
        default:
                return res;
        }

        if ((SAS_ADDR(sas_addr) == 0) || (res == -ECOMM)) {
                phy->phy_state = PHY_EMPTY;
                sas_unregister_devs_sas_addr(dev, phy_id, last);
                /*
                 * Even though the PHY is empty, for convenience we discover
                 * the PHY to update the PHY info, like negotiated linkrate.
                 */
                sas_ex_phy_discover(dev, phy_id);
                return res;
        } else if (SAS_ADDR(sas_addr) == SAS_ADDR(phy->attached_sas_addr) && // <=== Compare the previous sas address with the current sas address
                   dev_type_flutter(type, phy->attached_dev_type)) {
                struct domain_device *ata_dev = sas_ex_to_ata(dev, phy_id);
                char *action = "";

                sas_ex_phy_discover(dev, phy_id);

                if (ata_dev && phy->attached_dev_type == SAS_SATA_PENDING)
                        action = ", needs recovery";
                pr_debug("ex %016llx phy%02d broadcast flutter%s\n",
                         SAS_ADDR(dev->sas_addr), phy_id, action);
                return res;
        }




OK, so let me have a closer look at v2.

I have to say that v2 is quite complicated...
Yes, but it works.

I'll check it again.

Thanks,
John