diff mbox series

[v3,3/3] scsi: libsas: Fix disk not being scanned in after being removed

Message ID 20240307093733.41222-4-yangxingui@huawei.com (mailing list archive)
State Superseded
Headers show
Series scsi: libsas: Fix disk not being scanned in after being removed | expand

Commit Message

yangxingui March 7, 2024, 9:37 a.m. UTC
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.

Therefore, call sas_set_ex_phy() to update the PHY info with the results of
the last query. And because the previous phy info will be used when calling
sas_unregister_devs_sas_addr(), sas_unregister_devs_sas_addr() should be
called before sas_set_ex_phy().

Fixes: d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to update PHY info")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

John Garry March 7, 2024, 10:08 a.m. UTC | #1
On 07/03/2024 09:37, 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.
> 
> Therefore, call sas_set_ex_phy() to update the PHY info with the results of
> the last query. And because the previous phy info will be used when calling
> sas_unregister_devs_sas_addr(), sas_unregister_devs_sas_addr() should be
> called before sas_set_ex_phy().
> 
> Fixes: d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to update PHY info")
> Signed-off-by: Xingui Yang<yangxingui@huawei.com>

I am also ok with a change to revert to allocating the resp memory with 
alloc_smp_resp(), but make the changes neat please:
Reviewed-by: John Garry <john.g.garry@oracle.com>
yangxingui March 7, 2024, 2:17 p.m. UTC | #2
Hi John,
On 2024/3/7 18:08, John Garry wrote:
> On 07/03/2024 09:37, 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.
>>
>> Therefore, call sas_set_ex_phy() to update the PHY info with the 
>> results of
>> the last query. And because the previous phy info will be used when 
>> calling
>> sas_unregister_devs_sas_addr(), sas_unregister_devs_sas_addr() should be
>> called before sas_set_ex_phy().
>>
>> Fixes: d8649fc1c5e4 ("scsi: libsas: Do discovery on empty PHY to 
>> update PHY info")
>> Signed-off-by: Xingui Yang<yangxingui@huawei.com>
> 
> I am also ok with a change to revert to allocating the resp memory with 
> alloc_smp_resp(), but make the changes neat please:
> Reviewed-by: John Garry <john.g.garry@oracle.com>
> .

Thanks for your review, I have updated the version.

Thanks,
Xingui
diff mbox series

Patch

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index d6147616339f..5ef77a7d235e 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1955,6 +1955,7 @@  static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
 	struct expander_device *ex = &dev->ex_dev;
 	struct ex_phy *phy = &ex->ex_phy[phy_id];
 	enum sas_device_type type = SAS_PHY_UNUSED;
+	struct smp_disc_resp disc_resp;
 	u8 sas_addr[SAS_ADDR_SIZE];
 	char msg[80] = "";
 	int res;
@@ -1966,7 +1967,7 @@  static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
 		 SAS_ADDR(dev->sas_addr), phy_id, msg);
 
 	memset(sas_addr, 0, SAS_ADDR_SIZE);
-	res = sas_get_phy_attached_dev(dev, phy_id, sas_addr, &type);
+	res = sas_get_phy_discover(dev, phy_id, &disc_resp);
 	switch (res) {
 	case SMP_RESP_NO_PHY:
 		phy->phy_state = PHY_NOT_PRESENT;
@@ -1984,14 +1985,18 @@  static int sas_rediscover_dev(struct domain_device *dev, int phy_id,
 		return res;
 	}
 
+	if (res == 0)
+		sas_get_sas_addr_and_dev_type(&disc_resp, sas_addr, &type);
+
 	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.
+		 * Even though the PHY is empty, for convenience we update
+		 * the PHY info, like negotiated linkrate.
 		 */
-		sas_ex_phy_discover(dev, phy_id);
+		if (res == 0)
+			sas_set_ex_phy(dev, phy_id, &disc_resp);
 		return res;
 	} else if (SAS_ADDR(sas_addr) == SAS_ADDR(phy->attached_sas_addr) &&
 		   dev_type_flutter(type, phy->attached_dev_type)) {