[PATCH 0/8] libsas: Support swapping disks and SATA phy link rate matching the pathway

[PATCH 0/8] libsas: Support swapping disks and SATA phy link rate matching the pathway

[Jason Yan]

The work flow of revalidation now is scanning expander phy by the
sequence of the phy and check if the phy have changed. This will leads
to some issues of swapping disks or replacing a disk with a new one.

And when an expander phy attached to a SATA phy is using a physical link
rate greater than the expander host phys's linkrate, the disk will failed
to be discovered(ATA commands all failed). This is described in sas
protocal spec as:

"If an expander phy attached to a SATA phy is using a physical link rate
greater than the maximum connection rate supported by the pathway from
an STP initiator port, a management application client should use the
SMP PHY CONTROL function (see 10.4.3.10) to set the PROGRAMMED MAXIMUM
PHYSICAL LINK RATE field of the expander phy to the maximum connection
rate supported by the pathway from that STP initiator port."

This patchset addresses the issues above by these main changes:
1. Let the revalidation first scan all phys, mark all changed phys, then
revalidate in two steps. First check if we need to unregister some devices.
if we need to unregister some devices, raise a new bcast and return.
Second, if no devices need to be unregistered, discover new devices.
2. Check the SATA phy's linkrate to see if it is greater than any phy's 
linkrate it may pass through. Remember the minimum linkrate of the pathway
and set the SATA phy linkrate to it using the SMP PHY CONTROL function.
3. Other changes such as checking the ata devices class and id to ensure
the same device after flutter and so on.

Jason Yan (8):
  scsi: libsas: delete dead code in scsi_transport_sas.c
  scsi: libsas: check the lldd callback correctly
  scsi: libsas: always unregister the old device if going to discover
    new
  scsi: libsas: trigger a new revalidation to discover the device
  scsi: libsas: check if the same sata device when flutter
  scsi: libsas: reset the phy state and address if discover failed
  scsi: libsas: fix issue of swapping two sas disks
  scsi: libsas: support SATA phy link rate unmatch the pathway

 drivers/ata/libata-core.c          |   3 +-
 drivers/scsi/libsas/sas_ata.c      | 131 ++++++++++++++++++++++++
 drivers/scsi/libsas/sas_discover.c |   4 +-
 drivers/scsi/libsas/sas_expander.c | 198 +++++++++++++++++++++++++++++--------
 drivers/scsi/libsas/sas_port.c     |   2 +
 drivers/scsi/scsi_transport_sas.c  |   2 -
 include/linux/libata.h             |   2 +
 include/scsi/libsas.h              |   1 +
 include/scsi/sas_ata.h             |   6 ++
 9 files changed, 305 insertions(+), 44 deletions(-)

-- 
2.13.6

[PATCH 1/8] scsi: libsas: delete dead code in scsi_transport_sas.c

[Jason Yan]

This code is dead and no clue implies that it will be back again.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/scsi_transport_sas.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 08acbabfae07..92d966e500d4 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -623,7 +623,6 @@ sas_phy_protocol_attr(identify.target_port_protocols,
 sas_phy_simple_attr(identify.sas_address, sas_address, "0x%016llx\n",
 		unsigned long long);
 sas_phy_simple_attr(identify.phy_identifier, phy_identifier, "%d\n", u8);
-//sas_phy_simple_attr(port_identifier, port_identifier, "%d\n", int);
 sas_phy_linkspeed_attr(negotiated_linkrate);
 sas_phy_linkspeed_attr(minimum_linkrate_hw);
 sas_phy_linkspeed_rw_attr(minimum_linkrate);
@@ -1813,7 +1812,6 @@ sas_attach_transport(struct sas_function_template *ft)
 	SETUP_PHY_ATTRIBUTE(device_type);
 	SETUP_PHY_ATTRIBUTE(sas_address);
 	SETUP_PHY_ATTRIBUTE(phy_identifier);
-	//SETUP_PHY_ATTRIBUTE(port_identifier);
 	SETUP_PHY_ATTRIBUTE(negotiated_linkrate);
 	SETUP_PHY_ATTRIBUTE(minimum_linkrate_hw);
 	SETUP_PHY_ATTRIBUTE_RW(minimum_linkrate);
-- 
2.13.6

[PATCH 2/8] scsi: libsas: check the lldd callback correctly

[Jason Yan]

We are using lldd_port_deformed so we'd better check if lldd_port_deformed
is NULL.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_discover.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index a0fa7ef3a071..354f6db5bb66 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -260,7 +260,7 @@ static void sas_suspend_devices(struct work_struct *work)
 	 * phy_list is not being mutated
 	 */
 	list_for_each_entry(phy, &port->phy_list, port_phy_el) {
-		if (si->dft->lldd_port_formed)
+		if (si->dft->lldd_port_deformed)
 			si->dft->lldd_port_deformed(phy);
 		phy->suspended = 1;
 		port->suspended = 1;
-- 
2.13.6

[PATCH 3/8] scsi: libsas: always unregister the old device if going to discover new

[Jason Yan]

If we went into sas_rediscover_dev() the attached_sas_addr was already
insured not to be zero. So it's unnecessary to check if the
attached_sas_addr is zero.

And although if the sas address is not changed, we always have to
unregister the old device when we are going to register a new one. We
cannot just leave the device there and bring up the new.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: chenxiang <chenxiang66@hisilicon.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_expander.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 8b7114348def..629c580d906b 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2054,14 +2054,11 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
 		return res;
 	}
 
-	/* delete the old link */
-	if (SAS_ADDR(phy->attached_sas_addr) &&
-	    SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr)) {
-		SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
-			    SAS_ADDR(dev->sas_addr), phy_id,
-			    SAS_ADDR(phy->attached_sas_addr));
-		sas_unregister_devs_sas_addr(dev, phy_id, last);
-	}
+	/* we always have to delete the old device when we went here */
+	SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
+		    SAS_ADDR(dev->sas_addr), phy_id,
+		    SAS_ADDR(phy->attached_sas_addr));
+	sas_unregister_devs_sas_addr(dev, phy_id, last);
 
 	return sas_discover_new(dev, phy_id);
 }
-- 
2.13.6

[PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[Jason Yan]

Now if a new device replaced a old device, the sas address will change.
We unregister the old device and discover the new device in one
revalidation process. But after we deferred the sas_port_delete(), the
sas port is not deleted when we registering the new port and device.
This will make the sysfs complain of creating duplicate filename.

Fix this by doing the replacement in two steps. The first revalidation
only delete the old device and trigger a new revalidation. The second
revalidation discover the new device.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: chenxiang <chenxiang66@hisilicon.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_expander.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 629c580d906b..25ad9ef54e6c 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2013,6 +2013,8 @@ static int sas_rediscover_dev(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];
+	struct asd_sas_port *port = dev->port;
+	struct asd_sas_phy *sas_phy;
 	enum sas_device_type type = SAS_PHY_UNUSED;
 	u8 sas_addr[8];
 	int res;
@@ -2060,7 +2062,14 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
 		    SAS_ADDR(phy->attached_sas_addr));
 	sas_unregister_devs_sas_addr(dev, phy_id, last);
 
