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

yangxingui <[email protected]>
Newsgroups org.kernel.vger.linux-scsi,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hi, John

Kindly ping for review...

On 2026/6/24 14:32, Xingui Yang wrote:
> 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 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 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.
> 
> Additionally, per the discussion in v3 [4], the existing replace code
> path also suffers from the same 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. This series also optimizes the replace path to use the async
> pattern, ensuring proper ordering for both flutter and replace cases.
> 
> Changes from v8:
> - In sas_dev_is_flutter(), call sas_ex_phy_discover() before
>    sas_ex_to_dev() to ensure PHY state is always updated and to avoid
>    use-after-free since the child device pointer is obtained after the
>    sleeping SMP request completes, eliminating the need for kref
> - Addressed Sashiko AI review [5][6] feedback on PHY discovery bypass
>    and TOCTOU concerns
> 
> Changes from v7:
> Addressed issues identified by Sashiko AI review [5][6]:
> - In sas_dev_is_flutter(), reorder sas_addr check before linkrate check
>    to ensure address restoration is not skipped when both change
>    simultaneously, preventing device leak
> - In sas_ex_to_dev(), add defensive NULL check for ex_dev to guard
>    against callers passing a NULL device
> 
> Not addressed (pre-existing subsystem design):
> - sas_find_dev_by_rphy() returns unreferenced pointer: subsystem-wide
>    pattern used by 10+ call sites, should be a separate patch
> - ex_phy->port TOCTOU: discovery path is serialized by disco_mutex,
>    no race occurs in practice
> 
> Changes from v6:
> - Add comment for restoring phy->attached_sas_addr to child_dev->sas_addr
> - Optimize the conditional structure in sas_dev_is_flutter()
> 
> Changes from v5:
> - In sas_addr change handling, restore phy->attached_sas_addr to
>    child_dev->sas_addr before returning false, ensuring
>    sas_unregister_devs_sas_addr() can properly match the device via
>    sas_phy_match_dev_addr() for correct device unregistration
> 
> Changes from v4:
> - Rename sas_rediscover_phy to sas_rediscover_ex_phy for consistency
>    with expander phy symbol naming convention
> - Rename sas_is_flutter to sas_dev_is_flutter per John's suggestion
> - Check return value of sas_ex_phy_discover() for errors
> - Factor out child_dev checks to improve code clarity
> 
> Changes from v3:
> - Also optimize the replace code path to use async discovery pattern
> - Introduce sas_is_flutter() and sas_rediscover_phy() helpers
>    to encapsulate the flutter handling logic and avoid function bloat
> - Fix replace code path sysfs duplication 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
> 
> Changes from v1:
> - Split into three patches
> 
> [1] https://lore.kernel.org/linux-scsi/[email protected]/
> [2] https://lore.kernel.org/linux-scsi/[email protected]/
> [3] https://lore.kernel.org/linux-scsi/[email protected]/
> [4] https://lore.kernel.org/linux-scsi/[email protected]/
> [5] https://lore.kernel.org/linux-scsi/[email protected]/
> [6] https://lore.kernel.org/linux-scsi/[email protected]/
> 
> 
> Xingui Yang (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 | 102 +++++++++++++++++++++++------
>   drivers/scsi/libsas/sas_internal.h |   1 +
>   2 files changed, 84 insertions(+), 19 deletions(-)
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.