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 +++++++++++++++++++++++++- 1 file changed, 25 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)) -- 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-eh.c | 3 ++- 1 file changed, 2 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 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)) { -- cgit v1.2.3