[PATCH v3 0/2] libsas: rediscover improvements for linkrate/sas_addr changes

[PATCH v3 0/2] libsas: rediscover improvements for linkrate/sas_addr changes

[Xingui Yang]

When a device attached to an expander phy experiences a linkrate change
(e.g., due to cable reconnection or negotiation), the current code in
sas_rediscover_dev() treats it as "broadcast flutter" and takes no action
if the SAS address and device type remain unchanged.

This series is a new version based on John Garry's suggestion [1] to
check the linkrate and mark the device as gone and rediscover when
flutter occurs, replacing the previous v2 patch series that used lldd
callbacks.

The previous v2 approach added lldd_dev_info_update callback which John
commented as "seem fragile and too specialized" [2]. This v3 series adopts
a simpler approach that directly checks linkrate/sas_addr changes in
sas_rediscover_dev() and triggers rediscovery using libsas's standard
async discovery pattern.

This aligns with Jason Yan's earlier work [3] which was verified to
solve the linkrate change issue.

Changes from v2:
- Drop lldd_dev_info_update callback approach per John Garry's suggestion
- Drop hisi_sas specific changes (no longer needed without callback)
- Use libsas's async discovery pattern for rediscovery
- Add sas_addr change detection alongside linkrate change

[1] https://lore.kernel.org/linux-scsi/c4e4c99f-a13c-4e28-8650-48be1f96d7cf@oracle.com/
[2] https://lore.kernel.org/linux-scsi/28bd9d5b-f597-0aae-5340-bd951b2083aa@huawei.com/
[3] https://lore.kernel.org/linux-scsi/20190130082412.9357-6-yanaijie@huawei.com/

Your Name (2):
  scsi: libsas: refactor sas_ex_to_ata() using new helper
    sas_ex_to_dev()
  scsi: libsas: Add linkrate and sas_addr change detection in rediscover

 drivers/scsi/libsas/sas_expander.c | 44 ++++++++++++++++++++++++------
 drivers/scsi/libsas/sas_internal.h |  1 +
 2 files changed, 37 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] scsi: libsas: refactor sas_ex_to_ata() using new helper sas_ex_to_dev()

[Xingui Yang]

The sas_ex_to_ata() function checks for an attached ATA device on an
expander phy. Refactor it to use a new helper function sas_ex_to_dev()
which returns any device type attached to an expander phy, improving code
reuse and allowing other code paths to find attached devices regardless
of type.

No functional changes intended.

Reviewed-by: Jason Yan <yanaijie@huawei.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 12 ++++++++----
 drivers/scsi/libsas/sas_internal.h |  1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index f471ab464a78..f55ae9a979cd 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -345,11 +345,9 @@ static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 		 SAS_ADDR(phy->attached_sas_addr), type);
 }
 
-/* check if we have an existing attached ata device on this expander phy */
-struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
+struct domain_device *sas_ex_to_dev(struct domain_device *ex_dev, int phy_id)
 {
 	struct ex_phy *ex_phy = &ex_dev->ex_dev.ex_phy[phy_id];
-	struct domain_device *dev;
 	struct sas_rphy *rphy;
 
 	if (!ex_phy->port)
@@ -359,7 +357,13 @@ struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
 	if (!rphy)
 		return NULL;
 
-	dev = sas_find_dev_by_rphy(rphy);
+	return sas_find_dev_by_rphy(rphy);
+}
+
+/* check if we have an existing attached ata device on this expander phy */
+struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id)
+{
+	struct domain_device *dev = sas_ex_to_dev(ex_dev, phy_id);
 
 	if (dev && dev_is_sata(dev))
 		return dev;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 7dce0f587149..350a70484bde 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -91,6 +91,7 @@ int sas_smp_get_phy_events(struct sas_phy *phy);
 
 void sas_device_set_phy(struct domain_device *dev, struct sas_port *port);
 struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
+struct domain_device *sas_ex_to_dev(struct domain_device *ex_dev, int phy_id);
 struct domain_device *sas_ex_to_ata(struct domain_device *ex_dev, int phy_id);
 int sas_ex_phy_discover(struct domain_device *dev, int single);
 int sas_get_report_phy_sata(struct domain_device *dev, int phy_id,
-- 
2.43.0

Re: [PATCH v3 1/2] scsi: libsas: refactor sas_ex_to_ata() using new helper sas_ex_to_dev()

[John Garry]

On 15/05/2026 09:45, Xingui Yang wrote:
> The sas_ex_to_ata() function checks for an attached ATA device on an
> expander phy. Refactor it to use a new helper function sas_ex_to_dev()
> which returns any device type attached to an expander phy, improving code
> reuse 
I think you mean that later we can improve code reuse. You imply that 
code reuse improves now in this change.

> and allowing other code paths to find attached devices regardless
> of type.
> 
> No functional changes intended.
> 
> Reviewed-by: Jason Yan<yanaijie@huawei.com>
> Signed-off-by: Xingui Yang<yangxingui@huawei.com>

Reviewed-by: John Garry <john.g.garry@oracle.com>

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

[Xingui Yang]

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@oracle.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 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;
+			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;
 	}
 
-- 
2.43.0

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@oracle.com>
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
>   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;
>   	}
>   

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

