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

[yangxingui]

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@xxxxxxxxxx>
> > Suggested-by: John Garry <john.g.garry@xxxxxxxxxx>
>
> This looks ok, so:
>
> Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>

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
.