-	return sas_discover_new(dev, phy_id);
+	/* force the next revalidation find this phy and bring it up */
+	phy->phy_change_count = -1;
+	ex->ex_change_count = -1;
+	sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
+			port_phy_el);
+	port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+
+	return 0;
 }
 
 /**
-- 
2.13.6

[PATCH 5/8] scsi: libsas: check if the same sata device when flutter

[Jason Yan]

The ata device do not have a real sas address. If a ata device is
replaced with another one, the sas address is the same. Now libsas treat
this senario as flutter and do not delete the old one and discover the
new one. This will cause the data read from or write to the wrong
device.

And also when hotplugging a sata device, libsas entered to the flutter
case and sometimes found the phy attached address is abnormal. The log
is like this:

sas: ex 500e004aaaaaaa1f phy6 originated BROADCAST(CHANGE)
sas: ex 500e004aaaaaaa1f phy06:U:0 attached: 0000000000000000 (no device)
sas: ex 500e004aaaaaaa1f phy 0x6 broadcast flutter

Fix this issue by checking the phy attached address and the ata device's
class and id if they are the same as the origin. The ata class and id is
readed in ata EH process. When ata EH is scheduled, revalidate will be
deferred and a new bcast will be raised.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
Reviewed-by: John Garry <john.garry@huawei.com>
CC: chenxiang <chenxiang66@hisilicon.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Tejun Heo <tj@kernel.org>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/ata/libata-core.c          |  3 ++-
 drivers/scsi/libsas/sas_ata.c      | 18 ++++++++++++++++++
 drivers/scsi/libsas/sas_expander.c | 28 ++++++++++++++++++++++++++++
 include/linux/libata.h             |  2 ++
 include/scsi/libsas.h              |  1 +
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 68596bd4cf06..7656ad58b381 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4228,7 +4228,7 @@ void ata_std_postreset(struct ata_link *link, unsigned int *classes)
  *	RETURNS:
  *	1 if @dev matches @new_class and @new_id, 0 otherwise.
  */
-static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,
+int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,
 			       const u16 *new_id)
 {
 	const u16 *old_id = dev->id;
@@ -7391,3 +7391,4 @@ EXPORT_SYMBOL_GPL(ata_cable_80wire);
 EXPORT_SYMBOL_GPL(ata_cable_unknown);
 EXPORT_SYMBOL_GPL(ata_cable_ignore);
 EXPORT_SYMBOL_GPL(ata_cable_sata);
+EXPORT_SYMBOL_GPL(ata_dev_same_device);
\ No newline at end of file
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 0cc1567eacc1..83f2c920480b 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -620,6 +620,22 @@ static int sas_get_ata_command_set(struct domain_device *dev)
 	return ata_dev_classify(&tf);
 }
 
+static void sas_ata_store_id(struct domain_device *dev)
+{
+	struct ata_device *ata_dev = sas_to_ata_dev(dev);
+	unsigned char model[ATA_ID_PROD_LEN + 1];
+	unsigned char serial[ATA_ID_SERNO_LEN + 1];
+
+	/* store the ata device's class and id */
+	memcpy(dev->sata_dev.id, ata_dev->id, ATA_ID_WORDS);
+	dev->sata_dev.class = ata_dev->class;
+
+	ata_id_c_string(ata_dev->id, model, ATA_ID_PROD, sizeof(model));
+	ata_id_c_string(ata_dev->id, serial, ATA_ID_SERNO, sizeof(serial));
+
+	sas_ata_printk(KERN_INFO, dev, "model:%s serial:%s\n", model, serial);
+}
+
 void sas_probe_sata(struct asd_sas_port *port)
 {
 	struct domain_device *dev, *n;
@@ -644,6 +660,8 @@ void sas_probe_sata(struct asd_sas_port *port)
 		 */
 		if (ata_dev_disabled(sas_to_ata_dev(dev)))
 			sas_fail_probe(dev, __func__, -ENODEV);
+		else
+			sas_ata_store_id(dev);
 	}
 
 }
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 25ad9ef54e6c..4617eccb0c43 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2053,9 +2053,37 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
 			action = ", needs recovery";
 		SAS_DPRINTK("ex %016llx phy 0x%x broadcast flutter%s\n",
 			    SAS_ADDR(dev->sas_addr), phy_id, action);
+
+		/* the phy attached address will be updated by sas_ex_phy_discover()
+		 * and sometimes become abnormal
+		 */
+		if (SAS_ADDR(phy->attached_sas_addr) != SAS_ADDR(sas_addr) ||
+		    SAS_ADDR(phy->attached_sas_addr) == 0) {
+			/* if attached_sas_addr become abnormal, we must set the
+			 * original address back so that the device can be unregistered
+			 */
+			memcpy(phy->attached_sas_addr, sas_addr, SAS_ADDR_SIZE);
+			SAS_DPRINTK("phy address(%016llx) abnormal, origin:%016llx\n",
+				    SAS_ADDR(phy->attached_sas_addr),
+				    SAS_ADDR(sas_addr));
+			goto unregister;
+		}
+
+
+		if (ata_dev) {
+			struct ata_device *adev = sas_to_ata_dev(ata_dev);
+			unsigned int class = ata_dev->sata_dev.class;
+			u16 *id = ata_dev->sata_dev.id;
+
+			/* to see if the disk is replaced with another one */
+			if (!ata_dev_same_device(adev, class, id))
+				goto unregister;
+		}
+
 		return res;
 	}
 
+unregister:
 	/* we always have to delete the old device when we went here */
 	SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
 		    SAS_ADDR(dev->sas_addr), phy_id,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1795fecdea17..a1aff2fdf11b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1140,6 +1140,8 @@ extern int sata_scr_write(struct ata_link *link, int reg, u32 val);
 extern int sata_scr_write_flush(struct ata_link *link, int reg, u32 val);
 extern bool ata_link_online(struct ata_link *link);
 extern bool ata_link_offline(struct ata_link *link);
+extern int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,
+			     const u16 *new_id);
 #ifdef CONFIG_PM
 extern int ata_host_suspend(struct ata_host *host, pm_message_t mesg);
 extern void ata_host_resume(struct ata_host *host);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 225ab7783dfd..8f403b7bb552 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -164,6 +164,7 @@ struct sata_device {
 	struct ata_host ata_host;
 	struct smp_resp rps_resp ____cacheline_aligned; /* report_phy_sata_resp */
 	u8     fis[ATA_RESP_FIS_SIZE];
+	u16    id[ATA_ID_WORDS];
 };
 
 struct ssp_device {
-- 
2.13.6

[PATCH 6/8] scsi: libsas: reset the phy state and address if discover failed

[Jason Yan]

When we failed to discover the device, the phy state and address is
still kept in ex_phy. So when the next time we revalidate this phy the
address and device type is the same, it will be considered as flutter
and will not be discovered again. So the device will not be brought up.

Fix this by reset the phy state and address to the initial value. Then
in the next revalidation the device will be discovered agian.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: Xiaofei Tan <tanxiaofei@huawei.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_expander.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 4617eccb0c43..6b6de85466c6 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1107,6 +1107,15 @@ static int sas_ex_discover_dev(struct domain_device *dev, int phy_id)
 
 			}
 		}
+	} else {
+		/* if we failed to discover this device, we have to
+		 * reset the expander phy state and address so that we
+		 * will not treat the phy as flutter in the next
+		 * revalidation
+		 */
+		ex_phy->phy_state = PHY_VACANT;
+		ex_phy->attached_dev_type = SAS_PHY_UNUSED;
+		memset(ex_phy->attached_sas_addr, 0, SAS_ADDR_SIZE);
 	}
 
 	return res;
-- 
2.13.6

[PATCH 7/8] scsi: libsas: fix issue of swapping two sas disks

[Jason Yan]

The work flow of revalidation now is scanning expander phy by the
sequence of the phy and check if the phy have changed. This will leads
to an issue of swapping two sas disks on one expander.

Assume we have two sas disks, connected with expander phy10 and phy11:

phy10: 5000cca04eb1001d  port-0:0:10
phy11: 5000cca04eb043ad  port-0:0:11

Swap these two disks, and imaging the following scenario:

revalidation 1:
  -->phy10: 0 --> delete phy10 domain device
  -->phy11: 5000cca04eb043ad (no change)
revalidation done

revalidation 2:
  -->step 1, check phy10:
  -->phy10: 5000cca04eb043ad   --> add to wide port(port-0:0:11) (phy11
       address is still 5000cca04eb043ad now)

  -->step 2, check phy11:
  -->phy11: 0  --> phy11 address is 0 now, but it's part of wide
       port(port-0:0:11), the domain device will not be deleted.
revalidation done

revalidation 3:
  -->phy10, 5000cca04eb043ad (no change)
  -->phy11: 5000cca04eb1001d --> try to add port-0:0:11 but failed,
       port-0:0:11 already exist, trigger a warning as follows
revalidation done

[14790.189699] sysfs: cannot create duplicate filename
'/devices/pci0000:74/0000:74:02.0/host0/port-0:0/expander-0:0/port-0:0:11'
[14790.201081] CPU: 25 PID: 5031 Comm: kworker/u192:3 Not tainted
4.16.0-rc1-191134-g138f084-dirty #228
[14790.210199] Hardware name: Huawei D06/D06, BIOS Hisilicon D06 EC UEFI
Nemo 2.0 RC0 - B303 05/16/2018
[14790.219323] Workqueue: 0000:74:02.0_disco_q sas_revalidate_domain
[14790.225404] Call trace:
[14790.227842]  dump_backtrace+0x0/0x18c
[14790.231492]  show_stack+0x14/0x1c
[14790.234798]  dump_stack+0x88/0xac
[14790.238101]  sysfs_warn_dup+0x64/0x7c
[14790.241751]  sysfs_create_dir_ns+0x90/0xa0
[14790.245835]  kobject_add_internal+0xa0/0x284
[14790.250092]  kobject_add+0xb8/0x11c
[14790.253570]  device_add+0xe8/0x598
[14790.256960]  sas_port_add+0x24/0x50
[14790.260436]  sas_ex_discover_devices+0xb10/0xc30
[14790.265040]  sas_ex_revalidate_domain+0x1d8/0x518
[14790.269731]  sas_revalidate_domain+0x12c/0x154
[14790.274163]  process_one_work+0x128/0x2b0
[14790.278160]  worker_thread+0x14c/0x408
[14790.281897]  kthread+0xfc/0x128
[14790.285026]  ret_from_fork+0x10/0x18
[14790.288598] ------------[ cut here ]------------

At last, the disk 5000cca04eb1001d is lost.

The basic idea of fix this issue is to let the revalidation first scan
all phys, and then unregisterring devices. Only when no devices need to
be unregisterred, go to the next step to discover new devices. If there
are devices need unregister, unregister those devices and raise a new
bcast. The next revalidation will process the discovering of the new
devices.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: Xiaofei Tan <tanxiaofei@huawei.com>
CC: chenxiang <chenxiang66@hisilicon.com>
CC: John Garry <john.garry@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_expander.c | 149 ++++++++++++++++++++++++++++---------
 1 file changed, 112 insertions(+), 37 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 6b6de85466c6..52d96965191c 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2022,8 +2022,6 @@ static int sas_rediscover_dev(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];
-	struct asd_sas_port *port = dev->port;
-	struct asd_sas_phy *sas_phy;
 	enum sas_device_type type = SAS_PHY_UNUSED;
 	u8 sas_addr[8];
 	int res;
@@ -2101,10 +2099,6 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
 
 	/* force the next revalidation find this phy and bring it up */
 	phy->phy_change_count = -1;
-	ex->ex_change_count = -1;
-	sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
-			port_phy_el);
-	port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
 
 	return 0;
 }
