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

[John Garry]

On 15/05/2026 09:45, 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.
>
> Add validation checks after sas_ex_phy_discover() to detect linkrate and
> sas_addr changes. When changes are detected, mark the device as gone and
> schedule rediscovery via libsas's async discovery pattern:
> - Set phy_change_count and ex_change_count to -1 to force revalidation
> - Unregister the device and schedule DISCE_REVALIDATE_DOMAIN event
> - The old device is destroyed by sas_destruct_devices()
> - New event triggers discovery via sas_discover_new() since
>    attached_sas_addr is cleared
>
> Suggested-by: John Garry <john.g.garry@xxxxxxxxxx>
> Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
> ---
>   drivers/scsi/libsas/sas_expander.c | 32 ++++++++++++++++++++++++++----
>   1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index f55ae9a979cd..720db4128727 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -2017,15 +2017,39 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
>   		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);
> +		struct domain_device *child_dev = sas_ex_to_dev(dev, phy_id);
> +		bool need_rediscover = false;
>   		char *action = "";
>   
>   		sas_ex_phy_discover(dev, phy_id);
>   
> -		if (ata_dev && phy->attached_dev_type == SAS_SATA_PENDING)
> +		if (child_dev && dev_is_sata(child_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);
> +		} else if (child_dev && phy->linkrate != child_dev->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);
> +			need_rediscover = true;
> +		} else if (child_dev &&
> +			   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));
> +			need_rediscover = true;
> +		}
> +
> +		if (need_rediscover) {
> +			set_bit(SAS_DEV_GONE, &child_dev->state);
> +			phy->phy_change_count = -1;
> +			ex->ex_change_count = -1;

the current code has following:


	/* we always have to delete the old device when we went here */
	pr_info("ex %016llx phy%02d replace %016llx\n",
		SAS_ADDR(dev->sas_addr), phy_id,
		SAS_ADDR(phy->attached_sas_addr));
	sas_unregister_devs_sas_addr(dev, phy_id, last);

	res = sas_discover_new(dev, phy_id);

Can this be reused (to lose and find the device with updated info)? Or 
why not good enough?

I don't know why you need full revalidation.

> +			sas_unregister_devs_sas_addr(dev, phy_id, true);
> +			sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
> +		} else {
> +			pr_debug("ex %016llx phy%02d broadcast flutter%s\n",
> +				 SAS_ADDR(dev->sas_addr), phy_id, action);
> +		}
>   		goto out_free_resp;
>   	}
>