[yangxingui]

On 2026/5/21 16:13, John Garry wrote:
> 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@oracle.com>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
>> ---
>>   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.

Hi, John
As the commit log. The existing pattern (unregister + sas_discover_new) 
handles the "replace" case where the SAS address changes completely, 
implying a different device.
For the flutter case where we detect linkrate/sas_addr changes, we 
cannot reuse this synchronous pattern because:
sas_unregister_devs_sas_addr() only marks the device as gone (sets 
SAS_DEV_GONE) and adds it to port->destroy_list. The actual sysfs 
cleanup (sas_rphy_delete) happens later in sas_destruct_devices() called 
at the end of sas_revalidate_domain() in sas_discover.c.
I did try the approach you suggested (unregister + sas_discover_new), 
but it produces sysfs duplicate directory errors:
Workqueue: 0000:74:02.0_disco_q sas_revalidate_domain
Call trace:
  dump_backtrace+0x0/0x18c
  show_stack+0x14/0x1c
  dump_stack+0x88/0xac
  sysfs_warn_dup+0x64/0x7c
  sysfs_create_dir_ns+0x90/0xa0
  kobject_add_internal+0xa0/0x284
  kobject_add+0xb8/0x11c
  device_add+0xe8/0x598
  sas_port_add+0x24/0x50
  sas_ex_discover_devices+0xb10/0xc30

The async pattern with DISCE_REVALIDATE_DOMAIN ensures proper ordering:
1. Old device added to destroy_list
2. Current revalidate work completes → sas_destruct_devices() truly 
deletes old device's sysfs
3. New DISCE_REVALIDATE_DOMAIN event triggers → discovery starts with 
clean sysfs state
This follows libsas's async design pattern similar to sas_resume_devices 
in sas_port.c.

Thanks,
Xingui

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

[John Garry]

On 21/05/2026 10:04, yangxingui wrote:
>>
>> 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.
> 
> Hi, John
> As the commit log. The existing pattern (unregister + sas_discover_new) 
> handles the "replace" case where the SAS address changes completely, 
> implying a different device.
> For the flutter case where we detect linkrate/sas_addr changes,

Can you please clarify this: you say that the existing pattern handles 
"replace" case where the SAS address changes completely, and then 
flutter case covers sas_addr changes.

What is the difference in the SAS address changes between the two cases?


  we