@@ -2127,30 +2121,74 @@ static int sas_rediscover(struct domain_device *dev, const int phy_id)
 {
 	struct expander_device *ex = &dev->ex_dev;
 	struct ex_phy *changed_phy = &ex->ex_phy[phy_id];
-	int res = 0;
 	int i;
 	bool last = true;	/* is this the last phy of the port */
 
-	SAS_DPRINTK("ex %016llx phy%d originated BROADCAST(CHANGE)\n",
-		    SAS_ADDR(dev->sas_addr), phy_id);
+	for (i = 0; i < ex->num_phys; i++) {
+		struct ex_phy *phy = &ex->ex_phy[i];
 
-	if (SAS_ADDR(changed_phy->attached_sas_addr) != 0) {
-		for (i = 0; i < ex->num_phys; i++) {
-			struct ex_phy *phy = &ex->ex_phy[i];
+		if (i == phy_id)
+			continue;
+		if (SAS_ADDR(phy->attached_sas_addr) ==
+		    SAS_ADDR(changed_phy->attached_sas_addr)) {
+			SAS_DPRINTK("phy%d part of wide port with "
+				    "phy%d\n", phy_id, i);
+			last = false;
+			break;
+		}
+	}
+	return sas_rediscover_dev(dev, phy_id, last);
+}
 
-			if (i == phy_id)
-				continue;
-			if (SAS_ADDR(phy->attached_sas_addr) ==
-			    SAS_ADDR(changed_phy->attached_sas_addr)) {
-				SAS_DPRINTK("phy%d part of wide port with "
-					    "phy%d\n", phy_id, i);
-				last = false;
-				break;
-			}
+static inline int sas_ex_unregister(struct domain_device *dev,
+				 u8 *changed_phy,
+				 int nr)
+{
+	struct expander_device *ex = &dev->ex_dev;
+	int unregistered = 0;
+	struct ex_phy *phy;
+	int res;
+	int i;
+
+	for (i = 0; i < nr; i++) {
+		SAS_DPRINTK("ex %016llx phy%d originated BROADCAST(CHANGE)\n",
+			    SAS_ADDR(dev->sas_addr), changed_phy[i]);
+
+		phy = &ex->ex_phy[changed_phy[i]];
+
+		if (SAS_ADDR(phy->attached_sas_addr) != 0) {
+			res = sas_rediscover(dev, changed_phy[i]);
+			changed_phy[i] = 0xff;
+			unregistered++;
 		}
-		res = sas_rediscover_dev(dev, phy_id, last);
-	} else
-		res = sas_discover_new(dev, phy_id);
+	}
+
+	return unregistered;
+}
+
+static inline int sas_ex_register(struct domain_device *dev,
+				 u8 *changed_phy,
+				 int nr)
+{
+	struct expander_device *ex = &dev->ex_dev;
+	struct ex_phy *phy;
+	int res = 0;
+	int i;
+
+	for (i = 0; i < nr; i++) {
+		if (changed_phy[i] == 0xff)
+			continue;
+
+		phy = &ex->ex_phy[changed_phy[i]];
+
+		WARN(SAS_ADDR(phy->attached_sas_addr) != 0,
+		     "phy%02d impossible attached_sas_addr %016llx\n",
+		     changed_phy[i],
+		     SAS_ADDR(phy->attached_sas_addr));
+
+		res = sas_discover_new(dev, changed_phy[i]);
+	}
+
 	return res;
 }
 
@@ -2166,23 +2204,60 @@ static int sas_rediscover(struct domain_device *dev, const int phy_id)
 int sas_ex_revalidate_domain(struct domain_device *port_dev)
 {
 	int res;
+	struct expander_device *ex;
 	struct domain_device *dev = NULL;
+	u8 changed_phy[MAX_EXPANDER_PHYS];
+	int unregistered = 0;
+	int phy_id;
+	int nr = 0;
+	int i = 0;
 
 	res = sas_find_bcast_dev(port_dev, &dev);
-	if (res == 0 && dev) {
-		struct expander_device *ex = &dev->ex_dev;
-		int i = 0, phy_id;
-
-		do {
-			phy_id = -1;
-			res = sas_find_bcast_phy(dev, &phy_id, i, true);
-			if (phy_id == -1)
-				break;
-			res = sas_rediscover(dev, phy_id);
-			i = phy_id + 1;
-		} while (i < ex->num_phys);
+	if (res != 0 || !dev)
+		return res;
+
+	memset(changed_phy, 0xff, MAX_EXPANDER_PHYS);
+	ex = &dev->ex_dev;
+
+	do {
+		phy_id = -1;
+		res = sas_find_bcast_phy(dev, &phy_id, i, true);
+		if (phy_id == -1)
+			break;
+		changed_phy[nr++] = phy_id;
+		i = phy_id + 1;
+	} while (i < dev->ex_dev.num_phys);
+
+	if (nr == 0)
+		return res;
+
+	unregistered = sas_ex_unregister(dev, changed_phy, nr);
+
+	/* we have unregistered some devices in this pass and need to
+	 * go again to pick up on any new devices on a separate pass
+	 */
+	if (unregistered > 0) {
+		struct asd_sas_port *port = dev->port;
+		struct asd_sas_phy *sas_phy;
+		struct ex_phy *phy;
+
+		for (i = 0; i < nr; i++) {
+			if (changed_phy[i] == 0xff)
+				continue;
+			phy = &ex->ex_phy[changed_phy[i]];
+			phy->phy_change_count = -1;
+		}
+		ex->ex_change_count = -1;
+
+		sas_phy = container_of(dev->port->phy_list.next,
+				struct asd_sas_phy,
+				port_phy_el);
+		port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
+
+		return 0;
 	}
-	return res;
+
+	return sas_ex_register(dev, changed_phy, nr);
 }
 
 void sas_smp_handler(struct bsg_job *job, struct Scsi_Host *shost,
-- 
2.13.6

[PATCH 8/8] scsi: libsas: support SATA phy link rate unmatch the pathway

[Jason Yan]

If a SATA disk attached to a expander phy and it's linkrate is greater
than the expander host phy's linkrate, the disk will failed to discover.
The topology is like below:

   +----------+           +----------+
   |          |           |          |
   |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
   |          |           |          |
   |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
   |initiator |           |          |
   | device   |-- 3.0 G --| Expander |-- 6.0 G -- SAS  disk
   |          |           |          |
   |          |-- 3.0 G --|          |-- 6.0 G -- SATA disk  -->failed to connect
   |          |           |          |
   |          |           |          |-- 6.0 G -- SATA disk  -->failed to connect
   |          |           |          |
   +----------+           +----------+

And when we check the sas protocal spec, this scenario is described as
this:

7.13 Rate matching
......
If an expander phy attached to a SATA phy is using a physical link rate
greater than the maximum connection rate supported by the pathway from
an STP initiator port, a management application client should use the
SMP PHY CONTROL function (see 10.4.3.10) to set the PROGRAMMED MAXIMUM
PHYSICAL LINK RATE field of the expander phy to the maximum connection
rate supported by the pathway from that STP initiator port.

In order to support this scenario, checking the SATA disk's linkrate
to see if it is greater than any phy's linkrate it may pass through.
Remember the minimum linkrate of the pathway and set the SATA phy
linkrate to it using the SMP PHY CONTROL function.

Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: chenxiang <chenxiang66@hisilicon.com>
CC: John Garry <john.garry@huawei.com>
CC: chenqilin <chenqilin2@huawei.com>
CC: Johannes Thumshirn <jthumshirn@suse.de>
CC: Ewan Milne <emilne@redhat.com>
CC: Christoph Hellwig <hch@lst.de>
CC: Tomas Henzl <thenzl@redhat.com>
CC: Dan Williams <dan.j.williams@intel.com>
CC: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/libsas/sas_ata.c      | 113 +++++++++++++++++++++++++++++++++++++
 drivers/scsi/libsas/sas_discover.c |   2 +
 drivers/scsi/libsas/sas_port.c     |   2 +
 include/scsi/sas_ata.h             |   6 ++
 4 files changed, 123 insertions(+)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 83f2c920480b..0cddce9bf1c8 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -388,6 +388,119 @@ static int sas_ata_printk(const char *level, const struct domain_device *ddev,
 	return r;
 }
 
+static enum sas_linkrate sas_find_min_pathway(struct domain_device *ddev)
+{
+	enum sas_linkrate min_linkrate = SAS_LINK_RATE_12_0_GBPS;
+	struct domain_device *child;
+	struct expander_device *ex;
+	struct asd_sas_phy *phy;
+	int i;
+
+	child = ddev;
+	ddev = ddev->parent;
+
+	while (ddev) {
+		if (ddev->dev_type != SAS_EDGE_EXPANDER_DEVICE &&
+		    ddev->dev_type != SAS_FANOUT_EXPANDER_DEVICE)
+			break;
+
+		ex = &ddev->ex_dev;
+
+		for (i = 0; i < ex->num_phys; i++) {
+			struct ex_phy *phy = &ex->ex_phy[i];
+
+			if (phy->phy_state == PHY_VACANT ||
+			    phy->phy_state == PHY_NOT_PRESENT)
+				continue;
+
+			if (phy->linkrate < SAS_LINK_RATE_1_5_GBPS)
+				continue;
+
+			if (SAS_ADDR(phy->attached_sas_addr) == SAS_ADDR(child->sas_addr))
+				if (min_linkrate > phy->linkrate)
+					min_linkrate = phy->linkrate;
+		}
+
+		child = ddev;
+		ddev = ddev->parent;
+	}
+
+	/* check the direct attached phy linkrate */
+	list_for_each_entry(phy, &child->port->phy_list, port_phy_el) {
+		if (SAS_ADDR(phy->attached_sas_addr) == SAS_ADDR(child->sas_addr))
+			if (min_linkrate > phy->linkrate)
+				min_linkrate = phy->linkrate;
+	}
+
+	return min_linkrate;
+}
+
+static void sas_ata_check_pathway(void *data, async_cookie_t cookie)
+{
+	struct domain_device *dev = data;
+	struct domain_device *ddev = dev->parent;
+	struct sas_phy_linkrates rates;
+	enum sas_linkrate linkrate;
+	int ret;
+
+	if (!ddev) {
+		sas_put_device(dev);
+		return;
+	}
+
+	/*
+	 * According to Serial Attached SCSI - 1.1 (SAS-1.1):
+	 * If an expander phy attached to a SATA phy is using a physical link
+	 * rate greater than the maximum connection rate supported by the
+	 * pathway from an STP initiator port, a management application client
+	 * should use the SMP PHY CONTROL function (see 10.4.3.10) to set the
+	 * PROGRAMMED MAXIMUM PHYSICAL LINK RATE field of the expander phy to
+	 * the maximum connection rate supported by the pathway from that STP
+	 * initiator port.
+	 */
+
+	linkrate = sas_find_min_pathway(ddev);
+
+	if (dev->linkrate > linkrate) {
+		struct sas_phy *phy = sas_get_local_phy(dev);
+
+		rates.minimum_linkrate = 0;
+		rates.maximum_linkrate = linkrate;
+		ret = sas_smp_phy_control(ddev, phy->number,
+			PHY_FUNC_LINK_RESET, &rates);
+
+		SAS_DPRINTK("ex %016llx phy%02d set max linkrate to %X %s\n",
+			    SAS_ADDR(ddev->sas_addr), phy->number, linkrate,
+			    ret ? "failed" : "succeed");
+		sas_put_local_phy(phy);
+	}
+
+	sas_put_device(dev);
+}
+
+void sas_ata_check_topology(struct asd_sas_port *port)
+{
+	ASYNC_DOMAIN_EXCLUSIVE(async);
+	struct domain_device *dev;
+
+	spin_lock(&port->dev_list_lock);
+	list_for_each_entry(dev, &port->dev_list, dev_list_node) {
+		if (!dev_is_sata(dev))
+			continue;
+
+		/* hold a reference since we may be
+		 * racing with final remove
+		 */
+		kref_get(&dev->kref);
+
+		async_schedule_domain(sas_ata_check_pathway, dev, &async);
+	}
+	spin_unlock(&port->dev_list_lock);
+
+	async_synchronize_full_domain(&async);
+
+}
+
 static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 			      unsigned long deadline)
 {
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 354f6db5bb66..34bfc622b910 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -529,6 +529,8 @@ static void sas_revalidate_domain(struct work_struct *work)
 	sas_destruct_devices(port);
 	sas_destruct_ports(port);
 	sas_probe_devices(port);
+
+	sas_ata_check_topology(port);
 }
 
 /* ---------- Events ---------- */
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index fad23dd39114..ddf004bf667e 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -194,6 +194,8 @@ static void sas_form_port(struct asd_sas_phy *phy)
 
 	sas_discover_event(phy->port, DISCE_DISCOVER_DOMAIN);
 	flush_workqueue(sas_ha->disco_q);
+
+	sas_ata_check_topology(port);
 }
 
 /**
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 00f41aeeecf5..9be6437c3777 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -48,6 +48,7 @@ void sas_probe_sata(struct asd_sas_port *port);
 void sas_suspend_sata(struct asd_sas_port *port);
 void sas_resume_sata(struct asd_sas_port *port);
 void sas_ata_end_eh(struct ata_port *ap);
+void sas_ata_check_topology(struct asd_sas_port *port);
 #else
 
 
@@ -100,6 +101,11 @@ static inline int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy
 static inline void sas_ata_end_eh(struct ata_port *ap)
 {
 }
+
+static inline void sas_ata_check_topology(struct asd_sas_port *port)
+{
+}
+
 #endif
 
 #endif /* _SAS_ATA_H_ */
-- 
2.13.6

Re: [PATCH 1/8] scsi: libsas: delete dead code in scsi_transport_sas.c

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 2/8] scsi: libsas: check the lldd callback correctly

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 3/8] scsi: libsas: always unregister the old device if going to discover new

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 2/8] scsi: libsas: check the lldd callback correctly

