From patchwork Thu Mar 1 19:40:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Damien Le Moal X-Patchwork-Id: 10252319 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 49F9160365 for ; Thu, 1 Mar 2018 19:40:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 37FBB28521 for ; Thu, 1 Mar 2018 19:40:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2C08D28534; Thu, 1 Mar 2018 19:40:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 841BE28521 for ; Thu, 1 Mar 2018 19:40:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161367AbeCATk3 (ORCPT ); Thu, 1 Mar 2018 14:40:29 -0500 Received: from esa4.hgst.iphmx.com ([216.71.154.42]:19114 "EHLO esa4.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161259AbeCATk1 (ORCPT ); Thu, 1 Mar 2018 14:40:27 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1519933228; x=1551469228; h=from:to:cc:subject:date:message-id:in-reply-to: references; bh=VDcO/0CPpt2w26xUKz1AEfZ55wI8YYQyAFjCcOI8koA=; b=NIshN4aVRJ3ZjWBp4UYxrLc73MeEXeTijZr1nWqKH4D+YuGX4+rRp9Rc nDsiBLqUx2DmwFu5J8K5ubRXX9dZTN0piW6lYDPpKJcRa3t+BKMQ+yxTO Bb0JCMPNNcUlgz2tAuxda5HkU9PjRzMTRfJ6CBttRyJ0hHfGrF4vzRWwr zup8JaS9IsdWcVoSaced3AoVck15oPEjQyQq3fA3g2Yflm0uMNcFvPqO3 59qYYVKEVRuaksnmfOVHrU16KR+M/q7+uErF58n96pq+svQYiHJ2yfa7j rH9+SXYczguH8QVJZbklRkkiamHtSy7Qn07MCEQMUJr+RCMnSN6bGMvVK A==; X-IronPort-AV: E=Sophos;i="5.47,409,1515427200"; d="scan'208";a="72555660" Received: from uls-op-cesaip02.wdc.com (HELO uls-op-cesaep02.wdc.com) ([199.255.45.15]) by ob1.hgst.iphmx.com with ESMTP; 02 Mar 2018 03:40:28 +0800 Received: from uls-op-cesaip01.wdc.com ([10.248.3.36]) by uls-op-cesaep02.wdc.com with ESMTP; 01 Mar 2018 11:33:50 -0800 Received: from washi.fujisawa.hgst.com ([10.149.53.254]) by uls-op-cesaip01.wdc.com with ESMTP; 01 Mar 2018 11:40:27 -0800 From: Damien Le Moal To: linux-scsi@vger.kernel.org, "Martin K . Petersen" , linux-ata@vger.kernel.org, Tejun Heo Cc: Bart Van Assche , Hannes Reinecke Subject: [PATCH 1/6] scsi: Introduce scsi_zbc_noretry_cmd() Date: Fri, 2 Mar 2018 04:40:19 +0900 Message-Id: <20180301194024.25532-2-damien.lemoal@wdc.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20180301194024.25532-1-damien.lemoal@wdc.com> References: <20180301194024.25532-1-damien.lemoal@wdc.com> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP For ZBC/ZAC devices, retrying a command with a condition known to lead to a failure is useless. One example is an unaligned write past the write pointer of a sequential zone. Retrying the same command will result in an error again. Currently, these iknown error condition cases are handled in sd_zbc.c using the sd_zbc_complete() function which is called from sd_done() when a command completes. However, these known error conditions are not handled in libata, nor is scsi_noretry_cmd() considering them. Fix this by introducing the function scsi_zbc_noretry_cmd() and use this function in scsi_noretry_cmd(). This allows simplifying sd_zbc_complete() which now only has to deal with report zones command reply. scsi_zbc_noretry_cmd() is also exported so that it can be used from libata. Signed-off-by: Damien Le Moal Reviewed-by: Hannes Reinecke --- drivers/scsi/scsi_error.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/scsi/sd.c | 2 +- drivers/scsi/sd.h | 8 +++--- drivers/scsi/sd_zbc.c | 47 ++++----------------------------- include/scsi/scsi_eh.h | 1 + 5 files changed, 77 insertions(+), 47 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index ca53a5f785ee..abb33d250176 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1671,6 +1671,66 @@ static void scsi_eh_offline_sdevs(struct list_head *work_q, return; } +/** + * scsi_zbc_noretry_cmd - Determine if ZBC device command can be retried + * @scmd: Failed cmd to check + * + * Test the error condition of a failed ZBC device command to determine cases + * that are known to be not worth retrying. + * If the specified command is not intended for a ZBC device, do nothing. + */ +bool scsi_zbc_noretry_cmd(struct scsi_cmnd *scmd) +{ + struct request *rq = scmd->request; + struct scsi_sense_hdr sshdr; + + /* + * The request queue zone model may not be set when this is called + * during device probe/revalidation. In that case, just fall back to + * default behavior and let the caller decide what to do with failures. + */ + if (!blk_queue_is_zoned(rq->q)) + return false; + + if (!scsi_command_normalize_sense(scmd, &sshdr)) + /* no valid sense data, don't know, so maybe retry */ + return false; + + if (sshdr.sense_key != ILLEGAL_REQUEST) + return false; + + switch (req_op(rq)) { + case REQ_OP_ZONE_RESET: + + if (sshdr.asc == 0x24) { + /* + * INVALID FIELD IN CDB error: reset of a conventional + * zone was attempted. Nothing to worry about, so be + * quiet about the error. + */ + if (!blk_rq_is_passthrough(rq)) + rq->rq_flags |= RQF_QUIET; + return true; + } + return false; + + case REQ_OP_WRITE: + case REQ_OP_WRITE_ZEROES: + case REQ_OP_WRITE_SAME: + + /* + * INVALID ADDRESS FOR WRITE error: It is unlikely that + * retrying write requests failed with any kind of + * alignement error will result in success. So don't. + */ + return sshdr.asc == 0x21; + + default: + return false; + } +} +EXPORT_SYMBOL_GPL(scsi_zbc_noretry_cmd); + /** * scsi_noretry_cmd - determine if command should be failed fast * @scmd: SCSI cmd to examine. @@ -1699,6 +1759,12 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd) return 0; check_type: + /* + * For ZBC, do not retry conditions that will only fail again. + */ + if (scmd->device->type == TYPE_ZBC && + scsi_zbc_noretry_cmd(scmd)) + return 1; /* * assume caller has checked sense and determined * the check condition was retryable. diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index bff21e636ddd..93c6baa7d677 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2041,7 +2041,7 @@ static int sd_done(struct scsi_cmnd *SCpnt) out: if (sd_is_zoned(sdkp)) - sd_zbc_complete(SCpnt, good_bytes, &sshdr); + sd_zbc_complete(SCpnt, good_bytes); SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt, "sd_done: completed %d of %d bytes\n", diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 0d663b5e45bb..b777ffecf386 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -284,8 +284,7 @@ extern void sd_zbc_remove(struct scsi_disk *sdkp); extern void sd_zbc_print_zones(struct scsi_disk *sdkp); extern int sd_zbc_setup_report_cmnd(struct scsi_cmnd *cmd); extern int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd); -extern void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes, - struct scsi_sense_hdr *sshdr); +extern void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes); #else /* CONFIG_BLK_DEV_ZONED */ @@ -310,8 +309,9 @@ static inline int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd) } static inline void sd_zbc_complete(struct scsi_cmnd *cmd, - unsigned int good_bytes, - struct scsi_sense_hdr *sshdr) {} + unsigned int good_bytes) +{ +} #endif /* CONFIG_BLK_DEV_ZONED */ diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c index 6c348a211ebb..14174f26af98 100644 --- a/drivers/scsi/sd_zbc.c +++ b/drivers/scsi/sd_zbc.c @@ -271,53 +271,16 @@ int sd_zbc_setup_reset_cmnd(struct scsi_cmnd *cmd) * sd_zbc_complete - ZBC command post processing. * @cmd: Completed command * @good_bytes: Command reply bytes - * @sshdr: command sense header * - * Called from sd_done(). Process report zones reply and handle reset zone - * and write commands errors. + * Called from sd_done(). Process report zones reply. */ -void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes, - struct scsi_sense_hdr *sshdr) +void sd_zbc_complete(struct scsi_cmnd *cmd, unsigned int good_bytes) { - int result = cmd->result; struct request *rq = cmd->request; - switch (req_op(rq)) { - case REQ_OP_ZONE_RESET: - - if (result && - sshdr->sense_key == ILLEGAL_REQUEST && - sshdr->asc == 0x24) - /* - * INVALID FIELD IN CDB error: reset of a conventional - * zone was attempted. Nothing to worry about, so be - * quiet about the error. - */ - rq->rq_flags |= RQF_QUIET; - break; - - case REQ_OP_WRITE: - case REQ_OP_WRITE_ZEROES: - case REQ_OP_WRITE_SAME: - - if (result && - sshdr->sense_key == ILLEGAL_REQUEST && - sshdr->asc == 0x21) - /* - * INVALID ADDRESS FOR WRITE error: It is unlikely that - * retrying write requests failed with any kind of - * alignement error will result in success. So don't. - */ - cmd->allowed = 0; - break; - - case REQ_OP_ZONE_REPORT: - - if (!result) - sd_zbc_report_zones_complete(cmd, good_bytes); - break; - - } + if (req_op(rq) == REQ_OP_ZONE_REPORT && + !cmd->result) + sd_zbc_report_zones_complete(cmd, good_bytes); } /** diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h index 2b7e227960e1..4c3626192255 100644 --- a/include/scsi/scsi_eh.h +++ b/include/scsi/scsi_eh.h @@ -18,6 +18,7 @@ extern int scsi_block_when_processing_errors(struct scsi_device *); extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd, struct scsi_sense_hdr *sshdr); extern int scsi_check_sense(struct scsi_cmnd *); +extern bool scsi_zbc_noretry_cmd(struct scsi_cmnd *); static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr) {