[PATCH 0/4] libsas updates for 2.6.40 (resend)

[PATCH 0/4] libsas updates for 2.6.40 (resend)

[Dan Williams]

This is a resend [1] of our pending libsas patch queue, plus one more for .40
consideration "libsas: Add option for SATA soft reset".  We're still
investigating whether having a soft reset handler will be beneficial for isci.
If it does we need an available hook in the .40 baseline, so we can
drop/hook-up our private implementation.  The only other change from the last
posting [2] is to drop the problematic locking in "libsas: fix/amend device
gone notification in sas_deform_port()".

--
Dan

[1]: http://marc.info/?l=linux-scsi&m=130504487714639&w=2
[2]: http://marc.info/?l=linux-scsi&m=130444681502181&w=2

---

Dan Williams (3):
      libsas: flush initial device discovery before completing ->scan_finished()
      libsas: fix/amend device gone notification in sas_deform_port()
      libsas: check dev->gone before submitting sata i/o

Dave Jiang (1):
      libsas: Add option for SATA soft reset


 drivers/scsi/aic94xx/aic94xx_init.c |    2 +-
 drivers/scsi/libsas/sas_ata.c       |   41 ++++++++++++++++++++++++++++++++++-
 drivers/scsi/libsas/sas_internal.h  |    2 +-
 drivers/scsi/libsas/sas_phy.c       |    4 ++-
 drivers/scsi/libsas/sas_port.c      |   21 ++++++++++--------
 drivers/scsi/libsas/sas_scsi_host.c |   14 ++++++------
 drivers/scsi/mvsas/mv_sas.c         |    2 +-
 drivers/scsi/pm8001/pm8001_sas.c    |    2 +-
 include/scsi/libsas.h               |   14 ++++++++++++
 9 files changed, 79 insertions(+), 23 deletions(-)

[PATCH 1/4] libsas: flush initial device discovery before completing ->scan_finished()

[Dan Williams]

During initial scan libsas drivers start their phys and notify libsas
with PORTE_BYTES_DMAED events as port links are established.  This
notification in turn causes libsas to post DISCE_DISCOVER_DOMAIN events
to the queue.  Calling scsi_flush_work() at the end of scan_finished
guarantees that all preceding PORTE_BYTES_DMAED events have been
registered in the queue, but it does not guarantee that the resulting
DISCE_DISCOVER_DOMAIN events have been processed because
flush_workqueue() explicitly avoids live-locking with incoming work.

Introduce sas_flush_discovery() to guarantee that all initial discovery
events have completed.  It is called after the driver determines all
initial PORTE_BYTES_DMAED events have had a chance to enter the queue.
This does not cover BCNs that are generated during expander bring up,
only the initial sas_discover_domain() event.

Cc: David Milburn <dmilburn@redhat.com>
Cc: Srinivas <satyasrinivasp@hcl.in>
Cc: jack_wang@usish.com
Cc: lindar_liu@usish.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/aic94xx/aic94xx_init.c |    2 +-
 drivers/scsi/mvsas/mv_sas.c         |    2 +-
 drivers/scsi/pm8001/pm8001_sas.c    |    2 +-
 include/scsi/libsas.h               |   13 +++++++++++++
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 3b7e83d..f763daa 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -972,7 +972,7 @@ static int asd_scan_finished(struct Scsi_Host *shost, unsigned long time)
 	if (time < HZ)
 		return 0;
 	/* Wait for discovery to finish */
-	scsi_flush_work(shost);
+	sas_flush_discovery(shost);
 	return 1;
 }
 
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index adedaa9..e34c378 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -440,7 +440,7 @@ int mvs_scan_finished(struct Scsi_Host *shost, unsigned long time)
 	if (time < HZ)
 		return 0;
 	/* Wait for discovery to finish */
-	scsi_flush_work(shost);
+	sas_flush_discovery(shost);
 	return 1;
 }
 
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 6ae059e..997cf59 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -253,7 +253,7 @@ int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time)
 	if (time < HZ)
 		return 0;
 	/* Wait for discovery to finish */