[John Garry]

On 29/05/2018 03:23, Jason Yan wrote:
> We are using lldd_port_deformed so we'd better check if lldd_port_deformed
> is NULL.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> CC: John Garry <john.garry@huawei.com>
> CC: Johannes Thumshirn <jthumshirn@suse.de>
> CC: Ewan Milne <emilne@redhat.com>
> CC: Christoph Hellwig <hch@lst.de>
> CC: Tomas Henzl <thenzl@redhat.com>
> CC: Dan Williams <dan.j.williams@intel.com>
> CC: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/libsas/sas_discover.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
> index a0fa7ef3a071..354f6db5bb66 100644
> --- a/drivers/scsi/libsas/sas_discover.c
> +++ b/drivers/scsi/libsas/sas_discover.c
> @@ -260,7 +260,7 @@ static void sas_suspend_devices(struct work_struct *work)
>  	 * phy_list is not being mutated
>  	 */
>  	list_for_each_entry(phy, &port->phy_list, port_phy_el) {
> -		if (si->dft->lldd_port_formed)
> +		if (si->dft->lldd_port_deformed)

If you do make this change, then you can remove 
hisi_sas_port_deformed(), as it is just a stub to avoid a jump to NULL 
from above.

>  			si->dft->lldd_port_deformed(phy);
>  		phy->suspended = 1;
>  		port->suspended = 1;
>

Re: [PATCH 1/8] scsi: libsas: delete dead code in scsi_transport_sas.c

[John Garry]

On 29/05/2018 03:23, Jason Yan wrote:
> This code is dead and no clue implies that it will be back again.

I think PHY port identifier was dropped in 65c92b09acf0218b64f, and code 
below is just remnants.

>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> CC: John Garry <john.garry@huawei.com>
> CC: Johannes Thumshirn <jthumshirn@suse.de>
> CC: Ewan Milne <emilne@redhat.com>
> CC: Christoph Hellwig <hch@lst.de>
> CC: Tomas Henzl <thenzl@redhat.com>
> CC: Dan Williams <dan.j.williams@intel.com>
> CC: Hannes Reinecke <hare@suse.com>

Reviewed-by: John Garry <john.garry@huawei.com>

> ---
>  drivers/scsi/scsi_transport_sas.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index 08acbabfae07..92d966e500d4 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -623,7 +623,6 @@ sas_phy_protocol_attr(identify.target_port_protocols,
>  sas_phy_simple_attr(identify.sas_address, sas_address, "0x%016llx\n",
>  		unsigned long long);
>  sas_phy_simple_attr(identify.phy_identifier, phy_identifier, "%d\n", u8);
> -//sas_phy_simple_attr(port_identifier, port_identifier, "%d\n", int);
>  sas_phy_linkspeed_attr(negotiated_linkrate);
>  sas_phy_linkspeed_attr(minimum_linkrate_hw);
>  sas_phy_linkspeed_rw_attr(minimum_linkrate);
> @@ -1813,7 +1812,6 @@ sas_attach_transport(struct sas_function_template *ft)
>  	SETUP_PHY_ATTRIBUTE(device_type);
>  	SETUP_PHY_ATTRIBUTE(sas_address);
>  	SETUP_PHY_ATTRIBUTE(phy_identifier);
> -	//SETUP_PHY_ATTRIBUTE(port_identifier);
>  	SETUP_PHY_ATTRIBUTE(negotiated_linkrate);
>  	SETUP_PHY_ATTRIBUTE(minimum_linkrate_hw);
>  	SETUP_PHY_ATTRIBUTE_RW(minimum_linkrate);
>

Re: [PATCH 3/8] scsi: libsas: always unregister the old device if going to discover new

[John Garry]

On 29/05/2018 03:23, Jason Yan wrote:
> If we went into sas_rediscover_dev() the attached_sas_addr was already
> insured not to be zero. So it's unnecessary to check if the
> attached_sas_addr is zero.
>
> And although if the sas address is not changed, we always have to
> unregister the old device when we are going to register a new one. We
> cannot just leave the device there and bring up the new.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> CC: chenxiang <chenxiang66@hisilicon.com>
> CC: John Garry <john.garry@huawei.com>
> CC: Johannes Thumshirn <jthumshirn@suse.de>
> CC: Ewan Milne <emilne@redhat.com>
> CC: Christoph Hellwig <hch@lst.de>
> CC: Tomas Henzl <thenzl@redhat.com>
> CC: Dan Williams <dan.j.williams@intel.com>
> CC: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/libsas/sas_expander.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 8b7114348def..629c580d906b 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -2054,14 +2054,11 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
>  		return res;
>  	}
>
> -	/* delete the old link */
> -	if (SAS_ADDR(phy->attached_sas_addr) &&
> -	    SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr)) {
> -		SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
> -			    SAS_ADDR(dev->sas_addr), phy_id,
> -			    SAS_ADDR(phy->attached_sas_addr));
> -		sas_unregister_devs_sas_addr(dev, phy_id, last);
> -	}

The preceeding checks in code check for no device/comm fail or SATA flutter.

If we're rediscovering the device and the SAS address has not changed, 
then why previously still try to discover a new device? I'm guessing 
sas_discover_new() had no affect in this case, since maybe since the PHY 
was already discovered. But that would not make sense since you say "we 
are going to register a new one". Or, if we are always going to register 
a new one, how did we ensure we always unregistered the old device 
previously (when SAS address did not change)?

