Patchwork [v6,2/7] libsas: improve ata debug statements

login
register
mail settings
Submitter Dan Williams
Date Jan. 21, 2012, 1:50 a.m.
Message ID <20120121015049.24930.6244.stgit@localhost6.localdomain6>
Download mbox | patch
Permalink /patch/137135/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Dan Williams - Jan. 21, 2012, 1:50 a.m.
It's difficult to determine which domain_device is triggering error recovery,
so convert messages like:

  sas: ex 5001b4da000e703f phy08:T attached: 5001b4da000e7028
  sas: ex 5001b4da000e703f phy09:T attached: 5001b4da000e7029
  ...
  ata7: sas eh calling libata port error handler
  ata8: sas eh calling libata port error handler

...into:

  sas: ex 5001517e85cfefff phy05:T:9 attached: 5001517e85cfefe5 (stp)
  sas: ex 5001517e3b0af0bf phy11:T:8 attached: 5001517e3b0af0ab (stp)
  ...
  sas: ata7: end_device-21:1: dev error handler
  sas: ata8: end_device-20:0:5: dev error handler

which shows attached link rate, device type, and associates a
domain_device with its ata_port id to correlate messages emitted from
libata-eh.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/scsi/libsas/sas_ata.c      |   43 ++++++++++++++++++++++++++++--------
 drivers/scsi/libsas/sas_expander.c |   38 ++++++++++++++++++++++++++------
 2 files changed, 64 insertions(+), 17 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jack Wang - Jan. 21, 2012, 5:36 a.m.
> 