-	scsi_flush_work(shost);
+	sas_flush_discovery(shost);
 	return 1;
 }
 
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 8f6bb9c..d603b26e 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -405,6 +405,19 @@ static inline void sas_phy_disconnected(struct asd_sas_phy *phy)
 	phy->linkrate = SAS_LINK_RATE_UNKNOWN;
 }
 
+/* Before returning from ->scan_finished() an LLDD calls this routine to
+ * ensure that all port notifications have been promoted to domain
+ * discovery events, and that initial domain discovery has completed
+ */
+static inline void sas_flush_discovery(struct Scsi_Host *shost)
+{
+	/* flush port events */
+	scsi_flush_work(shost);
+
+	/* flush domain discovery events queued by the port events */
+	scsi_flush_work(shost);
+}
+
 /* ---------- Tasks ---------- */
 /*
       service_response |  SAS_TASK_COMPLETE  |  SAS_TASK_UNDELIVERED |

Re: [PATCH 1/4] libsas: flush initial device discovery before completing ->scan_finished()

[Dan Williams]

On Tue, May 24, 2011 at 1:17 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> During initial scan libsas drivers start their phys and notify libsas
> with PORTE_BYTES_DMAED events as port links are established.  This
> notification in turn causes libsas to post DISCE_DISCOVER_DOMAIN events
> to the queue.  Calling scsi_flush_work() at the end of scan_finished
> guarantees that all preceding PORTE_BYTES_DMAED events have been
> registered in the queue, but it does not guarantee that the resulting
> DISCE_DISCOVER_DOMAIN events have been processed because
> flush_workqueue() explicitly avoids live-locking with incoming work.
>
> Introduce sas_flush_discovery() to guarantee that all initial discovery
> events have completed.  It is called after the driver determines all
> initial PORTE_BYTES_DMAED events have had a chance to enter the queue.
> This does not cover BCNs that are generated during expander bring up,
> only the initial sas_discover_domain() event.

I notice that scsi-misc picked up all the patches from the series,
save this one.

Oversight, or nak?

Note that latest upstream still says the following for flush_workqueue():

 * We sleep until all works which were queued on entry have been handled,
 * but we are not livelocked by new incoming ones.

...so isci will continue to open code this double flush.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] libsas: flush initial device discovery before completing ->scan_finished()

[James Bottomley]

On Fri, 2011-05-27 at 17:41 -0700, Dan Williams wrote:
> On Tue, May 24, 2011 at 1:17 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > During initial scan libsas drivers start their phys and notify libsas
> > with PORTE_BYTES_DMAED events as port links are established.  This
> > notification in turn causes libsas to post DISCE_DISCOVER_DOMAIN events
> > to the queue.  Calling scsi_flush_work() at the end of scan_finished
> > guarantees that all preceding PORTE_BYTES_DMAED events have been
> > registered in the queue, but it does not guarantee that the resulting
> > DISCE_DISCOVER_DOMAIN events have been processed because
> > flush_workqueue() explicitly avoids live-locking with incoming work.
> >
> > Introduce sas_flush_discovery() to guarantee that all initial discovery
> > events have completed.  It is called after the driver determines all
> > initial PORTE_BYTES_DMAED events have had a chance to enter the queue.
> > This does not cover BCNs that are generated during expander bring up,
> > only the initial sas_discover_domain() event.
> 
> I notice that scsi-misc picked up all the patches from the series,
> save this one.
> 
> Oversight, or nak?

Neither ... as I've said several times before, I want to use tejun's
drain infrastructure for this.

James

Re: [PATCH 1/4] libsas: flush initial device discovery before completing ->scan_finished()

[Dan Williams]

On Fri, May 27, 2011 at 8:14 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Fri, 2011-05-27 at 17:41 -0700, Dan Williams wrote:
>> On Tue, May 24, 2011 at 1:17 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> > During initial scan libsas drivers start their phys and notify libsas
>> > with PORTE_BYTES_DMAED events as port links are established.  This
>> > notification in turn causes libsas to post DISCE_DISCOVER_DOMAIN events
>> > to the queue.  Calling scsi_flush_work() at the end of scan_finished
>> > guarantees that all preceding PORTE_BYTES_DMAED events have been
>> > registered in the queue, but it does not guarantee that the resulting
>> > DISCE_DISCOVER_DOMAIN events have been processed because
>> > flush_workqueue() explicitly avoids live-locking with incoming work.
>> >
>> > Introduce sas_flush_discovery() to guarantee that all initial discovery
>> > events have completed.  It is called after the driver determines all
>> > initial PORTE_BYTES_DMAED events have had a chance to enter the queue.
>> > This does not cover BCNs that are generated during expander bring up,
>> > only the initial sas_discover_domain() event.
>>
>> I notice that scsi-misc picked up all the patches from the series,
>> save this one.
>>
>> Oversight, or nak?
>
> Neither ... as I've said several times before, I want to use tejun's
> drain infrastructure for this.
>

Apologies, for the disconnect.

Found the drain_workqueue thread...

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] libsas: fix/amend device gone notification in sas_deform_port()

[Dan Williams]

Commit 56dd2c06 "libsas: Don't issue commands to devices that have been
hot-removed" edited Darrick's original patch to remove setting 'gone' in
the sas_deform_port() path because that prevented scsi sync cache
commands from being issued when the driver was unloaded.  However, this
allows true device gone notifications (as signaled port phy events) to
trigger sync cache commands to devices that are known to be unreachable.

Teach libsas which sas_deform_port() invocations are likely device gone
events.

Cc: Darrick J. Wong <djwong@us.ibm.com>
Cc: Haipao Fan <haipao.fan@intel.com>
Cc: Maciej Trela <maciej.trela@intel.com>
Cc: David Milburn <dmilburn@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_internal.h |    2 +-
 drivers/scsi/libsas/sas_phy.c      |    4 ++--
 drivers/scsi/libsas/sas_port.c     |   21 ++++++++++++---------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 8b538bd..14e21b5 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -57,7 +57,7 @@ int  sas_init_queue(struct sas_ha_struct *sas_ha);
 int  sas_init_events(struct sas_ha_struct *sas_ha);
 void sas_shutdown_queue(struct sas_ha_struct *sas_ha);
 
-void sas_deform_port(struct asd_sas_phy *phy);
+void sas_deform_port(struct asd_sas_phy *phy, int gone);
 
 void sas_porte_bytes_dmaed(struct work_struct *work);
 void sas_porte_broadcast_rcvd(struct work_struct *work);
diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c
index b459c4b..e0f5018 100644
--- a/drivers/scsi/libsas/sas_phy.c
+++ b/drivers/scsi/libsas/sas_phy.c
@@ -39,7 +39,7 @@ static void sas_phye_loss_of_signal(struct work_struct *work)
 	sas_begin_event(PHYE_LOSS_OF_SIGNAL, &phy->ha->event_lock,
 			&phy->phy_events_pending);
 	phy->error = 0;
-	sas_deform_port(phy);
+	sas_deform_port(phy, 1);
 }
 
 static void sas_phye_oob_done(struct work_struct *work)
@@ -66,7 +66,7 @@ static void sas_phye_oob_error(struct work_struct *work)
 	sas_begin_event(PHYE_OOB_ERROR, &phy->ha->event_lock,
 			&phy->phy_events_pending);
 
-	sas_deform_port(phy);
+	sas_deform_port(phy, 1);
 
 	if (!port && phy->enabled && i->dft->lldd_control_phy) {
 		phy->error++;
diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 5257fdf..42fd1f2 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -57,7 +57,7 @@ static void sas_form_port(struct asd_sas_phy *phy)
 
 	if (port) {
 		if (!phy_is_wideport_member(port, phy))
-			sas_deform_port(phy);
+			sas_deform_port(phy, 0);
 		else {
 			SAS_DPRINTK("%s: phy%d belongs to port%d already(%d)!\n",
 				    __func__, phy->id, phy->port->id,
@@ -153,28 +153,31 @@ static void sas_form_port(struct asd_sas_phy *phy)
  * This is called when the physical link to the other phy has been
  * lost (on this phy), in Event thread context. We cannot delay here.
  */