> +	/* we always have to delete the old device when we went here */
> +	SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
> +		    SAS_ADDR(dev->sas_addr), phy_id,
> +		    SAS_ADDR(phy->attached_sas_addr));
> +	sas_unregister_devs_sas_addr(dev, phy_id, last);
>
>  	return sas_discover_new(dev, phy_id);
>  }
>

Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[John Garry]

On 29/05/2018 03:23, Jason Yan wrote:
> Now if a new device replaced a old device, the sas address will change.
> We unregister the old device and discover the new device in one
> revalidation process. But after we deferred the sas_port_delete(), the
> sas port is not deleted when we registering the new port and device.
> This will make the sysfs complain of creating duplicate filename.
>
> Fix this by doing the replacement in two steps. The first revalidation
> only delete the old device and trigger a new revalidation. The second
> revalidation discover the new device.
>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> CC: chenxiang <chenxiang66@hisilicon.com>
> CC: John Garry <john.garry@huawei.com>
> CC: Johannes Thumshirn <jthumshirn@suse.de>
> CC: Ewan Milne <emilne@redhat.com>
> CC: Christoph Hellwig <hch@lst.de>
> CC: Tomas Henzl <thenzl@redhat.com>
> CC: Dan Williams <dan.j.williams@intel.com>
> CC: Hannes Reinecke <hare@suse.com>
> ---
>  drivers/scsi/libsas/sas_expander.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 629c580d906b..25ad9ef54e6c 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -2013,6 +2013,8 @@ static int sas_rediscover_dev(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];
> +	struct asd_sas_port *port = dev->port;
> +	struct asd_sas_phy *sas_phy;
>  	enum sas_device_type type = SAS_PHY_UNUSED;
>  	u8 sas_addr[8];
>  	int res;
> @@ -2060,7 +2062,14 @@ static int sas_rediscover_dev(struct domain_device *dev, int phy_id, bool last)
>  		    SAS_ADDR(phy->attached_sas_addr));
>  	sas_unregister_devs_sas_addr(dev, phy_id, last);
>
> -	return sas_discover_new(dev, phy_id);
> +	/* force the next revalidation find this phy and bring it up */
> +	phy->phy_change_count = -1;
> +	ex->ex_change_count = -1;
> +	sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
> +			port_phy_el);
> +	port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
> +

This is less than ideal: that is, restarting another discovery with this 
artifical broadcast event. We do something similar when re-enabling 
revalidation.

Can we do all the event processing synchronised to the original event?

> +	return 0;
>  }
>
>  /**
>

Re: [PATCH 8/8] scsi: libsas: support SATA phy link rate unmatch the pathway

[John Garry]

On 29/05/2018 03:23, Jason Yan wrote:
> If a SATA disk attached to a expander phy and it's linkrate is greater
> than the expander host phy's linkrate, the disk will failed to discover.
> The topology is like below:
>
>    +----------+           +----------+
>    |          |           |          |
>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>    |          |           |          |
>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>    |initiator |           |          |
>    | device   |-- 3.0 G --| Expander |-- 6.0 G -- SAS  disk
>    |          |           |          |
>    |          |-- 3.0 G --|          |-- 6.0 G -- SATA disk  -->failed to connect
>    |          |           |          |
>    |          |           |          |-- 6.0 G -- SATA disk  -->failed to connect
>    |          |           |          |
>    +----------+           +----------+
>
> And when we check the sas protocal spec, this scenario is described as
> this:
>
> 7.13 Rate matching
> ......
> If an expander phy attached to a SATA phy is using a physical link rate
> greater than the maximum connection rate supported by the pathway from
> an STP initiator port, a management application client should use the
> SMP PHY CONTROL function (see 10.4.3.10) to set the PROGRAMMED MAXIMUM
> PHYSICAL LINK RATE field of the expander phy to the maximum connection
> rate supported by the pathway from that STP initiator port.
>
> In order to support this scenario, checking the SATA disk's linkrate
> to see if it is greater than any phy's linkrate it may pass through.
> Remember the minimum linkrate of the pathway and set the SATA phy
> linkrate to it using the SMP PHY CONTROL function.

As we (re)discover the tree, can we keep track of the min pathway to the 
root PHY dynamically (per expander), and then take action for any SATA 
devices attached which have a negotiated linkrate greater (than the 
expanders min pathway)? This would be an alternate to your approach of 
finishing discovery and then checking the min pathway as a whole new step.

Re: [PATCH 2/8] scsi: libsas: check the lldd callback correctly

[Jason Yan]

On 2018/5/31 22:09, John Garry wrote:
> On 29/05/2018 03:23, Jason Yan wrote:
>> We are using lldd_port_deformed so we'd better check if
>> lldd_port_deformed
>> is NULL.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> CC: John Garry <john.garry@huawei.com>
>> CC: Johannes Thumshirn <jthumshirn@suse.de>
>> CC: Ewan Milne <emilne@redhat.com>
>> CC: Christoph Hellwig <hch@lst.de>
>> CC: Tomas Henzl <thenzl@redhat.com>
>> CC: Dan Williams <dan.j.williams@intel.com>
>> CC: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/libsas/sas_discover.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_discover.c
>> b/drivers/scsi/libsas/sas_discover.c
>> index a0fa7ef3a071..354f6db5bb66 100644
>> --- a/drivers/scsi/libsas/sas_discover.c
>> +++ b/drivers/scsi/libsas/sas_discover.c
>> @@ -260,7 +260,7 @@ static void sas_suspend_devices(struct work_struct
>> *work)
>>       * phy_list is not being mutated
>>       */
>>      list_for_each_entry(phy, &port->phy_list, port_phy_el) {
>> -        if (si->dft->lldd_port_formed)
>> +        if (si->dft->lldd_port_deformed)
>
> If you do make this change, then you can remove
> hisi_sas_port_deformed(), as it is just a stub to avoid a jump to NULL
> from above.
>

OK, I will remove it.

>>              si->dft->lldd_port_deformed(phy);
>>          phy->suspended = 1;
>>          port->suspended = 1;
>>
>
>
>
> .
>

Re: [PATCH 3/8] scsi: libsas: always unregister the old device if going to discover new

[Jason Yan]

On 2018/5/31 23:09, John Garry wrote:
> On 29/05/2018 03:23, Jason Yan wrote:
>> If we went into sas_rediscover_dev() the attached_sas_addr was already
>> insured not to be zero. So it's unnecessary to check if the
>> attached_sas_addr is zero.
>>
>> And although if the sas address is not changed, we always have to
>> unregister the old device when we are going to register a new one. We
>> cannot just leave the device there and bring up the new.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> CC: chenxiang <chenxiang66@hisilicon.com>
>> CC: John Garry <john.garry@huawei.com>
>> CC: Johannes Thumshirn <jthumshirn@suse.de>
>> CC: Ewan Milne <emilne@redhat.com>
>> CC: Christoph Hellwig <hch@lst.de>
>> CC: Tomas Henzl <thenzl@redhat.com>
>> CC: Dan Williams <dan.j.williams@intel.com>
>> CC: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/libsas/sas_expander.c | 13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c
>> b/drivers/scsi/libsas/sas_expander.c
>> index 8b7114348def..629c580d906b 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -2054,14 +2054,11 @@ static int sas_rediscover_dev(struct
>> domain_device *dev, int phy_id, bool last)
>>          return res;
>>      }
>>
>> -    /* delete the old link */
>> -    if (SAS_ADDR(phy->attached_sas_addr) &&
>> -        SAS_ADDR(sas_addr) != SAS_ADDR(phy->attached_sas_addr)) {
>> -        SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
>> -                SAS_ADDR(dev->sas_addr), phy_id,
>> -                SAS_ADDR(phy->attached_sas_addr));
>> -        sas_unregister_devs_sas_addr(dev, phy_id, last);
>> -    }
>
> The preceeding checks in code check for no device/comm fail or SATA
> flutter.
>
> If we're rediscovering the device and the SAS address has not changed,
> then why previously still try to discover a new device? I'm guessing
> sas_discover_new() had no affect in this case, since maybe since the PHY
> was already discovered.

