[PATCH 0/2] Check and update the device link rate during discovery

[PATCH 0/2] Check and update the device link rate during discovery

[Yihang Li]

SATA devices attached to an expander maybe generate incorrect IOs when
physical link re-establish at a lower link rate. This is due to the
expander PHY attached to a SATA PHY is using link rate greater than the
physical PHY link rate which is re-established.

This patchset fixes the issue.

Yihang Li (2):
  scsi: libsas: Add sas_update_linkrate()
  scsi: libsas: Add sas_check_port_linkrate()

 drivers/scsi/libsas/sas_discover.c | 18 +++++++++++++-
 drivers/scsi/libsas/sas_expander.c | 40 ++++++++++++++++++++++++++++++
 drivers/scsi/libsas/sas_internal.h |  1 +
 3 files changed, 58 insertions(+), 1 deletion(-)

-- 
2.30.0

[PATCH 1/2] scsi: libsas: Add sas_update_linkrate()

[Yihang Li]

Add support for updates the link rate of all expander devices and
expander SATA PHYs connected to the port and triggers revalidation.

Signed-off-by: Yihang Li <liyihang9@huawei.com>
---
 drivers/scsi/libsas/sas_expander.c | 40 ++++++++++++++++++++++++++++++
 drivers/scsi/libsas/sas_internal.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index a18259f68c40..0cfb98c791c9 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2029,6 +2029,46 @@ static bool dev_type_flutter(enum sas_device_type new, enum sas_device_type old)
 	return false;
 }
 
+void sas_update_linkrate(struct asd_sas_port *port)
+{
+	struct domain_device *parent, *dev, *n;
+	struct sas_phy *local_phy;
+	struct ex_phy *ex_phy;
+	int phy_id;
+
+	list_for_each_entry_safe(dev, n, &port->dev_list, dev_list_node) {
+		if (dev_is_expander(dev->dev_type)) {
+			dev->linkrate = port->linkrate;
+			dev->min_linkrate = port->linkrate;
+			dev->max_linkrate = port->linkrate;
+			dev->ex_dev.ex_change_count = -1;
+		}
+
+		local_phy = sas_get_local_phy(dev);
+		if (scsi_is_sas_phy_local(local_phy)) {
+			sas_put_local_phy(local_phy);
+			continue;
+		}
+
+		parent = dev->parent;
+		phy_id = local_phy->number;
+		ex_phy = &parent->ex_dev.ex_phy[phy_id];
+		if (dev_is_sata(dev)) {
+			if (dev->linkrate > parent->min_linkrate) {
+				struct sas_phy_linkrates rates = {
+					.maximum_linkrate = parent->min_linkrate,
+					.minimum_linkrate = parent->min_linkrate,
+				};
+
+				sas_smp_phy_control(parent, phy_id,
+						    PHY_FUNC_LINK_RESET, &rates);
+				ex_phy->phy_change_count = -1;
+			}
+		}
+		sas_put_local_phy(local_phy);
+	}
+}
+
 void async_sas_ex_phy_refresh_linkrate(void *data, async_cookie_t cookie)
 {
 	struct domain_device *dev = data;
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 591b217b0813..fc976043a523 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -36,6 +36,7 @@ int sas_show_oob_mode(enum sas_oob_mode oob_mode, char *buf);
 
 int  sas_register_phys(struct sas_ha_struct *sas_ha);
 void sas_unregister_phys(struct sas_ha_struct *sas_ha);
+void sas_update_linkrate(struct asd_sas_port *port);
 void async_sas_ex_phy_refresh_linkrate(void *data, async_cookie_t cookie);
 
 struct asd_sas_event *sas_alloc_event(struct asd_sas_phy *phy, gfp_t gfp_flags);
-- 
2.30.0

[PATCH 2/2] scsi: libsas: Add sas_check_port_linkrate()

[Yihang Li]

We found that in the scenario where the expander device is connected to
a wide port, a physical link connected to the wide port link down and
re-establish the link at a lower link rate, while the expander device
link rate and all expander PHY link rates maintain the original link rate,
the following error occurs:

[175712.419423] hisi_sas_v3_hw 0000:74:02.0: erroneous completion iptt=2985 task=00000000268357f1 dev id=10 exp 0x500e004aaaaaaa1f phy9 addr=500e004aaaaaaa09 CQ hdr: 0x102b 0xa0ba9 0x1000 0x20000 Error info: 0x200 0x0 0x0 0x0

After analysis, it is concluded that: when the physical link is
re-established, the link rate of the expander device and the expander PHY
are not updated. As a result, the expander PHY attached to a SATA PHY is
using link rate greater than the physical PHY link rate.

Therefore, add support for check whether the link rate of physical PHY
which is connected to the port changes after the phy up occur, if the
link rate of the newly established physical phy is lower than the link
rate of the port, a smaller link rate is transmitted to the port and
update the device link rate that needs to be updated in port->dev_list.

Signed-off-by: Yihang Li <liyihang9@huawei.com>
---
 drivers/scsi/libsas/sas_discover.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 6998560812f2..e453d94fbd30 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -164,6 +164,20 @@ static int sas_get_port_device(struct asd_sas_port *port)
 	return 0;
 }
 
