From patchwork Tue Mar 27 19:05:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10311507 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 CFD32600F6 for ; Tue, 27 Mar 2018 19:05:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C1BEA28928 for ; Tue, 27 Mar 2018 19:05:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B652629CDC; Tue, 27 Mar 2018 19:05:45 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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 2D2AB28928 for ; Tue, 27 Mar 2018 19:05:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751082AbeC0TFo (ORCPT ); Tue, 27 Mar 2018 15:05:44 -0400 Received: from smtp.infotech.no ([82.134.31.41]:44503 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750936AbeC0TFn (ORCPT ); Tue, 27 Mar 2018 15:05:43 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id B59352041CF; Tue, 27 Mar 2018 21:05:41 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Pzl5Y6peSaLR; Tue, 27 Mar 2018 21:05:41 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-184-164-15-239.dyn.295.ca [184.164.15.239]) by smtp.infotech.no (Postfix) with ESMTPA id 7B684204154; Tue, 27 Mar 2018 21:05:40 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [PATCH v3 3/7] scsi_io_completion_nz_result function added Date: Tue, 27 Mar 2018 15:05:30 -0400 Message-Id: <20180327190534.10449-4-dgilbert@interlog.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20180327190534.10449-1-dgilbert@interlog.com> References: <20180327190534.10449-1-dgilbert@interlog.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 Break out several intertwined paths when cmd->result is non zero and place them in the scsi_io_completion_nz_result helper function. The logic is not changed. Signed-off-by: Douglas Gilbert --- A reviewer requested the original helper function's two return values be reduced to one: the blk_stat variable. This required a hack to differentiate the default setting of blk_stat (BLK_STS_OK) from the case when the helper assigns BLK_STS_OK as the return value. The hack is to return the otherwise unused BLK_STS_NOTSUPP value as an indication that the helper didn't change anything. That hack was judged by another reviewer to be worse that the "two return values" ugliness it was trying to address. So back to the original "two return values" solution. Returning a structure containing result and blk_stat is another possibility but returning structures is frowned upon in some circles. drivers/scsi/scsi_lib.c | 126 ++++++++++++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 52 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 598836804745..3f65bb541ca3 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -751,6 +751,78 @@ static blk_status_t __scsi_error_from_host_byte(struct scsi_cmnd *cmd, } } +/* + * Helper for scsi_io_completion() when cmd->result is non-zero. Returns a + * new result that may suppress further error checking. Also modifies + * *blk_statp in some cases. + */ +static int scsi_io_completion_nz_result(struct scsi_cmnd *cmd, int result, + blk_status_t *blk_statp) +{ + bool sense_valid; + bool about_current; + struct request *req = cmd->request; + struct scsi_sense_hdr sshdr; + + sense_valid = scsi_command_normalize_sense(cmd, &sshdr); + about_current = sense_valid ? !scsi_sense_is_deferred(&sshdr) : true; + + if (blk_rq_is_passthrough(req)) { + if (sense_valid) { + /* + * SG_IO wants current and deferred errors + */ + scsi_req(req)->sense_len = + min(8 + cmd->sense_buffer[7], + SCSI_SENSE_BUFFERSIZE); + } + if (about_current) + *blk_statp = __scsi_error_from_host_byte(cmd, result); + } else if (blk_rq_bytes(req) == 0 && about_current) { + /* + * Flush commands do not transfers any data, and thus cannot use + * good_bytes != blk_rq_bytes(req) as the signal for an error. + * This sets *blk_statp explicitly for the problem case. + */ + *blk_statp = __scsi_error_from_host_byte(cmd, result); + } + /* + * Recovered errors need reporting, but they're always treated as + * success, so fiddle the result code here. For passthrough requests + * we already took a copy of the original into sreq->result which + * is what gets returned to the user + */ + if (sense_valid && (sshdr.sense_key == RECOVERED_ERROR)) { + bool do_print = true; + /* + * if ATA PASS-THROUGH INFORMATION AVAILABLE [0x0, 0x1d] + * skip print since caller wants ATA registers. Only occurs + * on SCSI ATA PASS_THROUGH commands when CK_COND=1 + */ + if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d)) + do_print = false; + else if (req->rq_flags & RQF_QUIET) + do_print = false; + if (do_print) + scsi_print_sense(cmd); + /* for passthrough, *blk_statp may be set, so clear */ + *blk_statp = BLK_STS_OK; + result = 0; + } + /* + * Another corner case: the SCSI status byte is non-zero but 'good'. + * Example: PRE-FETCH command returns SAM_STAT_CONDITION_MET when + * it is able to fit nominated LBs in its cache (and SAM_STAT_GOOD + * if it can't fit). Treat SAM_STAT_CONDITION_MET and the related + * intermediate statuses (both obsolete in SAM-4) as good. + */ + if (status_byte(result) && scsi_status_is_good(result)) { + *blk_statp = BLK_STS_OK; + result = 0; + } + return result; +} + /* * Function: scsi_io_completion() * @@ -794,26 +866,14 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) ACTION_DELAYED_RETRY} action; unsigned long wait_for = (cmd->allowed + 1) * req->timeout; - if (result) { + if (result) { /* does not necessarily mean there is an error */ sense_valid = scsi_command_normalize_sense(cmd, &sshdr); if (sense_valid) sense_current = !scsi_sense_is_deferred(&sshdr); + result = scsi_io_completion_nz_result(cmd, result, &blk_stat); } if (blk_rq_is_passthrough(req)) { - if (result) { - if (sense_valid) { - /* - * SG_IO wants current and deferred errors - */ - scsi_req(req)->sense_len = - min(8 + cmd->sense_buffer[7], - SCSI_SENSE_BUFFERSIZE); - } - if (sense_current) - blk_stat = __scsi_error_from_host_byte(cmd, - result); - } /* * __scsi_error_from_host_byte may have reset the host_byte */ @@ -831,13 +891,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) BUG(); return; } - } else if (blk_rq_bytes(req) == 0 && result && sense_current) { - /* - * Flush commands do not transfers any data, and thus cannot use - * good_bytes != blk_rq_bytes(req) as the signal for an error. - * This sets blk_stat explicitly for the problem case. - */ - blk_stat = __scsi_error_from_host_byte(cmd, result); } /* no bidi support for !blk_rq_is_passthrough yet */ @@ -851,37 +904,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) "%u sectors total, %d bytes done.\n", blk_rq_sectors(req), good_bytes)); - /* - * Recovered errors need reporting, but they're always treated as - * success, so fiddle the result code here. For passthrough requests - * we already took a copy of the original into sreq->result which - * is what gets returned to the user - */ - if (sense_valid && (sshdr.sense_key == RECOVERED_ERROR)) { - /* if ATA PASS-THROUGH INFORMATION AVAILABLE skip - * print since caller wants ATA registers. Only occurs on - * SCSI ATA PASS_THROUGH commands when CK_COND=1 - */ - if ((sshdr.asc == 0x0) && (sshdr.ascq == 0x1d)) - ; - else if (!(req->rq_flags & RQF_QUIET)) - scsi_print_sense(cmd); - result = 0; - /* for passthrough, blk_stat may be set */ - blk_stat = BLK_STS_OK; - } - /* - * Another corner case: the SCSI status byte is non-zero but 'good'. - * Example: PRE-FETCH command returns SAM_STAT_CONDITION_MET when - * it is able to fit nominated LBs in its cache (and SAM_STAT_GOOD - * if it can't fit). Treat SAM_STAT_CONDITION_MET and the related - * intermediate statuses (both obsolete in SAM-4) as good. - */ - if (status_byte(result) && scsi_status_is_good(result)) { - result = 0; - blk_stat = BLK_STS_OK; - } - /* * Next deal with any sectors which we were able to correctly * handle. Failed, zero length commands always need to drop down