From 79487259a4c90acf020f6ce1c63bb1db9ad3fc4d Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 9 May 2018 09:28:08 +0900 Subject: libata: Fix comment typo in ata_eh_analyze_tf() Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index c016829a38fd..6c9d3fc5212f 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1866,10 +1866,10 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc, if (qc->flags & ATA_QCFLAG_SENSE_VALID) { int ret = scsi_check_sense(qc->scsicmd); /* - * SUCCESS here means that the sense code could + * SUCCESS here means that the sense code could be * evaluated and should be passed to the upper layers * for correct evaluation. - * FAILED means the sense code could not interpreted + * FAILED means the sense code could not be interpreted * and the device would need to be reset. * NEEDS_RETRY and ADD_TO_MLQUEUE means that the * command would need to be retried. -- cgit v1.2.3 From 54fb131b325c663b368b2be150f6875124df1d76 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 9 May 2018 09:28:09 +0900 Subject: libata: Fix ata_err_string() Add proper error string output for ATA_ERR_NCQ and ATA_ERR_NODEV_HINT instead of returning "unknown error". Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 6c9d3fc5212f..e85ab60b462c 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1483,6 +1483,10 @@ static const char *ata_err_string(unsigned int err_mask) return "invalid argument"; if (err_mask & AC_ERR_DEV) return "device error"; + if (err_mask & AC_ERR_NCQ) + return "NCQ error"; + if (err_mask & AC_ERR_NODEV_HINT) + return "Polling detection error"; return "unknown error"; } -- cgit v1.2.3 From 7eb49509dd6b2a4ed0c18a0f8c187afbacf98b42 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 9 May 2018 09:28:11 +0900 Subject: libata: Honor RQF_QUIET flag Currently, libata ignores requests RQF_QUIET flag and print error messages for failed commands, regardless if this flag is set in the command request. Fix this by introducing the ata_eh_quiet() function and using this function in ata_eh_link_autopsy() to determine if the EH context should be quiet. This works by counting the number of failed commands and the number of commands with the quiet flag set. If both numbers are equal, the the EH context can be set to quiet and all error messages suppressed. Otherwise, only the error messages for the failed commands are suppressed and the link Emask and irq_stat messages printed. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 26 +++++++++++++++++++++++++- drivers/ata/libata-scsi.c | 3 +++ 2 files changed, 28 insertions(+), 1 deletion(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index e85ab60b462c..b933357edc22 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2153,6 +2153,21 @@ static inline int ata_eh_worth_retry(struct ata_queued_cmd *qc) return qc->err_mask != AC_ERR_DEV; /* retry if not dev error */ } +/** + * ata_eh_quiet - check if we need to be quiet about a command error + * @qc: qc to check + * + * Look at the qc flags anbd its scsi command request flags to determine + * if we need to be quiet about the command failure. + */ +static inline bool ata_eh_quiet(struct ata_queued_cmd *qc) +{ + if (qc->scsicmd && + qc->scsicmd->request->rq_flags & RQF_QUIET) + qc->flags |= ATA_QCFLAG_QUIET; + return qc->flags & ATA_QCFLAG_QUIET; +} + /** * ata_eh_link_autopsy - analyze error and determine recovery action * @link: host link to perform autopsy on @@ -2170,7 +2185,7 @@ static void ata_eh_link_autopsy(struct ata_link *link) struct ata_eh_context *ehc = &link->eh_context; struct ata_device *dev; unsigned int all_err_mask = 0, eflags = 0; - int tag; + int tag, nr_failed = 0, nr_quiet = 0; u32 serror; int rc; @@ -2236,8 +2251,17 @@ static void ata_eh_link_autopsy(struct ata_link *link) if (qc->flags & ATA_QCFLAG_IO) eflags |= ATA_EFLAG_IS_IO; trace_ata_eh_link_autopsy_qc(qc); + + /* Count quiet errors */ + if (ata_eh_quiet(qc)) + nr_quiet++; + nr_failed++; } + /* If all failed commands requested silence, then be quiet */ + if (nr_quiet == nr_failed) + ehc->i.flags |= ATA_EHI_QUIET; + /* enforce default EH actions */ if (ap->pflags & ATA_PFLAG_FROZEN || all_err_mask & (AC_ERR_HSM | AC_ERR_TIMEOUT)) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 89a9d4a2efc8..8d76de9189e4 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -872,6 +872,9 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct ata_device *dev, qc->sg = scsi_sglist(cmd); qc->n_elem = scsi_sg_count(cmd); + + if (cmd->request->rq_flags & RQF_QUIET) + qc->flags |= ATA_QCFLAG_QUIET; } else { cmd->result = (DID_OK << 16) | (QUEUE_FULL << 1); cmd->scsi_done(cmd); -- cgit v1.2.3 From 804689ad2d9b66d0d3920b48cf05881049d44589 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 9 May 2018 09:28:12 +0900 Subject: libata: Fix command retry decision For failed commands with valid sense data (e.g. NCQ commands), scsi_check_sense() is used in ata_analyze_tf() to determine if the command can be retried. In such case, rely on this decision and ignore the command error mask based decision done in ata_worth_retry(). This fixes useless retries of commands such as unaligned writes on zoned disks (TYPE_ZAC). Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index b933357edc22..1035de1e5120 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2237,12 +2237,16 @@ static void ata_eh_link_autopsy(struct ata_link *link) if (qc->err_mask & ~AC_ERR_OTHER) qc->err_mask &= ~AC_ERR_OTHER; - /* SENSE_VALID trumps dev/unknown error and revalidation */ + /* + * SENSE_VALID trumps dev/unknown error and revalidation. Upper + * layers will determine whether the command is worth retrying + * based on the sense data and device class/type. Otherwise, + * determine directly if the command is worth retrying using its + * error mask and flags. + */ if (qc->flags & ATA_QCFLAG_SENSE_VALID) qc->err_mask &= ~(AC_ERR_DEV | AC_ERR_OTHER); - - /* determine whether the command is worth retrying */ - if (ata_eh_worth_retry(qc)) + else if (ata_eh_worth_retry(qc)) qc->flags |= ATA_QCFLAG_RETRY; /* accumulate error info */ -- cgit v1.2.3 From 9d207accca2c262206d0a9327ef1444c70cc0bdf Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 11 May 2018 12:51:07 -0600 Subject: libata: remove assumption that ATA_MAX_QUEUE - 1 is the max In a few spots we iterate to ATA_MAX_QUEUE -1, including internal knowledge that the last tag is the internal tag. Remove this assumption. Signed-off-by: Jens Axboe Signed-off-by: Tejun Heo --- drivers/ata/libata-eh.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 1035de1e5120..9f9aad77fbcd 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -873,9 +873,12 @@ static int ata_eh_nr_in_flight(struct ata_port *ap) int nr = 0; /* count only non-internal commands */ - for (tag = 0; tag < ATA_MAX_QUEUE - 1; tag++) + for (tag = 0; tag < ATA_MAX_QUEUE; tag++) { + if (ata_tag_internal(tag)) + continue; if (ata_qc_from_tag(ap, tag)) nr++; + } return nr; } @@ -900,7 +903,7 @@ void ata_eh_fastdrain_timerfn(struct timer_list *t) /* No progress during the last interval, tag all * in-flight qcs as timed out and freeze the port. */ - for (tag = 0; tag < ATA_MAX_QUEUE - 1; tag++) { + for (tag = 0; tag < ATA_MAX_QUEUE; tag++) { struct ata_queued_cmd *qc = ata_qc_from_tag(ap, tag); if (qc) qc->err_mask |= AC_ERR_TIMEOUT; -- cgit v1.2.3 From 28361c403683c2b00d4f5e76045f3ccd299bf99d Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 11 May 2018 12:51:09 -0600 Subject: libata: add extra internal command Bump the internal tag to 32, instead of stealing the last tag in our regular command space. This works just fine, since we don't actually need a separate hardware tag for this. Internal commands cannot coexist with NCQ commands. As a bonus, we get rid of the special casing of what tag to use for the internal command. This is in preparation for utilizing all 32 commands for normal IO. Signed-off-by: Jens Axboe Signed-off-by: Tejun Heo --- drivers/ata/libata-core.c | 22 ++++++---------------- drivers/ata/libata-eh.c | 3 ++- include/linux/libata.h | 15 +++++++-------- 3 files changed, 15 insertions(+), 25 deletions(-) (limited to 'drivers/ata/libata-eh.c') diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 8fd352d4c190..5e2f679322cc 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1570,7 +1570,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev, u8 command = tf->command; int auto_timeout = 0; struct ata_queued_cmd *qc; - unsigned int tag, preempted_tag; + unsigned int preempted_tag; u32 preempted_sactive; u64 preempted_qc_active; int preempted_nr_active_links; @@ -1588,20 +1588,10 @@ unsigned ata_exec_internal_sg(struct ata_device *dev, } /* initialize internal qc */ + qc = __ata_qc_from_tag(ap, ATA_TAG_INTERNAL); - /* XXX: Tag 0 is used for drivers with legacy EH as some - * drivers choke if any other tag is given. This breaks - * ata_tag_internal() test for those drivers. Don't use new - * EH stuff without converting to it. - */ - if (ap->ops->error_handler) - tag = ATA_TAG_INTERNAL; - else - tag = 0; - - qc = __ata_qc_from_tag(ap, tag); - - qc->tag = qc->hw_tag = tag; + qc->tag = ATA_TAG_INTERNAL; + qc->hw_tag = 0; qc->scsicmd = NULL; qc->ap = ap; qc->dev = dev; @@ -5156,7 +5146,7 @@ void ata_qc_free(struct ata_queued_cmd *qc) qc->flags = 0; tag = qc->tag; - if (likely(ata_tag_valid(tag))) { + if (ata_tag_valid(tag)) { qc->tag = ATA_TAG_POISON; if (ap->flags & ATA_FLAG_SAS_HOST) ata_sas_free_tag(tag, ap); @@ -5415,7 +5405,7 @@ void ata_qc_issue(struct ata_queued_cmd *qc) WARN_ON_ONCE(link->sactive); ap->nr_active_links++; - link->active_tag = qc->hw_tag; + link->active_tag = qc->tag; } qc->flags |= ATA_QCFLAG_ACTIVE; diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 9f9aad77fbcd..eadbe26ba3dc 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1057,7 +1057,8 @@ static int ata_do_link_abort(struct ata_port *ap, struct ata_link *link) /* we're gonna abort all commands, no need for fast drain */ ata_eh_set_pending(ap, 0); - for (tag = 0; tag < ATA_MAX_QUEUE; tag++) { + /* include internal tag in iteration */ + for (tag = 0; tag <= ATA_MAX_QUEUE; tag++) { struct ata_queued_cmd *qc = ata_qc_from_tag(ap, tag); if (qc && (!link || qc->dev->link == link)) { diff --git a/include/linux/libata.h b/include/linux/libata.h index 60ce1ba26fdd..1d026719461b 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -125,9 +125,8 @@ enum { LIBATA_MAX_PRD = ATA_MAX_PRD / 2, LIBATA_DUMB_MAX_PRD = ATA_MAX_PRD / 4, /* Worst case */ ATA_DEF_QUEUE = 1, - /* tag ATA_MAX_QUEUE - 1 is reserved for internal commands */ ATA_MAX_QUEUE = 32, - ATA_TAG_INTERNAL = ATA_MAX_QUEUE - 1, + ATA_TAG_INTERNAL = ATA_MAX_QUEUE, ATA_SHORT_PAUSE = 16, ATAPI_MAX_DRAIN = 16 << 10, @@ -850,7 +849,7 @@ struct ata_port { unsigned int udma_mask; unsigned int cbl; /* cable type; ATA_CBL_xxx */ - struct ata_queued_cmd qcmd[ATA_MAX_QUEUE]; + struct ata_queued_cmd qcmd[ATA_MAX_QUEUE + 1]; unsigned long sas_tag_allocated; /* for sas tag allocation only */ u64 qc_active; int nr_active_links; /* #links with active qcs */ @@ -1486,14 +1485,14 @@ extern void ata_port_pbar_desc(struct ata_port *ap, int bar, ssize_t offset, const char *name); #endif -static inline unsigned int ata_tag_valid(unsigned int tag) +static inline bool ata_tag_internal(unsigned int tag) { - return (tag < ATA_MAX_QUEUE) ? 1 : 0; + return tag == ATA_TAG_INTERNAL; } -static inline bool ata_tag_internal(unsigned int tag) +static inline bool ata_tag_valid(unsigned int tag) { - return tag == ATA_TAG_INTERNAL; + return tag < ATA_MAX_QUEUE || ata_tag_internal(tag); } /* @@ -1656,7 +1655,7 @@ static inline void ata_qc_set_polling(struct ata_queued_cmd *qc) static inline struct ata_queued_cmd *__ata_qc_from_tag(struct ata_port *ap, unsigned int tag) { - if (likely(ata_tag_valid(tag))) + if (ata_tag_valid(tag)) return &ap->qcmd[tag]; return NULL; } -- cgit v1.2.3