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

[yangxingui]

On 2026/8/14 14:33, Jason Yan wrote:
> 在 2026/8/11 12:03, Xingui Yang 写道:
> > Introduce sas_dev_is_flutter() and sas_rediscover_ex_phy() to improve
> > flutter and device replace detection during rediscovery.
> >
> > sas_dev_is_flutter() calls sas_ex_phy_discover() before looking up the
> > child device via sas_ex_phy_to_dev(), ensuring the PHY state is always
> > updated and avoiding use-after-free since the child device pointer is
> > obtained after the sleeping SMP request completes.
> >
> > It adds validation for linkrate and sas_addr changes. When the SAS
> > address changes, phy->attached_sas_addr is restored to the original
> > address before returning false, so sas_unregister_devs_sas_addr() can
> > properly match and unregister the old device. The sas_addr check is
> > ordered before the linkrate check to avoid skipping the restoration
> > when both change simultaneously.
> >
> > sas_rediscover_ex_phy() uses the async discovery pattern
> > (sas_discover_event) instead of the synchronous sas_discover_new() to
> > ensure proper ordering between device unregistration and rediscovery,
> > avoiding sysfs_warn_dup() errors.
> >
> > Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
> > Suggested-by: John Garry <john.g.garry@xxxxxxxxxx>
> > ---
> >   drivers/scsi/libsas/sas_expander.c | 83 +++++++++++++++++++++++++-----
> >   1 file changed, 69 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/scsi/libsas/sas_expander.c 
> > b/drivers/scsi/libsas/sas_expander.c
> > index a5c5327cd0dd..811c9eb4fef1 100644
> > --- a/drivers/scsi/libsas/sas_expander.c
> > +++ b/drivers/scsi/libsas/sas_expander.c
> > @@ -1963,6 +1963,72 @@ static bool dev_type_flutter(enum 
> > sas_device_type new, enum sas_device_type old)
> >       return false;
> >   }
> > +static void sas_rediscover_ex_phy(struct domain_device *dev, int phy_id,
> > +                  bool last)
> > +{
> > +    struct expander_device *ex = &dev->ex_dev;
> > +    struct ex_phy *phy = &ex->ex_phy[phy_id];
> > +
> > +    phy->phy_change_count = -1;
> > +    ex->ex_change_count = -1;
> > +    sas_unregister_devs_sas_addr(dev, phy_id, last);
> > +    sas_discover_event(dev->port, DISCE_REVALIDATE_DOMAIN);
> > +}
> > +
> > +static bool sas_dev_is_flutter(struct domain_device *dev, int phy_id,
> > +                   u8 *sas_addr, enum sas_device_type type)
> > +{
> > +    struct expander_device *ex = &dev->ex_dev;
> > +    struct ex_phy *phy = &ex->ex_phy[phy_id];
> > +    struct domain_device *child_dev;
> > +    char *action = "";
> > +    int res;
> > +
> > +    if (SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr) ||
> > +        !dev_type_flutter(type, phy->attached_dev_type))
> > +        return false;
> > +
> > +    res = sas_ex_phy_discover(dev, phy_id);
> > +    if (res)
> > +        return false;
> > +
> > +    child_dev = sas_ex_phy_to_dev(dev, phy_id);
> > +    if (!child_dev)
> > +        goto out;
> > +
> > +    if (dev_is_sata(child_dev) &&
> > +        phy->attached_dev_type == SAS_SATA_PENDING) {
> > +        action = ", needs recovery";
> > +        goto out;
> > +    }
> > +
> > +    if (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));
> > +        /*
> > +         * Device unregistering relies on address matching. Restore
> > +         * attached_sas_addr back to the original address so that the 
> > old
> > +         * device can be unregistered later
> > +         */
> > +        memcpy(phy->attached_sas_addr, child_dev->sas_addr, 
> > SAS_ADDR_SIZE);
> > +        return false;
> > +    }
> > +
> > +    if (child_dev->linkrate != phy->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);
> > +        return false;
> > +    }
> > +
> > +out:
> > +    pr_debug("ex %016llx phy%02d broadcast flutter%s\n",
> > +         SAS_ADDR(dev->sas_addr), phy_id, action);
> > +    return true;
> > +}
> > +
> >   static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
> >                     bool last, int sibling)
> >   {
> > @@ -2016,27 +2082,16 @@ static int sas_rediscover_dev(struct 
> > domain_device *dev, int phy_id,
> >           if (res == 0)
> >               sas_set_ex_phy(dev, phy_id, disc_resp);
> >           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);
> > -        char *action = "";
> > -
> > -        sas_ex_phy_discover(dev, phy_id);
> > +    }
> > -        if (ata_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);
> > +    if (sas_dev_is_flutter(dev, phy_id, sas_addr, type))
> >           goto out_free_resp;
> > -    }
> >       /* we always have to delete the old device when we went here */
>
> Please remove this comment since you are not deleting the old device now.

Hi, Jason

We will call sas_rediscover_ex_phy(dev, phy_id, last) later to delete 
the old device, and rediscover the device.

Thanks,
Xingui