Re: [PATCH v10 2/2] scsi: libsas: Add linkrate and sas_addr change detection in rediscover

On 2026/8/14 14:33, Jason Yan wrote:
> 在 2026/8/11 12:03, Xingui Yang 写道:
>> Introduce sas_dev_is_flutter() and sas_rediscover_ex_phy() to improve
>> flutter and device replace detection during rediscovery.
>>
>> sas_dev_is_flutter() calls sas_ex_phy_discover() before looking up the
>> child device via sas_ex_phy_to_dev(), ensuring the PHY state is always
>> updated and avoiding use-after-free since the child device pointer is
>> obtained after the sleeping SMP request completes.
>>
>> It adds validation for linkrate and sas_addr changes. When the SAS
>> address changes, phy->attached_sas_addr is restored to the original
>> address before returning false, so sas_unregister_devs_sas_addr() can
>> properly match and unregister the old device. The sas_addr check is
>> ordered before the linkrate check to avoid skipping the restoration
>> when both change simultaneously.
>>
>> sas_rediscover_ex_phy() uses the async discovery pattern
>> (sas_discover_event) instead of the synchronous sas_discover_new() to
>> ensure proper ordering between device unregistration and rediscovery,
>> avoiding sysfs_warn_dup() errors.
>>
>> Signed-off-by: Xingui Yang <[email protected]>
>> Suggested-by: John Garry <[email protected]>
>> ---
>>   drivers/scsi/libsas/sas_expander.c | 83 +++++++++++++++++++++++++-----
>>   1 file changed, 69 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c 
>> b/drivers/scsi/libsas/sas_expander.c
>> index a5c5327cd0dd..811c9eb4fef1 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -1963,6 +1963,72 @@ static bool dev_type_flutter(enum 
>> sas_device_type new, enum sas_device_type old)
>>       return false;
>>   }
>> +static void sas_rediscover_ex_phy(struct domain_device *dev, int phy_id,
>> +                  bool last)
>> +{
>> +    struct expander_device *ex = &dev->ex_dev;
>> +    struct ex_phy *phy = &ex->ex_phy[phy_id];
>> +
>> +    phy->phy_change_count = -1;
>> +    ex->ex_change_count = -1;
>> +    sas_unregister_devs_sas_addr(dev, phy_id, last);
>> +    sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
>> +}
>> +
>> +static bool sas_dev_is_flutter(struct domain_device *dev, int phy_id,
>> +                   u8 *sas_addr, enum sas_device_type type)
>> +{
>> +    struct expander_device *ex = &dev->ex_dev;
>> +    struct ex_phy *phy = &ex->ex_phy[phy_id];
>> +    struct domain_device *child_dev;
>> +    char *action = "";
>> +    int res;
>> +
>> +    if (SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr) ||
>> +        !dev_type_flutter(type, phy->attached_dev_type))
>> +        return false;
>> +
>> +    res = sas_ex_phy_discover(dev, phy_id);
>> +    if (res)
>> +        return false;
>> +
>> +    child_dev = sas_ex_phy_to_dev(dev, phy_id);
>> +    if (!child_dev)
>> +        goto out;
>> +
>> +    if (dev_is_sata(child_dev) &&
>> +        phy->attached_dev_type == SAS_SATA_PENDING) {
>> +        action = ", needs recovery";
>> +        goto out;
>> +    }
>> +
>> +    if (SAS_ADDR(child_dev->sas_addr) != 
>> SAS_ADDR(phy->attached_sas_addr)) {
>> +        pr_info("ex %016llx phy%02d sas_addr changed from %016llx to 
>> %016llx\n",
>> +            SAS_ADDR(dev->sas_addr), phy_id,
>> +            SAS_ADDR(child_dev->sas_addr),
>> +            SAS_ADDR(phy->attached_sas_addr));
>> +        /*
>> +         * Device unregistering relies on address matching. Restore
>> +         * attached_sas_addr back to the original address so that the 
>> old
>> +         * device can be unregistered later
>> +         */
>> +        memcpy(phy->attached_sas_addr, child_dev->sas_addr, 
>> SAS_ADDR_SIZE);
>> +        return false;
>> +    }
>> +
>> +    if (child_dev->linkrate != phy->linkrate) {
>> +        pr_info("ex %016llx phy%02d linkrate changed from %d to %d\n",
>> +            SAS_ADDR(dev->sas_addr), phy_id,
>> +            child_dev->linkrate, phy->linkrate);
>> +        return false;
>> +    }
>> +
>> +out:
>> +    pr_debug("ex %016llx phy%02d broadcast flutter%s\n",
>> +         SAS_ADDR(dev->sas_addr), phy_id, action);
>> +    return true;
>> +}
>> +
>>   static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
>>                     bool last, int sibling)
>>   {
>> @@ -2016,27 +2082,16 @@ static int sas_rediscover_dev(struct 
>> domain_device *dev, int phy_id,
>>           if (res == 0)
>>               sas_set_ex_phy(dev, phy_id, disc_resp);
>>           goto out_free_resp;
>> -    } else if (SAS_ADDR(sas_addr) == SAS_ADDR(phy->attached_sas_addr) &&
>> -           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);
>> +    if (sas_dev_is_flutter(dev, phy_id, sas_addr, type))
>>           goto out_free_resp;
>> -    }
>>       /* we always have to delete the old device when we went here */
> 
> Please remove this comment since you are not deleting the old device now.

Hi, Jason

We will call sas_rediscover_ex_phy(dev, phy_id, last) later to delete 
the old device, and rediscover the device.

Thanks,
Xingui