-void sas_deform_port(struct asd_sas_phy *phy)
+void sas_deform_port(struct asd_sas_phy *phy, int gone)
 {
 	struct sas_ha_struct *sas_ha = phy->ha;
 	struct asd_sas_port *port = phy->port;
 	struct sas_internal *si =
 		to_sas_internal(sas_ha->core.shost->transportt);
+	struct domain_device *dev;
 	unsigned long flags;
 
 	if (!port)
 		return;		  /* done by a phy event */
 
-	if (port->port_dev)
-		port->port_dev->pathways--;
+	dev = port->port_dev;
+	if (dev)
+		dev->pathways--;
 
 	if (port->num_phys == 1) {
+		if (dev && gone)
+			dev->gone = 1;
 		sas_unregister_domain_devices(port);
 		sas_port_delete(port->port);
 		port->port = NULL;
 	} else
 		sas_port_delete_phy(port->port, phy->phy);
 
-
 	if (si->dft->lldd_port_deformed)
 		si->dft->lldd_port_deformed(phy);
 
@@ -244,7 +247,7 @@ void sas_porte_link_reset_err(struct work_struct *work)
 	sas_begin_event(PORTE_LINK_RESET_ERR, &phy->ha->event_lock,
 			&phy->port_events_pending);
 
-	sas_deform_port(phy);
+	sas_deform_port(phy, 1);
 }
 
 void sas_porte_timer_event(struct work_struct *work)