When we went here, means it is not flutter, something must change,
either the device type or the phy address. Then we call
sas_discover_new(). And sas_discover_new() sure *have* effect in this
case. Please check sas_discover_new() carefully.

  But that would not make sense since you say "we
> are going to register a new one". Or, if we are always going to register
> a new one, how did we ensure we always unregistered the old device
> previously (when SAS address did not change)?
>

If SAS address did not change, the device type must changed, otherwise
it will be a "flutter" and won't get here. So if the device type
changed, do we have a reason to keep the device? I don't think so.

>> +    /* we always have to delete the old device when we went here */
>> +    SAS_DPRINTK("ex %016llx phy 0x%x replace %016llx\n",
>> +            SAS_ADDR(dev->sas_addr), phy_id,
>> +            SAS_ADDR(phy->attached_sas_addr));
>> +    sas_unregister_devs_sas_addr(dev, phy_id, last);
>>
>>      return sas_discover_new(dev, phy_id);
>>  }
>>
>
>
>
> .
>

Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[Jason Yan]

On 2018/5/31 23:42, John Garry wrote:
> On 29/05/2018 03:23, Jason Yan wrote:
>> Now if a new device replaced a old device, the sas address will change.
>> We unregister the old device and discover the new device in one
>> revalidation process. But after we deferred the sas_port_delete(), the
>> sas port is not deleted when we registering the new port and device.
>> This will make the sysfs complain of creating duplicate filename.
>>
>> Fix this by doing the replacement in two steps. The first revalidation
>> only delete the old device and trigger a new revalidation. The second
>> revalidation discover the new device.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> CC: chenxiang <chenxiang66@hisilicon.com>
>> CC: John Garry <john.garry@huawei.com>
>> CC: Johannes Thumshirn <jthumshirn@suse.de>
>> CC: Ewan Milne <emilne@redhat.com>
>> CC: Christoph Hellwig <hch@lst.de>
>> CC: Tomas Henzl <thenzl@redhat.com>
>> CC: Dan Williams <dan.j.williams@intel.com>
>> CC: Hannes Reinecke <hare@suse.com>
>> ---
>>  drivers/scsi/libsas/sas_expander.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c
>> b/drivers/scsi/libsas/sas_expander.c
>> index 629c580d906b..25ad9ef54e6c 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -2013,6 +2013,8 @@ static int sas_rediscover_dev(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];
>> +    struct asd_sas_port *port = dev->port;
>> +    struct asd_sas_phy *sas_phy;
>>      enum sas_device_type type = SAS_PHY_UNUSED;
>>      u8 sas_addr[8];
>>      int res;
>> @@ -2060,7 +2062,14 @@ static int sas_rediscover_dev(struct
>> domain_device *dev, int phy_id, bool last)
>>              SAS_ADDR(phy->attached_sas_addr));
>>      sas_unregister_devs_sas_addr(dev, phy_id, last);
>>
>> -    return sas_discover_new(dev, phy_id);
>> +    /* force the next revalidation find this phy and bring it up */
>> +    phy->phy_change_count = -1;
>> +    ex->ex_change_count = -1;
>> +    sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
>> +            port_phy_el);
>> +    port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
>> +
>
> This is less than ideal: that is, restarting another discovery with this
> artifical broadcast event. We do something similar when re-enabling
> revalidation.
>

That will back to what we have discussed before. The sas port
adding/removing is delayed outside the disco_mutex. we can only do the
adding or removing once inside the disco_mutex.

> Can we do all the event processing synchronised to the original event?
>

Actually bcast is a very special event, and what we do in revalidation
at one time is scanning all phy changes, which may include many bcast
events(especially before our first patchset), and the next
revalidations may have nothing to do.

So "do all the event processing synchronised to the original event" is
impossible actually. Maybe if the bcast can indicate which device
originated it, we will achieve this goal.

But if you mean we shall do this device removing and rediscovering in
one revalidation if it is not a "flutter", I think we can wrap a new
function for sas_revalidate_domain(), such as:


while (need_to_revalidate_again)
	need_to_revalidate_again = sas_revalidate_domain()

In this way the sas_port adding/removing is packed in one loop, we won't 
have the annoyance of "duplicate filename" warning. What do you
think?

>> +    return 0;
>>  }
>>
>>  /**
>>
>
>
>
> .
>

Re: [PATCH 8/8] scsi: libsas: support SATA phy link rate unmatch the pathway

[Jason Yan]

On 2018/6/1 0:05, John Garry wrote:
> On 29/05/2018 03:23, Jason Yan wrote:
>> If a SATA disk attached to a expander phy and it's linkrate is greater
>> than the expander host phy's linkrate, the disk will failed to discover.
>> The topology is like below:
>>
>>    +----------+           +----------+
>>    |          |           |          |
>>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>>    |          |           |          |
>>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>>    |initiator |           |          |
>>    | device   |-- 3.0 G --| Expander |-- 6.0 G -- SAS  disk
>>    |          |           |          |
>>    |          |-- 3.0 G --|          |-- 6.0 G -- SATA disk  -->failed
>> to connect
>>    |          |           |          |
>>    |          |           |          |-- 6.0 G -- SATA disk  -->failed
>> to connect
>>    |          |           |          |
>>    +----------+           +----------+
>>
>> And when we check the sas protocal spec, this scenario is described as
>> this:
>>
>> 7.13 Rate matching
>> ......
>> If an expander phy attached to a SATA phy is using a physical link rate
>> greater than the maximum connection rate supported by the pathway from
>> an STP initiator port, a management application client should use the
>> SMP PHY CONTROL function (see 10.4.3.10) to set the PROGRAMMED MAXIMUM
>> PHYSICAL LINK RATE field of the expander phy to the maximum connection
>> rate supported by the pathway from that STP initiator port.
>>
>> In order to support this scenario, checking the SATA disk's linkrate
>> to see if it is greater than any phy's linkrate it may pass through.
>> Remember the minimum linkrate of the pathway and set the SATA phy
>> linkrate to it using the SMP PHY CONTROL function.
>
> As we (re)discover the tree, can we keep track of the min pathway to the
> root PHY dynamically (per expander), and then take action for any SATA
> devices attached which have a negotiated linkrate greater (than the
> expanders min pathway)? This would be an alternate to your approach of
> finishing discovery and then checking the min pathway as a whole new step.
>

Seems better, I will have a try to see if it works. Thanks.

>
> .
>

Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[John Garry]

On 01/06/2018 01:59, Jason Yan wrote:
>
>
> On 2018/5/31 23:42, John Garry wrote:
>> On 29/05/2018 03:23, Jason Yan wrote:
>>> Now if a new device replaced a old device, the sas address will change.
>>> We unregister the old device and discover the new device in one
>>> revalidation process. But after we deferred the sas_port_delete(), the
>>> sas port is not deleted when we registering the new port and device.
>>> This will make the sysfs complain of creating duplicate filename.
>>>
>>> Fix this by doing the replacement in two steps. The first revalidation
>>> only delete the old device and trigger a new revalidation. The second
>>> revalidation discover the new device.
>>>
>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>> CC: chenxiang <chenxiang66@hisilicon.com>
>>> CC: John Garry <john.garry@huawei.com>
>>> CC: Johannes Thumshirn <jthumshirn@suse.de>
>>> CC: Ewan Milne <emilne@redhat.com>
>>> CC: Christoph Hellwig <hch@lst.de>
>>> CC: Tomas Henzl <thenzl@redhat.com>
>>> CC: Dan Williams <dan.j.williams@intel.com>
>>> CC: Hannes Reinecke <hare@suse.com>
>>> ---
>>>  drivers/scsi/libsas/sas_expander.c | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/scsi/libsas/sas_expander.c
>>> b/drivers/scsi/libsas/sas_expander.c
>>> index 629c580d906b..25ad9ef54e6c 100644
>>> --- a/drivers/scsi/libsas/sas_expander.c
>>> +++ b/drivers/scsi/libsas/sas_expander.c
>>> @@ -2013,6 +2013,8 @@ static int sas_rediscover_dev(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];
>>> +    struct asd_sas_port *port = dev->port;
>>> +    struct asd_sas_phy *sas_phy;
>>>      enum sas_device_type type = SAS_PHY_UNUSED;
>>>      u8 sas_addr[8];
>>>      int res;
>>> @@ -2060,7 +2062,14 @@ static int sas_rediscover_dev(struct
>>> domain_device *dev, int phy_id, bool last)
>>>              SAS_ADDR(phy->attached_sas_addr));
>>>      sas_unregister_devs_sas_addr(dev, phy_id, last);
>>>
>>> -    return sas_discover_new(dev, phy_id);
>>> +    /* force the next revalidation find this phy and bring it up */
>>> +    phy->phy_change_count = -1;
>>> +    ex->ex_change_count = -1;
>>> +    sas_phy = container_of(port->phy_list.next, struct asd_sas_phy,
>>> +            port_phy_el);
>>> +    port->ha->notify_port_event(sas_phy, PORTE_BROADCAST_RCVD);
>>> +
>>
>> This is less than ideal: that is, restarting another discovery with this
>> artifical broadcast event. We do something similar when re-enabling
>> revalidation.
>>
>
> That will back to what we have discussed before. The sas port
> adding/removing is delayed outside the disco_mutex. we can only do the
> adding or removing once inside the disco_mutex.
>
>> Can we do all the event processing synchronised to the original event?
>>
>
> Actually bcast is a very special event, and what we do in revalidation
> at one time is scanning all phy changes, which may include many bcast
> events(especially before our first patchset), and the next
> revalidations may have nothing to do.
>
> So "do all the event processing synchronised to the original event" is
> impossible actually. Maybe if the bcast can indicate which device
> originated it, we will achieve this goal.

