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

[yangxingui <yangxingui@huawei.com>]

On 2026/6/3 0:30, John Garry wrote:
> On 30/05/2026 03:49, Xingui Yang wrote:
>> In sas_rediscover_dev(), when detecting a "flutter" condition (same SAS
>> address and compatible device type), the code assumes the device remains
>> unchanged and only handles SATA pending state recovery. However, this
>> approach misses two important scenarios:
>>
>> First, the flutter detection only compares SAS address and device type,
>> ignoring potential linkrate changes that may have already occurred.
>>
>> Second, after sas_ex_phy_discover() re-queries the expander phy, both
>> linkrate and attached SAS address may be updated. The current code does
>> not validate these changes against the existing child device.
>>
>> Additionally, the replace code path (different SAS address detected)
>> has a sysfs duplication issue: sas_unregister_devs_sas_addr() only marks
>> the device as gone, but the actual sysfs cleanup happens later in
>> sas_destruct_devices(). Calling sas_discover_new() immediately after
>> unregister causes sysfs_warn_dup() errors.
>>
>> Introduce sas_dev_is_flutter() to check whether it is a true flutter with
>> validation for linkrate and sas_addr changes. It returns true for normal
>> flutter and false when changes are detected requiring rediscovery.
>>
>> Introduce sas_rediscover_ex_phy() to handle async rediscovery for both
>> flutter and replace cases. When invoked:
>> - Set phy_change_count and ex_change_count to -1 to force revalidation
>> - Unregister the device via sas_unregister_devs_sas_addr()
>> - Queue DISCE_REVALIDATE_DOMAIN event
>>
>> The old device sysfs is cleaned up by sas_destruct_devices() at the end
>> of current revalidation work. The new event triggers discovery via
>> sas_discover_new() since attached_sas_addr is cleared, avoiding the
>> sysfs duplication issue.
>>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> Suggested-by: John Garry <john.g.garry@oracle.com>
> 
> This looks ok, so:
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>

Hi, John

Thank you for your review!

After further analysis, I found a small issue in the sas_addr change
handling that needs a minor fix:

When sas_addr change is detected in sas_dev_is_flutter(), after
sas_ex_phy_discover() updates phy->attached_sas_addr to the new address,
subsequent sas_unregister_devs_sas_addr() cannot properly match the
device because sas_phy_match_dev_addr() compares phy->attached_sas_addr
with child_dev->sas_addr, which would mismatch.

So I added a memcpy() to restore phy->attached_sas_addr to
child_dev->sas_addr before returning false, ensuring proper device
unregistration:

     memcpy(phy->attached_sas_addr, child_dev->sas_addr,
            SAS_ADDR_SIZE);

This change is included in v6. Would you mind taking another
look when you have time?

Thanks,
Xingui
.