@@ -256,7 +259,7 @@ void sas_porte_timer_event(struct work_struct *work)
 	sas_begin_event(PORTE_TIMER_EVENT, &phy->ha->event_lock,
 			&phy->port_events_pending);
 
-	sas_deform_port(phy);
+	sas_deform_port(phy, 1);
 }
 
 void sas_porte_hard_reset(struct work_struct *work)
@@ -268,7 +271,7 @@ void sas_porte_hard_reset(struct work_struct *work)
 	sas_begin_event(PORTE_HARD_RESET, &phy->ha->event_lock,
 			&phy->port_events_pending);
 
-	sas_deform_port(phy);
+	sas_deform_port(phy, 1);
 }
 
 /* ---------- SAS port registration ---------- */
@@ -306,6 +309,6 @@ void sas_unregister_ports(struct sas_ha_struct *sas_ha)
 
 	for (i = 0; i < sas_ha->num_phys; i++)
 		if (sas_ha->sas_phy[i]->port)
-			sas_deform_port(sas_ha->sas_phy[i]);
+			sas_deform_port(sas_ha->sas_phy[i], 0);
 
 }

[PATCH 3/4] libsas: check dev->gone before submitting sata i/o

[Dan Williams]

Head off doomed-to-fail i/o in sas_queuecommand before sending it down
the ata path.

Before:
sd 7:0:0:0: [sdd] Synchronizing SCSI cache
ata8: no sense translation for status: 0x00
ata8: translated ATA stat/err 0x00/00 to SCSI SK/ASC/ASCQ 0xb/00/00
ata8.00: device reported invalid CHS sector 0
ata8: status=0x00 { }
ata8: no sense translation for status: 0x00
ata8: translated ATA stat/err 0x00/00 to SCSI SK/ASC/ASCQ 0xb/00/00
ata8.00: device reported invalid CHS sector 0
ata8: status=0x00 { }
ata8: no sense translation for status: 0x00
ata8: translated ATA stat/err 0x00/00 to SCSI SK/ASC/ASCQ 0xb/00/00
ata8.00: device reported invalid CHS sector 0
ata8: status=0x00 { }
sd 7:0:0:0: [sdd]  Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 7:0:0:0: [sdd]  Sense Key : Aborted Command [current] [descriptor]
sd 7:0:0:0: [sdd]  Add. Sense: No additional sense information
sd 7:0:0:0: [sdd] Stopping disk

After:
sd 9:0:0:0: [sdd] Synchronizing SCSI cache
sd 9:0:0:0: [sdd]  Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
sd 9:0:0:0: [sdd] Stopping disk
sd 9:0:0:0: [sdd] START_STOP FAILED
sd 9:0:0:0: [sdd]  Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK

This is a cosmetic change as sata i/o can still leak to a gone device,
but this addresses the nominal hotplug case when releasing the target.

Acked-by: Jack Wang <jack_wang@usish.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_scsi_host.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index f6e189f..eeba76c 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -207,6 +207,13 @@ static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
 		struct sas_ha_struct *sas_ha = dev->port->ha;
 		struct sas_task *task;
 
+		/* If the device fell off, no sense in issuing commands */
+		if (dev->gone) {
+			cmd->result = DID_BAD_TARGET << 16;
+			scsi_done(cmd);
+			goto out;
+		}
+
 		if (dev_is_sata(dev)) {
 			unsigned long flags;
 
@@ -216,13 +223,6 @@ static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
 			goto out;
 		}
 
-		/* If the device fell off, no sense in issuing commands */
-		if (dev->gone) {
-			cmd->result = DID_BAD_TARGET << 16;
-			scsi_done(cmd);
-			goto out;
-		}
-
 		res = -ENOMEM;
 		task = sas_create_task(cmd, dev, GFP_ATOMIC);
 		if (!task)

[PATCH 4/4] libsas: Add option for SATA soft reset

[Dan Williams]

From: Dave Jiang <dave.jiang@intel.com>

This allows a libsas driver to optionally provide a soft reset handler
for libata to drive.  The isci driver allows software to control the
assertion/deassertion of SRST.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 include/scsi/libsas.h         |    1 +
 2 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 31fc21f..d6706c5 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -279,6 +279,45 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 	return ret;
 }
 
+static int sas_ata_soft_reset(struct ata_link *link, unsigned int *class,
+			       unsigned long deadline)
+{
+	struct ata_port *ap = link->ap;
+	struct domain_device *dev = ap->private_data;
+	struct sas_internal *i =
+		to_sas_internal(dev->port->ha->core.shost->transportt);
+	int res = TMF_RESP_FUNC_FAILED;
+	int ret = 0;
+
+	if (i->dft->lldd_ata_soft_reset)
+		res = i->dft->lldd_ata_soft_reset(dev);
+
+	if (res != TMF_RESP_FUNC_COMPLETE) {
+		SAS_DPRINTK("%s: Unable to soft reset\n", __func__);
+		ret = -EAGAIN;
+	}
+
+	switch (dev->sata_dev.command_set) {
+		case ATA_COMMAND_SET:
+			SAS_DPRINTK("%s: Found ATA device.\n", __func__);
+			*class = ATA_DEV_ATA;
+			break;
+		case ATAPI_COMMAND_SET:
+			SAS_DPRINTK("%s: Found ATAPI device.\n", __func__);
+			*class = ATA_DEV_ATAPI;
+			break;
+		default:
+			SAS_DPRINTK("%s: Unknown SATA command set: %d.\n",
+				    __func__,
+				    dev->sata_dev.command_set);
+			*class = ATA_DEV_UNKNOWN;
+			break;
+	}
+
+	ap->cbl = ATA_CBL_SATA;
+	return ret;
+}
+
 static void sas_ata_post_internal(struct ata_queued_cmd *qc)
 {
 	if (qc->flags & ATA_QCFLAG_FAILED)
@@ -309,7 +348,7 @@ static void sas_ata_post_internal(struct ata_queued_cmd *qc)
 
 static struct ata_port_operations sas_sata_ops = {
 	.prereset		= ata_std_prereset,
-	.softreset		= NULL,
+	.softreset		= sas_ata_soft_reset,
 	.hardreset		= sas_ata_hard_reset,
 	.postreset		= ata_std_postreset,
 	.error_handler		= ata_std_error_handler,
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index d603b26e..b3418f4 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -617,6 +617,7 @@ struct sas_domain_function_template {
 	int (*lldd_clear_aca)(struct domain_device *, u8 *lun);
 	int (*lldd_clear_task_set)(struct domain_device *, u8 *lun);
 	int (*lldd_I_T_nexus_reset)(struct domain_device *);
+	int (*lldd_ata_soft_reset)(struct domain_device *);
 	int (*lldd_lu_reset)(struct domain_device *, u8 *lun);
 	int (*lldd_query_task)(struct sas_task *);