From: Douglas Gilbert <dgilbert@interlog.com>
To: Jack Wang <jack_wang@usish.com>
Cc: 'Dan Williams' <dan.j.williams@intel.com>,
linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org
Subject: Re: [PATCH v6 2/7] libsas: improve ata debug statements
Date: Sat, 21 Jan 2012 15:26:17 -0500 [thread overview]
Message-ID: <4F1B1F69.5010000@interlog.com> (raw)
In-Reply-To: <FA947937481B428783C02E0BD63531CA@usish.com.cn>
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
>
next prev parent reply other threads:[~2012-01-21 20:26 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-21 1:50 [PATCH v6 0/7] libsas error handling + discovery v6 Dan Williams
2012-01-21 1:50 ` [PATCH v6 1/7] libsas: fix sas_unregister_ports vs sas_drain_work Dan Williams
2012-01-21 1:50 ` [PATCH v6 2/7] libsas: improve ata debug statements Dan Williams
2012-01-21 5:36 ` Jack Wang
2012-01-21 20:26 ` Douglas Gilbert [this message]
2012-01-23 20:51 ` Dan Williams
2012-01-21 1:50 ` [PATCH v6 3/7] libsas: fix sas port naming Dan Williams
2012-01-21 1:51 ` [PATCH v6 4/7] libsas: let libata recover links that fail to transmit initial sig-fis Dan Williams
2012-01-21 1:51 ` [PATCH v6 5/7] libsas: delete device on sas address changed Dan Williams
2012-01-21 1:51 ` [PATCH v6 6/7] libsas: restore scan order Dan Williams
2012-01-21 1:51 ` [PATCH v6 7/7] libsas: async ata scanning Dan Williams
2012-01-21 6:12 ` [PATCH v6 0/7] libsas error handling + discovery v6 Jack Wang
2012-01-23 20:57 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F1B1F69.5010000@interlog.com \
--to=dgilbert@interlog.com \
--cc=dan.j.williams@intel.com \
--cc=jack_wang@usish.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.