[Jack Wang] 
Nice improvement.
Thanks.
> It's difficult to determine which domain_device is triggering error
recovery,
> so convert messages like:
> 
>   sas: ex 5001b4da000e703f phy08:T attached: 5001b4da000e7028
>   sas: ex 5001b4da000e703f phy09:T attached: 5001b4da000e7029
>   ...
>   ata7: sas eh calling libata port error handler
>   ata8: sas eh calling libata port error handler
> 
> ...into:
> 
>   sas: ex 5001517e85cfefff phy05:T:9 attached: 5001517e85cfefe5 (stp)
>   sas: ex 5001517e3b0af0bf phy11:T:8 attached: 5001517e3b0af0ab (stp)
>   ...
>   sas: ata7: end_device-21:1: dev error handler
>   sas: ata8: end_device-20:0:5: dev error handler
> 
> which shows attached link rate, device type, and associates a
> domain_device with its ata_port id to correlate messages emitted from
> libata-eh.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/scsi/libsas/sas_ata.c      |   43
> ++++++++++++++++++++++++++++--------
>  drivers/scsi/libsas/sas_expander.c |   38
++++++++++++++++++++++++++------
>  2 files changed, 64 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index 0ee6831..b43e395 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -317,6 +317,28 @@ static int local_ata_check_ready(struct ata_link
*link)
>  	}
>  }
> 
> +static int sas_ata_printk(const char *level, const struct domain_device
> *ddev,
> +		          const char *fmt, ...)
> +{
> +	struct ata_port *ap = ddev->sata_dev.ap;
> +	struct device *dev = &ddev->rphy->dev;
> +	struct va_format vaf;
> +	va_list args;
> +	int r;
> +
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	r = printk("%ssas: ata%u: %s: %pV",
> +		   level, ap->print_id, dev_name(dev), &vaf);
> +
> +	va_end(args);
> +
> +	return r;
> +}
> +
>  static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
>  			      unsigned long deadline)
>  {
> @@ -333,7 +355,7 @@ static int sas_ata_hard_reset(struct ata_link *link,
> unsigned int *class,
>  	res = i->dft->lldd_I_T_nexus_reset(dev);
> 
>  	if (res != TMF_RESP_FUNC_COMPLETE)
> -		SAS_DPRINTK("%s: Unable to reset ata device?\n", __func__);
> +		sas_ata_printk(KERN_DEBUG, dev, "Unable to reset ata
device?\n");
> 
>  	phy = sas_get_local_phy(dev);
>  	if (scsi_is_sas_phy_local(phy))
> @@ -344,7 +366,7 @@ static int sas_ata_hard_reset(struct ata_link *link,
> unsigned int *class,
> 
>  	ret = ata_wait_after_reset(link, deadline, check_ready);
>  	if (ret && ret != -EAGAIN)
> -		ata_link_err(link, "COMRESET failed (errno=%d)\n", ret);
> +		sas_ata_printk(KERN_ERR, dev, "reset failed (errno=%d)\n",
ret);
> 
>  	/* XXX: if the class changes during the reset the upper layer
>  	 * should be informed, if the device has gone away we assume
> @@ -665,7 +687,7 @@ static void async_sas_ata_eh(void *data,
async_cookie_t
> cookie)
>  	 * remove once all commands are completed
>  	 */
>  	kref_get(&dev->kref);
> -	ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error
> handler");
> +	sas_ata_printk(KERN_DEBUG, dev, "dev error handler\n");
>  	ata_scsi_port_error_handler(ha->core.shost, ap);
>  	sas_put_device(dev);
> 
> @@ -708,26 +730,27 @@ void sas_ata_eh(struct Scsi_Host *shost, struct
> list_head *work_q,
>  		struct list_head *done_q)
>  {
>  	struct scsi_cmnd *cmd, *n;
> -	struct ata_port *ap;
> +	struct domain_device *eh_dev;
> 
>  	do {
>  		LIST_HEAD(sata_q);
> -
> -		ap = NULL;
> +		eh_dev = NULL;
> 
>  		list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
>  			struct domain_device *ddev = cmd_to_domain_dev(cmd);
> 
>  			if (!dev_is_sata(ddev) || TO_SAS_TASK(cmd))
>  				continue;
> -			if (ap && ap != ddev->sata_dev.ap)
> +			if (eh_dev && eh_dev != ddev)
>  				continue;
> -			ap = ddev->sata_dev.ap;
> +			eh_dev = ddev;
>  			list_move(&cmd->eh_entry, &sata_q);
>  		}
> 
>  		if (!list_empty(&sata_q)) {
> -			ata_port_printk(ap, KERN_DEBUG, "sas eh calling
libata cmd
> error handler\n");
> +			struct ata_port *ap = eh_dev->sata_dev.ap;
> +
> +			sas_ata_printk(KERN_DEBUG, eh_dev, "cmd error
handler\n");
>  			ata_scsi_cmd_error_handler(shost, ap, &sata_q);
>  			/*
>  			 * ata's error handler may leave the cmd on the list
> @@ -743,7 +766,7 @@ void sas_ata_eh(struct Scsi_Host *shost, struct
list_head
> *work_q,
>  			while (!list_empty(&sata_q))
>  				list_del_init(sata_q.next);
>  		}
> -	} while (ap);
> +	} while (eh_dev);
>  }
> 
>  void sas_ata_schedule_reset(struct domain_device *dev)
> diff --git a/drivers/scsi/libsas/sas_expander.c
> b/drivers/scsi/libsas/sas_expander.c
> index 68a80a0..f8d941f 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -176,9 +176,10 @@ static void sas_set_ex_phy(struct domain_device *dev,
int
> phy_id,
>  	struct smp_resp *resp = disc_resp;
>  	struct discover_resp *dr = &resp->disc;
>  	struct sas_rphy *rphy = dev->rphy;
> -	int rediscover = (phy->phy != NULL);
> +	bool new_phy = !phy->phy;
> +	char *type;
> 
> -	if (!rediscover) {
> +	if (new_phy) {
>  		phy->phy = sas_phy_alloc(&rphy->dev, phy_id);
> 
>  		/* FIXME: error_handling */
> @@ -223,20 +224,43 @@ static void sas_set_ex_phy(struct domain_device
*dev,
> int phy_id,
>  	phy->phy->maximum_linkrate = dr->pmax_linkrate;
>  	phy->phy->negotiated_linkrate = phy->linkrate;
> 
> -	if (!rediscover)
> +	if (new_phy)
>  		if (sas_phy_add(phy->phy)) {
>  			sas_phy_free(phy->phy);
>  			return;
>  		}
> 
> -	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n",
> +	switch (phy->attached_dev_type) {
> +	case NO_DEVICE:
> +		type = "no device";
> +		break;
> +	case SAS_END_DEV:
> +		if (phy->attached_iproto) {
> +			if (phy->attached_tproto)
> +				type = "host+target";
> +			else
> +				type = "host";
> +		} else {
> +			if (dr->attached_sata_dev)
> +				type = "stp";
> +			else
> +				type = "ssp";
> +		}
> +		break;
> +	case EDGE_DEV:
> +	case FANOUT_DEV:
> +		type = "smp";
> +		break;
> +	default:
> +		type = "unknown";
> +	}
> +
> +	SAS_DPRINTK("ex %016llx phy%02d:%c:%X attached: %016llx (%s)\n",
>  		    SAS_ADDR(dev->sas_addr), phy->phy_id,
>  		    phy->routing_attr == TABLE_ROUTING ? 'T' :
>  		    phy->routing_attr == DIRECT_ROUTING ? 'D' :
>  		    phy->routing_attr == SUBTRACTIVE_ROUTING ? 'S' : '?',
> -		    SAS_ADDR(phy->attached_sas_addr));
> -
> -	return;
> +		    phy->linkrate, SAS_ADDR(phy->attached_sas_addr), type);
>  }
> 
>  /* check if we have an existing attached ata device on this expander phy
*/
> 
> --
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Douglas Gilbert - Jan. 21, 2012, 8:26 p.m.
On 12-01-21 12:36 AM, Jack Wang wrote:
>>
>
> [Jack Wang]
> Nice improvement.
> Thanks.
>> It's difficult to determine which domain_device is triggering error
> recovery,
>> so convert messages like:
>>
>>    sas: ex 5001b4da000e703f phy08:T attached: 5001b4da000e7028
>>    sas: ex 5001b4da000e703f phy09:T attached: 5001b4da000e7029
>>    ...
>>    ata7: sas eh calling libata port error handler
>>    ata8: sas eh calling libata port error handler
>>
>> ...into:
>>
>>    sas: ex 5001517e85cfefff phy05:T:9 attached: 5001517e85cfefe5 (stp)
>>    sas: ex 5001517e3b0af0bf phy11:T:8 attached: 5001517e3b0af0ab (stp)
>>    ...
>>    sas: ata7: end_device-21:1: dev error handler
>>    sas: ata8: end_device-20:0:5: dev error handler
>>
>> which shows attached link rate, device type, and associates a
>> domain_device with its ata_port id to correlate messages emitted from
>> libata-eh.

A couple of comments. The "T" in the "phy08:T" stands for
an expander phy's routing attribute being "table" I presume.
In smp_utils I'm thinking of changing that to "U" in the case
where that expander (SAS-2 or later) supports table-to-table
routing. The "U" indicates that phy can join other (sibling)
expander phys to become an "enclosure universal port" which
is nirvana for SAS expanders.

enum sas_dev_type::EDGE_DEV seems a pretty bad name and
should be changed to EXPANDER_DEV as that is more
accurate. Where fanout expanders ever made in SAS-1?
Not many I would expect. In SAS-2++ (SPL, SPL-2) the
"edge" and "fanout" distinction has been dropped and
the numerical value for fanout expanders (3) has been
marked as obsolete.

Doug Gilbert

>> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
>> ---
>>   drivers/scsi/libsas/sas_ata.c      |   43
>> ++++++++++++++++++++++++++++--------
>>   drivers/scsi/libsas/sas_expander.c |   38
> ++++++++++++++++++++++++++------
>>   2 files changed, 64 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
>> index 0ee6831..b43e395 100644
>> --- a/drivers/scsi/libsas/sas_ata.c
>> +++ b/drivers/scsi/libsas/sas_ata.c
>> @@ -317,6 +317,28 @@ static int local_ata_check_ready(struct ata_link
> *link)
>>   	}
>>   }
>>
>> +static int sas_ata_printk(const char *level, const struct domain_device
>> *ddev,
>> +		          const char *fmt, ...)
>> +{
>> +	struct ata_port *ap = ddev->sata_dev.ap;
>> +	struct device *dev =&ddev->rphy->dev;
>> +	struct va_format vaf;
>> +	va_list args;
>> +	int r;
>> +
>> +	va_start(args, fmt);
>> +
>> +	vaf.fmt = fmt;
>> +	vaf.va =&args;
>> +
>> +	r = printk("%ssas: ata%u: %s: %pV",
>> +		   level, ap->print_id, dev_name(dev),&vaf);
>> +
>> +	va_end(args);
>> +
>> +	return r;
>> +}
>> +
>>   static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
>>   			      unsigned long deadline)
>>   {
>> @@ -333,7 +355,7 @@ static int sas_ata_hard_reset(struct ata_link *link,
>> unsigned int *class,
>>   	res = i->dft->lldd_I_T_nexus_reset(dev);
>>
>>   	if (res != TMF_RESP_FUNC_COMPLETE)
>> -		SAS_DPRINTK("%s: Unable to reset ata device?\n", __func__);
>> +		sas_ata_printk(KERN_DEBUG, dev, "Unable to reset ata
> device?\n");
>>
>>   	phy = sas_get_local_phy(dev);
>>   	if (scsi_is_sas_phy_local(phy))
>> @@ -344,7 +366,7 @@ static int sas_ata_hard_reset(struct ata_link *link,
>> unsigned int *class,
>>
>>   	ret = ata_wait_after_reset(link, deadline, check_ready);
>>   	if (ret&&  ret != -EAGAIN)
>> -		ata_link_err(link, "COMRESET failed (errno=%d)\n", ret);
>> +		sas_ata_printk(KERN_ERR, dev, "reset failed (errno=%d)\n",
> ret);
>>
>>   	/* XXX: if the class changes during the reset the upper layer
>>   	 * should be informed, if the device has gone away we assume
>> @@ -665,7 +687,7 @@ static void async_sas_ata_eh(void *data,
> async_cookie_t
>> cookie)
>>   	 * remove once all commands are completed
>>   	 */
>>   	kref_get(&dev->kref);
>> -	ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error
>> handler");
>> +	sas_ata_printk(KERN_DEBUG, dev, "dev error handler\n");
>>   	ata_scsi_port_error_handler(ha->core.shost, ap);
>>   	sas_put_device(dev);
>>
>> @@ -708,26 +730,27 @@ void sas_ata_eh(struct Scsi_Host *shost, struct
>> list_head *work_q,
>>   		struct list_head *done_q)
>>   {
>>   	struct scsi_cmnd *cmd, *n;
>> -	struct ata_port *ap;
>> +	struct domain_device *eh_dev;
>>
>>   	do {
>>   		LIST_HEAD(sata_q);
>> -
>> -		ap = NULL;
>> +		eh_dev = NULL;
>>
>>   		list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
>>   			struct domain_device *ddev = cmd_to_domain_dev(cmd);
>>
>>   			if (!dev_is_sata(ddev) || TO_SAS_TASK(cmd))
>>   				continue;
>> -			if (ap&&  ap != ddev->sata_dev.ap)
>> +			if (eh_dev&&  eh_dev != ddev)
>>   				continue;
>> -			ap = ddev->sata_dev.ap;
>> +			eh_dev = ddev;
>>   			list_move(&cmd->eh_entry,&sata_q);
>>   		}
>>
>>   		if (!list_empty(&sata_q)) {
>> -			ata_port_printk(ap, KERN_DEBUG, "sas eh calling
> libata cmd
>> error handler\n");
>> +			struct ata_port *ap = eh_dev->sata_dev.ap;
>> +
>> +			sas_ata_printk(KERN_DEBUG, eh_dev, "cmd error
> handler\n");
>>   			ata_scsi_cmd_error_handler(shost, ap,&sata_q);
>>   			/*
>>   			 * ata's error handler may leave the cmd on the list
>> @@ -743,7 +766,7 @@ void sas_ata_eh(struct Scsi_Host *shost, struct
> list_head
>> *work_q,
>>   			while (!list_empty(&sata_q))
>>   				list_del_init(sata_q.next);
>>   		}
>> -	} while (ap);
>> +	} while (eh_dev);
>>   }
>>
>>   void sas_ata_schedule_reset(struct domain_device *dev)
>> diff --git a/drivers/scsi/libsas/sas_expander.c
>> b/drivers/scsi/libsas/sas_expander.c
>> index 68a80a0..f8d941f 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -176,9 +176,10 @@ static void sas_set_ex_phy(struct domain_device *dev,
> int
>> phy_id,
>>   	struct smp_resp *resp = disc_resp;
>>   	struct discover_resp *dr =&resp->disc;
>>   	struct sas_rphy *rphy = dev->rphy;
>> -	int rediscover = (phy->phy != NULL);
>> +	bool new_phy = !phy->phy;
>> +	char *type;
>>
>> -	if (!rediscover) {
>> +	if (new_phy) {
>>   		phy->phy = sas_phy_alloc(&rphy->dev, phy_id);
>>
>>   		/* FIXME: error_handling */
>> @@ -223,20 +224,43 @@ static void sas_set_ex_phy(struct domain_device
> *dev,
>> int phy_id,
>>   	phy->phy->maximum_linkrate = dr->pmax_linkrate;
>>   	phy->phy->negotiated_linkrate = phy->linkrate;
>>
>> -	if (!rediscover)
>> +	if (new_phy)
>>   		if (sas_phy_add(phy->phy)) {
>>   			sas_phy_free(phy->phy);
>>   			return;
>>   		}
>>
>> -	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n",
>> +	switch (phy->attached_dev_type) {
>> +	case NO_DEVICE:
>> +		type = "no device";
>> +		break;
>> +	case SAS_END_DEV:
>> +		if (phy->attached_iproto) {
>> +			if (phy->attached_tproto)
>> +				type = "host+target";
>> +			else
>> +				type = "host";
>> +		} else {
>> +			if (dr->attached_sata_dev)
>> +				type = "stp";
>> +			else
>> +				type = "ssp";
>> +		}
>> +		break;
>> +	case EDGE_DEV:
>> +	case FANOUT_DEV:
>> +		type = "smp";
>> +		break;
>> +	default:
>> +		type = "unknown";
>> +	}
>> +
>> +	SAS_DPRINTK("ex %016llx phy%02d:%c:%X attached: %016llx (%s)\n",
>>   		    SAS_ADDR(dev->sas_addr), phy->phy_id,
>>   		    phy->routing_attr == TABLE_ROUTING ? 'T' :
>>   		    phy->routing_attr == DIRECT_ROUTING ? 'D' :
>>   		    phy->routing_attr == SUBTRACTIVE_ROUTING ? 'S' : '?',
>> -		    SAS_ADDR(phy->attached_sas_addr));
>> -
>> -	return;
>> +		    phy->linkrate, SAS_ADDR(phy->attached_sas_addr), type);
>>   }
>>
>>   /* check if we have an existing attached ata device on this expander phy
> */
>>
>> --
>> 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
>
> --
> 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
>

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

Patch

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 0ee6831..b43e395 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -317,6 +317,28 @@  static int local_ata_check_ready(struct ata_link *link)
 	}
 }
 
+static int sas_ata_printk(const char *level, const struct domain_device *ddev,
+		          const char *fmt, ...)
+{
+	struct ata_port *ap = ddev->sata_dev.ap;
+	struct device *dev = &ddev->rphy->dev;
+	struct va_format vaf;
+	va_list args;
+	int r;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	r = printk("%ssas: ata%u: %s: %pV",
+		   level, ap->print_id, dev_name(dev), &vaf);
+
+	va_end(args);
+
+	return r;
+}
+
 static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 			      unsigned long deadline)
 {
@@ -333,7 +355,7 @@  static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 	res = i->dft->lldd_I_T_nexus_reset(dev);
 
 	if (res != TMF_RESP_FUNC_COMPLETE)
-		SAS_DPRINTK("%s: Unable to reset ata device?\n", __func__);
+		sas_ata_printk(KERN_DEBUG, dev, "Unable to reset ata device?\n");
 
 	phy = sas_get_local_phy(dev);
 	if (scsi_is_sas_phy_local(phy))
@@ -344,7 +366,7 @@  static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 
 	ret = ata_wait_after_reset(link, deadline, check_ready);
 	if (ret && ret != -EAGAIN)
-		ata_link_err(link, "COMRESET failed (errno=%d)\n", ret);
+		sas_ata_printk(KERN_ERR, dev, "reset failed (errno=%d)\n", ret);
 
 	/* XXX: if the class changes during the reset the upper layer
 	 * should be informed, if the device has gone away we assume
@@ -665,7 +687,7 @@  static void async_sas_ata_eh(void *data, async_cookie_t cookie)
 	 * remove once all commands are completed
 	 */
 	kref_get(&dev->kref);
-	ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata port error handler");
+	sas_ata_printk(KERN_DEBUG, dev, "dev error handler\n");
 	ata_scsi_port_error_handler(ha->core.shost, ap);
 	sas_put_device(dev);
 
@@ -708,26 +730,27 @@  void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 		struct list_head *done_q)
 {
 	struct scsi_cmnd *cmd, *n;
-	struct ata_port *ap;
+	struct domain_device *eh_dev;
 
 	do {
 		LIST_HEAD(sata_q);
-
-		ap = NULL;
+		eh_dev = NULL;
 
 		list_for_each_entry_safe(cmd, n, work_q, eh_entry) {
 			struct domain_device *ddev = cmd_to_domain_dev(cmd);
 
 			if (!dev_is_sata(ddev) || TO_SAS_TASK(cmd))
 				continue;
-			if (ap && ap != ddev->sata_dev.ap)
+			if (eh_dev && eh_dev != ddev)
 				continue;
-			ap = ddev->sata_dev.ap;
+			eh_dev = ddev;
 			list_move(&cmd->eh_entry, &sata_q);
 		}
 
 		if (!list_empty(&sata_q)) {
-			ata_port_printk(ap, KERN_DEBUG, "sas eh calling libata cmd error handler\n");
+			struct ata_port *ap = eh_dev->sata_dev.ap;
+
+			sas_ata_printk(KERN_DEBUG, eh_dev, "cmd error handler\n");
 			ata_scsi_cmd_error_handler(shost, ap, &sata_q);
 			/*
 			 * ata's error handler may leave the cmd on the list
@@ -743,7 +766,7 @@  void sas_ata_eh(struct Scsi_Host *shost, struct list_head *work_q,
 			while (!list_empty(&sata_q))
 				list_del_init(sata_q.next);
 		}
-	} while (ap);
+	} while (eh_dev);
 }
 
 void sas_ata_schedule_reset(struct domain_device *dev)
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 68a80a0..f8d941f 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -176,9 +176,10 @@  static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 	struct smp_resp *resp = disc_resp;
 	struct discover_resp *dr = &resp->disc;
 	struct sas_rphy *rphy = dev->rphy;
-	int rediscover = (phy->phy != NULL);
+	bool new_phy = !phy->phy;
+	char *type;
 
-	if (!rediscover) {
+	if (new_phy) {
 		phy->phy = sas_phy_alloc(&rphy->dev, phy_id);
 
 		/* FIXME: error_handling */
@@ -223,20 +224,43 @@  static void sas_set_ex_phy(struct domain_device *dev, int phy_id,
 	phy->phy->maximum_linkrate = dr->pmax_linkrate;
 	phy->phy->negotiated_linkrate = phy->linkrate;
 
-	if (!rediscover)
+	if (new_phy)
 		if (sas_phy_add(phy->phy)) {
 			sas_phy_free(phy->phy);
 			return;
 		}
 
-	SAS_DPRINTK("ex %016llx phy%02d:%c attached: %016llx\n",
+	switch (phy->attached_dev_type) {
+	case NO_DEVICE:
+		type = "no device";
+		break;
+	case SAS_END_DEV:
+		if (phy->attached_iproto) {
+			if (phy->attached_tproto)
+				type = "host+target";
+			else
+				type = "host";
+		} else {
+			if (dr->attached_sata_dev)
+				type = "stp";
+			else
+				type = "ssp";
+		}
+		break;
+	case EDGE_DEV:
+	case FANOUT_DEV:
+		type = "smp";
+		break;
+	default:
+		type = "unknown";
+	}
+
+	SAS_DPRINTK("ex %016llx phy%02d:%c:%X attached: %016llx (%s)\n",
 		    SAS_ADDR(dev->sas_addr), phy->phy_id,
 		    phy->routing_attr == TABLE_ROUTING ? 'T' :
 		    phy->routing_attr == DIRECT_ROUTING ? 'D' :
 		    phy->routing_attr == SUBTRACTIVE_ROUTING ? 'S' : '?',
-		    SAS_ADDR(phy->attached_sas_addr));
-
-	return;
+		    phy->linkrate, SAS_ADDR(phy->attached_sas_addr), type);
 }
 
 /* check if we have an existing attached ata device on this expander phy */