> cannot reuse this synchronous pattern because:
> sas_unregister_devs_sas_addr() only marks the device as gone (sets 
> SAS_DEV_GONE) and adds it to port->destroy_list. The actual sysfs 
> cleanup (sas_rphy_delete) happens later in sas_destruct_devices() called 
> at the end of sas_revalidate_domain() in sas_discover.c.
> I did try the approach you suggested (unregister + sas_discover_new), 
> but it produces sysfs duplicate directory errors:
> Workqueue: 0000:74:02.0_disco_q sas_revalidate_domain
> Call trace:
>   dump_backtrace+0x0/0x18c
>   show_stack+0x14/0x1c
>   dump_stack+0x88/0xac
>   sysfs_warn_dup+0x64/0x7c
>   sysfs_create_dir_ns+0x90/0xa0
>   kobject_add_internal+0xa0/0x284
>   kobject_add+0xb8/0x11c
>   device_add+0xe8/0x598
>   sas_port_add+0x24/0x50
>   sas_ex_discover_devices+0xb10/0xc30
> 
> The async pattern with DISCE_REVALIDATE_DOMAIN ensures proper ordering:
> 1. Old device added to destroy_list
> 2. Current revalidate work completes → sas_destruct_devices() truly 
> deletes old device's sysfs
> 3. New DISCE_REVALIDATE_DOMAIN event triggers → discovery starts with 
> clean sysfs state
> This follows libsas's async design pattern similar to sas_resume_devices 
> in sas_port.c.

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

[yangxingui]

On 2026/5/22 17:16, John Garry wrote:
> On 21/05/2026 10:04, yangxingui wrote:
>>>
>>> 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.
>>
>> Hi, John
>> As the commit log. The existing pattern (unregister + 
>> sas_discover_new) handles the "replace" case where the SAS address 
>> changes completely, implying a different device.
>> For the flutter case where we detect linkrate/sas_addr changes,
> 
> Can you please clarify this: you say that the existing pattern handles 
> "replace" case where the SAS address changes completely, and then 
> flutter case covers sas_addr changes.
> 
> What is the difference in the SAS address changes between the two cases?
> 
Hi, John

The difference is in when the SAS address change is detected:
Replace case:
- Detected immediately by the initial SMP DISCOVER response
- New SAS address differs from stored phy->attached_sas_addr

Flutter case:
- Initial SMP DISCOVER shows SAS address matching stored 
phy->attached_sas_addr
- Linkrate may change
- After sas_ex_phy_discover() refreshes phy info, child device address 
and linkrate may mismatched with refreshed phy info

Additional issue with Replace flow:
The existing replace code path also suffers from the same sysfs_warn_dup 
issue I mentioned earlier. sas_unregister_devs_sas_addr() only marks the 
device as gone and adds it to destroy_list. The actual sysfs cleanup 
happens later in sas_destruct_devices(). Calling sas_discover_new() 
immediately after unregister causes sysfs duplicate directory errors.

We need to optimize the replace process.

Thanks,
Xingui

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

[John Garry]

On 25/05/2026 03:25, yangxingui wrote:
>>>
>>> Hi, John
>>> As the commit log. The existing pattern (unregister + 
>>> sas_discover_new) handles the "replace" case where the SAS address 
>>> changes completely, implying a different device.
>>> For the flutter case where we detect linkrate/sas_addr changes,
>>
>> Can you please clarify this: you say that the existing pattern handles 
>> "replace" case where the SAS address changes completely, and then 
>> flutter case covers sas_addr changes.
>>
>> What is the difference in the SAS address changes between the two cases?
>>
> Hi, John
> 
> The difference is in when the SAS address change is detected:
> Replace case:
> - Detected immediately by the initial SMP DISCOVER response
> - New SAS address differs from stored phy->attached_sas_addr
> 
> Flutter case:
> - Initial SMP DISCOVER shows SAS address matching stored phy- 
>  >attached_sas_addr
> - Linkrate may change
> - After sas_ex_phy_discover() refreshes phy info, child device address 
> and linkrate may mismatched with refreshed phy info
> 
> Additional issue with Replace flow:
> The existing replace code path also suffers from the same sysfs_warn_dup 
> issue I mentioned earlier. sas_unregister_devs_sas_addr() only marks the 
> device as gone and adds it to destroy_list. The actual sysfs cleanup 
> happens later in sas_destruct_devices(). Calling sas_discover_new() 
> immediately after unregister causes sysfs duplicate directory errors.
> 

So can you actually recreate this issue? Or is it just theoretical?

About this following code:

+		if (need_rediscover) {
+			set_bit(SAS_DEV_GONE, &child_dev->state);
+			phy->phy_change_count = -1;
+			ex->ex_change_count = -1;
+			sas_unregister_devs_sas_addr(dev, phy_id, true);
+			sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
+		} else {


Can we factor it out with other code? AFAICR, this can pattern can be 
seen elsewhere.

Thanks

> We need to optimize the replace process.

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

[yangxingui]

On 2026/5/27 17:42, John Garry wrote:
> On 25/05/2026 03:25, yangxingui wrote:
>>>>
>>>> Hi, John
>>>> As the commit log. The existing pattern (unregister + 
>>>> sas_discover_new) handles the "replace" case where the SAS address 
>>>> changes completely, implying a different device.
>>>> For the flutter case where we detect linkrate/sas_addr changes,
>>>
>>> Can you please clarify this: you say that the existing pattern 
>>> handles "replace" case where the SAS address changes completely, and 
>>> then flutter case covers sas_addr changes.
>>>
>>> What is the difference in the SAS address changes between the two cases?
>>>
>> Hi, John
>>
>> The difference is in when the SAS address change is detected:
>> Replace case:
>> - Detected immediately by the initial SMP DISCOVER response
>> - New SAS address differs from stored phy->attached_sas_addr
>>
>> Flutter case:
>> - Initial SMP DISCOVER shows SAS address matching stored phy- 
>>  >attached_sas_addr
>> - Linkrate may change
>> - After sas_ex_phy_discover() refreshes phy info, child device address 
>> and linkrate may mismatched with refreshed phy info
>>
>> Additional issue with Replace flow:
>> The existing replace code path also suffers from the same 
>> sysfs_warn_dup issue I mentioned earlier. 
>> sas_unregister_devs_sas_addr() only marks the device as gone and adds 
>> it to destroy_list. The actual sysfs cleanup happens later in 
>> sas_destruct_devices(). Calling sas_discover_new() immediately after 
>> unregister causes sysfs duplicate directory errors.
>>
> 
> So can you actually recreate this issue? Or is it just theoretical?
Yes, it can recreate, such as quickly replacing a disk or swapping two 
disk. However, during normal use, we recommend that customers wait for 
one minute before inserting a new disk when replacing a disk to avoid 
this issue.

The following are example of similar exception logs:
log1：

[ 7957.283194][T67794] sas: ex 500e004aaaaaaa1f phy19 replace 
5001882016000000
[ 7957.290960][T67794] sysfs: cannot create duplicate filename 
'/devices/pci0000:74/0000:74:02.0/host1/port-1:2/expander-1:2/port-1:2:19'
[ 7957.317973][T67794] Hardware name: Huawei Technologies Co., Ltd. 
EVBCS/EVBCS, BIOS CS B055 2P TA 02/19/2020
[ 7957.328900][T67794] Workqueue: 0000:74:02.0_disco_q 
sas_revalidate_domain [libsas]
[ 7957.337636][T67794] Call trace:
[ 7957.341419][T67794]  dump_backtrace+0x0/0x200
[ 7957.346317][T67794]  show_stack+0x20/0x30
[ 7957.350887][T67794]  dump_stack+0xf0/0x138
[ 7957.355511][T67794]  sysfs_warn_dup+0x6c/0x90
[ 7957.360410][T67794]  sysfs_create_dir_ns+0xf8/0x11c
[ 7957.365795][T67794]  create_dir+0x30/0x18c
[ 7957.370416][T67794]  kobject_add_internal+0x5c/0x190
[ 7957.375877][T67794]  kobject_add+0x98/0x110
[ 7957.380566][T67794]  device_add+0x100/0x4a0
[ 7957.385255][T67794]  sas_port_add+0x30/0x74 [scsi_transport_sas]
[ 7957.391722][T67794]  sas_ex_discover_end_dev+0x328/0x630 [libsas]
[ 7957.398328][T67794]  sas_ex_discover_dev+0x230/0x320 [libsas]
[ 7957.404599][T67794]  sas_ex_discover_devices+0x60/0x100 [libsas]
[ 7957.411116][T67794]  sas_discover_new+0xa4/0x150 [libsas]
[ 7957.416986][T67794]  sas_rediscover_dev+0x198/0x260 [libsas]
[ 7957.423125][T67794]  sas_rediscover+0xb8/0x100 [libsas]
[ 7957.428854][T67794]  sas_ex_revalidate_domain+0x5c/0xe0 [libsas]
[ 7957.435320][T67794]  sas_revalidate_domain+0x1ac/0x1b4 [libsas]
[ 7957.441714][T67794]  process_one_work+0x1d8/0x4cc
[ 7957.446884][T67794]  worker_thread+0x158/0x410
[ 7957.451769][T67794]  kthread+0x108/0x13c
[ 7957.456089][T67794]  ret_from_fork+0x10/0x18
[ 7957.460800][T67794] kobject_add_internal failed for port-1:2:19 with 
-EEXIST, don't try to register things with the same name in the same 
directory.
[ 7957.474864][T67794] sas: ex 500e004aaaaaaa1f phy19 failed to discover

log2：
[ 8608.766031][T66335] sas: ex 500e004aaaaaaa1f phy17 change count has 
changed
[ 8608.774390][T66335] sysfs: cannot create duplicate filename 
'/devices/pci0000:74/0000:74:02.0/host1/port-1:2/expander-1:3/port-1:3:17'
[ 8608.800667][T66335] Hardware name: Huawei Technologies Co., Ltd. 
EVBCS/EVBCS, BIOS CS B055 2P TA 02/19/2020
[ 8608.811270][T66335] Workqueue: 0000:74:02.0_disco_q 
sas_revalidate_domain [libsas]
[ 8608.819685][T66335] Call trace:
[ 8608.823235][T66335]  dump_backtrace+0x0/0x200
[ 8608.827990][T66335]  show_stack+0x20/0x30
[ 8608.832387][T66335]  dump_stack+0xf0/0x138
[ 8608.836858][T66335]  sysfs_warn_dup+0x6c/0x90
[ 8608.841580][T66335]  sysfs_create_dir_ns+0xf8/0x11c
[ 8608.846814][T66335]  create_dir+0x30/0x18c
[ 8608.851257][T66335]  kobject_add_internal+0x5c/0x190
[ 8608.856566][T66335]  kobject_add+0x98/0x110
[ 8608.861083][T66335]  device_add+0x100/0x4a0
[ 8608.865588][T66335]  sas_port_add+0x30/0x74 [scsi_transport_sas]
[ 8608.871919][T66335]  sas_ex_discover_end_dev+0x328/0x630 [libsas]
[ 8608.878345][T66335]  sas_ex_discover_dev+0x230/0x320 [libsas]
[ 8608.884430][T66335]  sas_ex_discover_devices+0x60/0x100 [libsas]
[ 8608.890768][T66335]  sas_discover_new+0xa4/0x150 [libsas]
[ 8608.896506][T66335]  sas_rediscover+0xd8/0x100 [libsas]
[ 8608.902075][T66335]  sas_ex_revalidate_domain+0x5c/0xe0 [libsas]
[ 8608.908438][T66335]  sas_revalidate_domain+0x1ac/0x1b4 [libsas]
[ 8608.914708][T66335]  process_one_work+0x1d8/0x4cc
[ 8608.919757][T66335]  worker_thread+0x158/0x410
[ 8608.924533][T66335]  kthread+0x108/0x13c
[ 8608.928778][T66335]  ret_from_fork+0x10/0x18
[ 8608.933377][T66335] kobject_add_internal failed for port-1:3:17 with 
-EEXIST, don't try to register things with the same name in the same 
directory.
[ 8608.947364][T66335] sas: ex 500e004aaaaaaa1f phy17 failed to discover


> About this following code:
> 
> +        if (need_rediscover) {
> +            set_bit(SAS_DEV_GONE, &child_dev->state);
> +            phy->phy_change_count = -1;
> +            ex->ex_change_count = -1;
> +            sas_unregister_devs_sas_addr(dev, phy_id, true);
> +            sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
> +        } else {
> 
> 
> Can we factor it out with other code? AFAICR, this can pattern can be 
> seen elsewhere.

Yes, in the updated v4 version, I refined it into a function.

In the v4 test log, the disk is gone and then connected again.
test case:
phy=phy-10:0:0
echo 1.5 Gbit > /sys/class/sas_phy/$phy/maximum_linkrate

The details are as follows:

[  541.880156] sas: broadcast received: 0
[  541.880712] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.882509] sas: ex 500e004aaaaaaa1f phy00 change count has changed
[  541.883680] sas: ex 500e004aaaaaaa1f phy00 originated BROADCAST(CHANGE)
[  541.883703] sas: ex 500e004aaaaaaa1f rediscovering phy00
[  541.884838] sas: ex 500e004aaaaaaa1f phy00:U:8 attached: 
500e004aaaaaaa00 (stp pending)
[  541.884844] sas: ex 500e004aaaaaaa1f phy00 broadcast flutter, needs 
recovery
[  541.898893] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.899444] sas: broadcast received: 0
[  541.899506] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.899659] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.900213] sas: broadcast received: 0
[  541.900282] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.900367] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.900992] sas: broadcast received: 0
[  541.901041] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.901132] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.901160] sas: broadcast received: 0
[  541.901195] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.901278] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.901887] sas: broadcast received: 0
[  541.901948] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.902035] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.902673] sas: broadcast received: 0
[  541.902741] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.902836] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.903467] sas: broadcast received: 0
[  541.903536] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.903625] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  541.919967] sas: broadcast received: 0
[  541.920036] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  541.920385] sas: ex 500e004aaaaaaa1f phy00 change count has changed
[  541.920494] sas: ex 500e004aaaaaaa1f phy00 originated BROADCAST(CHANGE)
[  541.920504] sas: ex 500e004aaaaaaa1f rediscovering phy00
[  541.920687] sas: ex 500e004aaaaaaa1f phy00:U:8 attached: 
500e004aaaaaaa00 (stp)
[  541.920697] sas: ex 500e004aaaaaaa1f phy00 linkrate changed from 9 to 8
[  541.920710] sas: ex 500e004aaaaaaa1f phy00 replace 500e004aaaaaaa00
[  541.922648] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  542.007031] sd 10:0:0:0: [sdd] Stopping disk
[  542.007122] sd 10:0:0:0: [sdd] Start/Stop Unit failed: Result: 
hostbyte=0x04 driverbyte=DRIVER_OK
[  542.027347] hisi_sas_v3_hw 0000:70:04.0: dev[2:5] is gone
[  542.039000] sas: REVALIDATING DOMAIN on port 0, pid:8995
[  542.039358] sas: ex 500e004aaaaaaa1f phy00 change count has changed
[  542.039436] sas: broadcast received: 0
[  542.039470] sas: ex 500e004aaaaaaa1f phy00 originated BROADCAST(CHANGE)
[  542.039482] sas: ex 500e004aaaaaaa1f phy00 new device attached
[  542.039569] sas: ex 500e004aaaaaaa1f phy00:U:8 attached: 
500e004aaaaaaa00 (stp)
[  542.040152] hisi_sas_v3_hw 0000:70:04.0: dev[15:5] found
[  542.042112] sas: done REVALIDATING DOMAIN on port 0, pid:8995, res 0x0
[  542.153137] sas: Enter sas_scsi_recover_host busy: 0 failed: 0
[  542.153900] sas: ata10: end_device-10:0:1: dev error handler
[  542.153972] sas: ata11: end_device-10:0:2: dev error handler
[  542.153994] sas: ata12: end_device-10:0:5: dev error handler
[  542.154010] sas: ata13: end_device-10:0:6: dev error handler
[  542.154021] sas: ata14: end_device-10:0:9: dev error handler
[  542.154032] sas: ata15: end_device-10:0:10: dev error handler
[  542.154044] sas: ata9: end_device-10:0:0: dev error handler
[  542.519243] ata9.00: ATA-10: ST4000NM0035-1V4107, TN03, max UDMA/133
[  542.522538] ata9.00: 7814037168 sectors, multi 0: LBA48 NCQ (depth 32)
[  542.522561] ata9.00: Features: DIPM NCQ-sndrcv
[  542.527315] ata9.00: configured for UDMA/133
[  542.527976] sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 0 
tries: 1
[  542.570193] scsi 10:0:13:0: Direct-Access     ATA 
ST4000NM0035-1V4 TN03 PQ: 0 ANSI: 5
[  542.571771] scsi 10:0:13:0: Attached scsi generic sg3 type 0


Thanks,
Xingui