+static void sas_check_port_linkrate(struct asd_sas_port *port)
+{
+	struct asd_sas_phy *phy;
+	u32 link_rate = port->linkrate;
+
+	list_for_each_entry(phy, &port->phy_list, port_phy_el)
+		link_rate = min(link_rate, phy->linkrate);
+
+	if (port->linkrate != link_rate) {
+		port->linkrate = link_rate;
+		sas_update_linkrate(port);
+	}
+}
+
 /* ---------- Discover and Revalidate ---------- */
 
 int sas_notify_lldd_dev_found(struct domain_device *dev)
@@ -435,8 +449,10 @@ static void sas_discover_domain(struct work_struct *work)
 
 	clear_bit(DISCE_DISCOVER_DOMAIN, &port->disc.pending);
 
-	if (port->port_dev)
+	if (port->port_dev) {
+		sas_check_port_linkrate(port);
 		return;
+	}
 
 	error = sas_get_port_device(port);
 	if (error)
-- 
2.30.0

Re: [PATCH 0/2] Check and update the device link rate during discovery

[John Garry]

On 20/10/2022 15:16, Yihang Li wrote:
> SATA devices attached to an expander maybe generate incorrect IOs when
> physical link re-establish at a lower link rate. This is due to the
> expander PHY attached to a SATA PHY is using link rate greater than the
> physical PHY link rate which is re-established.
> 

How do you create this scenario? I thought that I added min pathway 
support for root phy, such that we have min pathway support for root phy 
through 1x expander to a sata disk for initial discovery. I can't 
remember exactly the scenario I added support for....

BTW, did you mean the send this to mainline scsi mailng list? You did 
not cc maintainers Martin and James.

> This patchset fixes the issue.
> 
> Yihang Li (2):
>    scsi: libsas: Add sas_update_linkrate()
>    scsi: libsas: Add sas_check_port_linkrate()
> 
>   drivers/scsi/libsas/sas_discover.c | 18 +++++++++++++-
>   drivers/scsi/libsas/sas_expander.c | 40 ++++++++++++++++++++++++++++++
>   drivers/scsi/libsas/sas_internal.h |  1 +
>   3 files changed, 58 insertions(+), 1 deletion(-)
> 

Re: [PATCH 0/2] Check and update the device link rate during discovery

[Yihang Li]

Hi,

Sorry, I sent the wrong email address, please ignore this series.

Yihang

On 2022/10/20 22:16, Yihang Li wrote:
> SATA devices attached to an expander maybe generate incorrect IOs when
> physical link re-establish at a lower link rate. This is due to the
> expander PHY attached to a SATA PHY is using link rate greater than the
> physical PHY link rate which is re-established.
> 
> This patchset fixes the issue.
> 
> Yihang Li (2):
>   scsi: libsas: Add sas_update_linkrate()
>   scsi: libsas: Add sas_check_port_linkrate()
> 
>  drivers/scsi/libsas/sas_discover.c | 18 +++++++++++++-
>  drivers/scsi/libsas/sas_expander.c | 40 ++++++++++++++++++++++++++++++
>  drivers/scsi/libsas/sas_internal.h |  1 +
>  3 files changed, 58 insertions(+), 1 deletion(-)
>