On 2024/2/27 11:06, yangxingui wrote:
Hi Jason,
On 2024/2/23 15:04, Jason Yan wrote:
On 2024/2/23 12:04, yangxingui wrote:
Hi, John
On 2024/2/22 20:41, John Garry wrote:
On 21/02/2024 07:31, Xingui Yang wrote:
As of commit d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to
update PHY info"), do discovery will send a new SMP_DISCOVER and
update
phy->phy_change_count. We found that if the disk is reconnected and
phy
change_count changes at this time, the disk scanning process will
not be
triggered.
So update the PHY info with the last query results.
Fixes: d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to
update PHY info")
Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
---
drivers/scsi/libsas/sas_expander.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c
b/drivers/scsi/libsas/sas_expander.c
index a2204674b680..9563f5589948 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1681,6 +1681,10 @@ int sas_get_phy_attached_dev(struct
domain_device *dev, int phy_id,
if (*type == 0)
memset(sas_addr, 0, SAS_ADDR_SIZE);
}
+
+ if ((SAS_ADDR(sas_addr) == 0) || (res == -ECOMM))
+ sas_set_ex_phy(dev, phy_id, disc_resp);
+
kfree(disc_resp);
return res;
}
@@ -1972,11 +1976,6 @@ static int sas_rediscover_dev(struct
domain_device *dev, int phy_id,
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);
It would be nice to be able to call sas_set_ex_phy() here (instead
of sas_get_phy_attached_dev()), but I assume that you can't do that
as the disc_resp memory is not available.
If we were to manually set the PHY info here instead, how would that
look?
Yes, I think it is indeed better to use sas_set_ex_phy, as you said,
disc_resp memory is not available. Maybe we can use
sas_get_phy_discover instead of sas_get_phy_attached_dev so we can
use disc_resp?
Can we directly set phy->negotiated_linkrate = SAS_PHY_DISABLED here?
For an empty PHY the other variables means nothing, so why bother get
and update them?
The value of the negotiated link rate has two possible values in the
current processing branch: SAS_LINK_RATE_UNKNOWN and SAS_PHY_DISABLED,
and both come from disc_resp. If we do not use disc_resp, but set a
fixed value SAS_PHY_DISABLED for it, it may not be appropriate.
OK, makes sense.
Thanks,
Xingui
.