I mean that since libsas does disocovery/revalidation for all expander 
PHYS for a single event, than all discovery/revalidation should be 
synchronised with that same event. I don't mean that for a given 
expander PHY which originated a broadcast event, the 
revalidation/discovery for that PHY should be synchronised with that 
same event. Like you said, I don't think it's possible.

On another point, one of the reasons to synchronise event processing was 
so events are not lost and are processed in order. In principle, by 
chaining these bcast events we lose that, since other PHY events may be 
queued before we queue the new artificial bcast events.

>
> But if you mean we shall do this device removing and rediscovering in
> one revalidation if it is not a "flutter", I think we can wrap a new
> function for sas_revalidate_domain(), such as:
>
>
> while (need_to_revalidate_again)
>     need_to_revalidate_again = sas_revalidate_domain()
>
> In this way the sas_port adding/removing is packed in one loop, we won't
> have the annoyance of "duplicate filename" warning. What do you
> think?

Something like that, where all the discovery/revalidation and related 
device + port processing is done before we complete the revalidation 
event processing. A single revalidation event may defer do device+port 
processing multiple times.

>
>>> +    return 0;
>>>  }
>>>
>>>  /**
>>>
>>
>>
>>
>> .
>>
>
>
> .
>

Re: [PATCH 8/8] scsi: libsas: support SATA phy link rate unmatch the pathway

[John Garry]

On 01/06/2018 02:21, Jason Yan wrote:
>
>
> On 2018/6/1 0:05, John Garry wrote:
>> On 29/05/2018 03:23, Jason Yan wrote:
>>> If a SATA disk attached to a expander phy and it's linkrate is greater
>>> than the expander host phy's linkrate, the disk will failed to discover.
>>> The topology is like below:
>>>
>>>    +----------+           +----------+
>>>    |          |           |          |
>>>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>>>    |          |           |          |
>>>    |          |-- 3.0 G --|          |-- 6.0 G -- SAS  disk
>>>    |initiator |           |          |
>>>    | device   |-- 3.0 G --| Expander |-- 6.0 G -- SAS  disk
>>>    |          |           |          |
>>>    |          |-- 3.0 G --|          |-- 6.0 G -- SATA disk  -->failed
>>> to connect
>>>    |          |           |          |
>>>    |          |           |          |-- 6.0 G -- SATA disk  -->failed
>>> to connect
>>>    |          |           |          |
>>>    +----------+           +----------+
>>>
>>> And when we check the sas protocal spec, this scenario is described as
>>> this:
>>>
>>> 7.13 Rate matching
>>> ......
>>> If an expander phy attached to a SATA phy is using a physical link rate
>>> greater than the maximum connection rate supported by the pathway from
>>> an STP initiator port, a management application client should use the
>>> SMP PHY CONTROL function (see 10.4.3.10) to set the PROGRAMMED MAXIMUM
>>> PHYSICAL LINK RATE field of the expander phy to the maximum connection
>>> rate supported by the pathway from that STP initiator port.
>>>
>>> In order to support this scenario, checking the SATA disk's linkrate
>>> to see if it is greater than any phy's linkrate it may pass through.
>>> Remember the minimum linkrate of the pathway and set the SATA phy
>>> linkrate to it using the SMP PHY CONTROL function.
>>
>> As we (re)discover the tree, can we keep track of the min pathway to the
>> root PHY dynamically (per expander), and then take action for any SATA
>> devices attached which have a negotiated linkrate greater (than the
>> expanders min pathway)? This would be an alternate to your approach of
>> finishing discovery and then checking the min pathway as a whole new
>> step.
>>
>
> Seems better, I will have a try to see if it works. Thanks.

Fine, it seems the tricky part here is to figure out when to issue the 
linkrate change request.

Re: [PATCH 4/8] scsi: libsas: trigger a new revalidation to discover the device

[Jason Yan]

On 2018/6/1 18:02, John Garry wrote:
> I mean that since libsas does disocovery/revalidation for all expander
> PHYS for a single event, than all discovery/revalidation should be
> synchronised with that same event. I don't mean that for a given
> expander PHY which originated a broadcast event, the
> revalidation/discovery for that PHY should be synchronised with that
> same event. Like you said, I don't think it's possible.
>
> On another point, one of the reasons to synchronise event processing was
> so events are not lost and are processed in order. In principle, by
> chaining these bcast events we lose that, since other PHY events may be
> queued before we queue the new artificial bcast events.
>

I got what you mean. I will try to keep the principle of synchronised 
event processing.

>>
>> But if you mean we shall do this device removing and rediscovering in
>> one revalidation if it is not a "flutter", I think we can wrap a new
>> function for sas_revalidate_domain(), such as:
>>
>>
>> while (need_to_revalidate_again)
>>     need_to_revalidate_again = sas_revalidate_domain()
>>
>> In this way the sas_port adding/removing is packed in one loop, we won't
>> have the annoyance of "duplicate filename" warning. What do you
>> think?
>
> Something like that, where all the discovery/revalidation and related
> device + port processing is done before we complete the revalidation
> event processing. A single revalidation event may defer do device+port
> processing multiple times.