Re: [PATCH] scsi: libsas: Fix disk not being scanned in after being removed

[John Garry <john.g.garry@oracle.com>]

On 05/03/2024 11:25, yangxingui wrote:
>>
>> It's like the change in this patch.
> This doesn't work properly. the previous sas address will be compared 
> with the currently queried sas address and the previous phy information 
> will also be used when calling sas_unregister_devs_sas_addr() after the 
> sas_rediscover_dev() function calls sas_get_phy_attached_dev(). 
> Therefore, it is more appropriate to update the phy information after 
> the device is unregistered.

ok, fine

> as follows:
> static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
>                                bool last, int sibling)
> {
>      ...
>         res = sas_get_phy_attached_dev(dev, phy_id, sas_addr, &type);
>          switch (res) {
>          case SMP_RESP_NO_PHY:
>                  phy->phy_state = PHY_NOT_PRESENT;
>                  sas_unregister_devs_sas_addr(dev, phy_id, last);
>                  return res;
>          case SMP_RESP_PHY_VACANT:
>                  phy->phy_state = PHY_VACANT;
>                  sas_unregister_devs_sas_addr(dev, phy_id, last);
>                  return res;
>          case SMP_RESP_FUNC_ACC:
>                  break;
>          case -ECOMM:
>                  break;
>          default:
>                  return res;
>          }
> 
>          if ((SAS_ADDR(sas_addr) == 0) || (res == -ECOMM)) {
>                  phy->phy_state = PHY_EMPTY;
>                  sas_unregister_devs_sas_addr(dev, phy_id, last);
>                  /*
>                   * Even though the PHY is empty, for convenience we 
> discover
>                   * the PHY to update the PHY info, like negotiated 
> linkrate.
>                   */
>                  sas_ex_phy_discover(dev, phy_id);
>                  return res;
>          } else if (SAS_ADDR(sas_addr) == 
> SAS_ADDR(phy->attached_sas_addr) && // <=== Compare the previous sas 
> address with the current sas address
>                     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);
>                  return res;
>          }
> 
>>
>>
>>>
>>> OK, so let me have a closer look at v2.
>>
>> I have to say that v2 is quite complicated...
> Yes, but it works.

I'll check it again.

Thanks,
John