From patchwork Mon Aug 6 04:51:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10556473 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 40F8714E2 for ; Mon, 6 Aug 2018 04:51:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 21F8F28FFB for ; Mon, 6 Aug 2018 04:51:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 139E529006; Mon, 6 Aug 2018 04:51:27 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 6588A28FFB for ; Mon, 6 Aug 2018 04:51:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726672AbeHFG6l (ORCPT ); Mon, 6 Aug 2018 02:58:41 -0400 Received: from smtp.infotech.no ([82.134.31.41]:35000 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726276AbeHFG6l (ORCPT ); Mon, 6 Aug 2018 02:58:41 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 7724220424C; Mon, 6 Aug 2018 06:51:22 +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 36wg8dBG1tzM; Mon, 6 Aug 2018 06:51:20 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-45-58-245-67.dyn.295.ca [45.58.245.67]) by smtp.infotech.no (Postfix) with ESMTPA id B9E3320423B; Mon, 6 Aug 2018 06:51:19 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [RFC PATCH 1/5] add tweakable bounds_check flag, now off by default Date: Mon, 6 Aug 2018 00:51:11 -0400 Message-Id: <20180806045115.7725-2-dgilbert@interlog.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180806045115.7725-1-dgilbert@interlog.com> References: <20180806045115.7725-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 Add bounds_check "rw" attribute to the sd driver. It controls whether each read/write operation submission does an "out of range" bounds check and a LBA/number_of_blocks alignment bounds check. The mainline kernel currently does both these checks. This patch changes that default to bounds_check=false, that is: the two checks are not done. SBC, SBC-2, SBC-3 and draft SBC-4 are require a device server (i.e. a SCSI disk) to fail any media command in which the LBA+number_of_blocks exceeds the capacity of the disk. So why should the sd driver also check it, given the block layer generated the request and knows the disk capacity and the disk itself also checks it? The block layer does almost all its block handling in units of 512 byte blocks whereas SCSI disks may have other logical block sizes (e.g. 4096 byte logical blocks). So when the block sizes are different it is possible that there is an alignment issue. But if that occurs, it would be a logic issue in the block layer. Note that the current mainline code does this check even when it is not needed (e.g. when the logical block size of the disk is 512 bytes). Signed-off-by: Douglas Gilbert --- Should a mechanism be added so this could set/cleared by: - a LLD for all disks it controls - kernel boot time parameter? drivers/scsi/sd.c | 55 +++++++++++++++++++++++++++++++++++++---------- drivers/scsi/sd.h | 1 + 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index d1d08f039bdd..b17b8c66881d 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -522,6 +522,32 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RW(max_write_same_blocks); +static ssize_t +bounds_check_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct scsi_disk *sdkp = to_scsi_disk(dev); + + return sprintf(buf, "%u\n", sdkp->bounds_check); +} + +static ssize_t +bounds_check_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct scsi_disk *sdkp = to_scsi_disk(dev); + int err, n; + + if (!capable(CAP_SYS_ADMIN)) + return -EACCES; + + err = kstrtoint(buf, 10, &n); + if (!err) + sdkp->bounds_check = !!n; + + return err ? err : count; +} +static DEVICE_ATTR_RW(bounds_check); + static struct attribute *sd_disk_attrs[] = { &dev_attr_cache_type.attr, &dev_attr_FUA.attr, @@ -535,6 +561,7 @@ static struct attribute *sd_disk_attrs[] = { &dev_attr_zeroing_mode.attr, &dev_attr_max_write_same_blocks.attr, &dev_attr_max_medium_access_timeouts.attr, + &dev_attr_bounds_check.attr, NULL, }; ATTRIBUTE_GROUPS(sd_disk); @@ -1088,7 +1115,6 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) struct scsi_disk *sdkp = scsi_disk(rq->rq_disk); sector_t lba = sectors_to_logical(sdp, blk_rq_pos(rq)); unsigned int nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq)); - unsigned int mask = logical_to_sectors(sdp, 1) - 1; unsigned char protect, fua; bool write = rq_data_dir(rq) == WRITE; bool dif, dix; @@ -1106,17 +1132,24 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) goto out; } - if (unlikely(blk_rq_pos(rq) + blk_rq_sectors(rq) - > logical_to_sectors(sdp, sdkp->capacity))) { - scmd_printk(KERN_ERR, cmd, "access beyond end of device\n"); - ret = BLKPREP_KILL; - goto out; - } + if (sdkp->bounds_check) { + unsigned int mask = logical_to_sectors(sdp, 1) - 1; - if (unlikely((blk_rq_pos(rq) & mask) || (blk_rq_sectors(rq) & mask))) { - scmd_printk(KERN_ERR, cmd, "request not aligned to the logical block size\n"); - ret = BLKPREP_KILL; - goto out; + if (unlikely(blk_rq_pos(rq) + blk_rq_sectors(rq) + > logical_to_sectors(sdp, sdkp->capacity))) { + scmd_printk(KERN_ERR, cmd, + "access beyond end of device\n"); + ret = BLKPREP_KILL; + goto out; + } + + if (unlikely((blk_rq_pos(rq) & mask) || + (blk_rq_sectors(rq) & mask))) { + scmd_printk(KERN_ERR, cmd, + "request not aligned to logical block size\n"); + ret = BLKPREP_KILL; + goto out; + } } /* diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 392c7d078ae3..6f58d130fb75 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -117,6 +117,7 @@ struct scsi_disk { unsigned urswrz : 1; unsigned security : 1; unsigned ignore_medium_access_errors : 1; + unsigned bounds_check : 1; }; #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,dev) From patchwork Mon Aug 6 04:51:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10556475 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AB2C4174A for ; Mon, 6 Aug 2018 04:51:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 99AB328FFE for ; Mon, 6 Aug 2018 04:51:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8DD7629006; Mon, 6 Aug 2018 04:51:27 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 237722902D for ; Mon, 6 Aug 2018 04:51:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726823AbeHFG6m (ORCPT ); Mon, 6 Aug 2018 02:58:42 -0400 Received: from smtp.infotech.no ([82.134.31.41]:35010 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726449AbeHFG6m (ORCPT ); Mon, 6 Aug 2018 02:58:42 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 53A49204255; Mon, 6 Aug 2018 06:51:24 +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 lqRsb4huARiP; Mon, 6 Aug 2018 06:51:22 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-45-58-245-67.dyn.295.ca [45.58.245.67]) by smtp.infotech.no (Postfix) with ESMTPA id 02AC220423F; Mon, 6 Aug 2018 06:51:20 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [RFC PATCH 2/5] break sd_done sense processing out to own function Date: Mon, 6 Aug 2018 00:51:12 -0400 Message-Id: <20180806045115.7725-3-dgilbert@interlog.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180806045115.7725-1-dgilbert@interlog.com> References: <20180806045115.7725-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 the sense key handling in the sd_done() (response) path into its own function. Note that the sense key only needs to be inspected when a SCSI status of Check Condition is returned. Signed-off-by: Douglas Gilbert --- No speed up here, just a clean up. There could possibly be a speed improvement (not observed in tests) from lessening the cache footprint of the sd_done() function which is on the fast path. drivers/scsi/sd.c | 112 ++++++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 53 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index b17b8c66881d..4b1402633c36 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1898,6 +1898,62 @@ static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) return min(good_bytes, transferred); } +/* Helper for scsi_done() when there is a sense buffer */ +static int sd_done_sense(struct scsi_cmnd *SCpnt, unsigned int good_bytes, + struct scsi_sense_hdr *sshdrp) +{ + struct request *req = SCpnt->request; + struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk); + + switch (sshdrp->sense_key) { + case HARDWARE_ERROR: + case MEDIUM_ERROR: + return sd_completed_bytes(SCpnt); + case RECOVERED_ERROR: + return scsi_bufflen(SCpnt); + case NO_SENSE: + /* This indicates a false check condition, so ignore it. An + * unknown amount of data was transferred so treat it as an + * error. + */ + SCpnt->result = 0; + memset(SCpnt->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); + break; + case ABORTED_COMMAND: + if (sshdrp->asc == 0x10) /* DIF: Target detected corruption */ + good_bytes = sd_completed_bytes(SCpnt); + break; + case ILLEGAL_REQUEST: + switch (sshdrp->asc) { + case 0x10: /* DIX: Host detected corruption */ + good_bytes = sd_completed_bytes(SCpnt); + break; + case 0x20: /* INVALID COMMAND OPCODE */ + case 0x24: /* INVALID FIELD IN CDB */ + switch (SCpnt->cmnd[0]) { + case UNMAP: + sd_config_discard(sdkp, SD_LBP_DISABLE); + break; + case WRITE_SAME_16: + case WRITE_SAME: + if (SCpnt->cmnd[1] & 8) { /* UNMAP */ + sd_config_discard(sdkp, SD_LBP_DISABLE); + } else { + sdkp->device->no_write_same = 1; + sd_config_write_same(sdkp); + req->__data_len = blk_rq_bytes(req); + req->rq_flags |= RQF_QUIET; + } + break; + } + } + break; + default: + break; + } + return good_bytes; +} + /** * sd_done - bottom half handler: called when the lower level * driver has completed (successfully or otherwise) a scsi command. @@ -1964,60 +2020,10 @@ static int sd_done(struct scsi_cmnd *SCpnt) } sdkp->medium_access_timed_out = 0; - if (driver_byte(result) != DRIVER_SENSE && - (!sense_valid || sense_deferred)) - goto out; + if (unlikely(driver_byte(result) == DRIVER_SENSE || + (sense_valid && !sense_deferred))) + good_bytes = sd_done_sense(SCpnt, good_bytes, &sshdr); - switch (sshdr.sense_key) { - case HARDWARE_ERROR: - case MEDIUM_ERROR: - good_bytes = sd_completed_bytes(SCpnt); - break; - case RECOVERED_ERROR: - good_bytes = scsi_bufflen(SCpnt); - break; - case NO_SENSE: - /* This indicates a false check condition, so ignore it. An - * unknown amount of data was transferred so treat it as an - * error. - */ - SCpnt->result = 0; - memset(SCpnt->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); - break; - case ABORTED_COMMAND: - if (sshdr.asc == 0x10) /* DIF: Target detected corruption */ - good_bytes = sd_completed_bytes(SCpnt); - break; - case ILLEGAL_REQUEST: - switch (sshdr.asc) { - case 0x10: /* DIX: Host detected corruption */ - good_bytes = sd_completed_bytes(SCpnt); - break; - case 0x20: /* INVALID COMMAND OPCODE */ - case 0x24: /* INVALID FIELD IN CDB */ - switch (SCpnt->cmnd[0]) { - case UNMAP: - sd_config_discard(sdkp, SD_LBP_DISABLE); - break; - case WRITE_SAME_16: - case WRITE_SAME: - if (SCpnt->cmnd[1] & 8) { /* UNMAP */ - sd_config_discard(sdkp, SD_LBP_DISABLE); - } else { - sdkp->device->no_write_same = 1; - sd_config_write_same(sdkp); - req->__data_len = blk_rq_bytes(req); - req->rq_flags |= RQF_QUIET; - } - break; - } - } - break; - default: - break; - } - - out: if (sd_is_zoned(sdkp)) sd_zbc_complete(SCpnt, good_bytes, &sshdr); From patchwork Mon Aug 6 04:51:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10556479 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1F83B13BB for ; Mon, 6 Aug 2018 04:51:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0D67828FFB for ; Mon, 6 Aug 2018 04:51:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 01ACC29006; Mon, 6 Aug 2018 04:51:28 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 90A8428FFB for ; Mon, 6 Aug 2018 04:51:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726921AbeHFG6n (ORCPT ); Mon, 6 Aug 2018 02:58:43 -0400 Received: from smtp.infotech.no ([82.134.31.41]:35024 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726449AbeHFG6n (ORCPT ); Mon, 6 Aug 2018 02:58:43 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id F3E2D204177; Mon, 6 Aug 2018 06:51:25 +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 RTBobEhsuwZR; Mon, 6 Aug 2018 06:51:24 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-45-58-245-67.dyn.295.ca [45.58.245.67]) by smtp.infotech.no (Postfix) with ESMTPA id 3EB06204248; Mon, 6 Aug 2018 06:51:22 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [RFC PATCH 3/5] streamline REQ_OP_READ-WRITE access Date: Mon, 6 Aug 2018 00:51:13 -0400 Message-Id: <20180806045115.7725-4-dgilbert@interlog.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180806045115.7725-1-dgilbert@interlog.com> References: <20180806045115.7725-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 Make the two block layer operations most frequently used (REQ_OP_READ and REQ_OP_WRITE) bypass the switch statements in the submission and response paths. Assume these two enums are 0 and 1 which allows a single comparison to select both of them. Check that assumption at driver start-up. Signed-off-by: Douglas Gilbert --- drivers/scsi/sd.c | 82 +++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 4b1402633c36..9f047fd3c92d 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1227,6 +1227,14 @@ static int sd_init_command(struct scsi_cmnd *cmd) { struct request *rq = cmd->request; + /* + * Assumption that REQ_OP_READ and REQ_OP_WRITE are 0 and 1 is + * checked in init_sd(). The following "if" is done to dispatch + * REQ_OP_READ and REQ_OP_WRITE quickly. + */ + if (likely(req_op(rq) < 2)) + return sd_setup_read_write_cmnd(cmd); + switch (req_op(rq)) { case REQ_OP_DISCARD: switch (scsi_disk(rq->rq_disk)->provisioning_mode) { @@ -1247,9 +1255,6 @@ static int sd_init_command(struct scsi_cmnd *cmd) return sd_setup_write_same_cmnd(cmd); case REQ_OP_FLUSH: return sd_setup_flush_cmnd(cmd); - case REQ_OP_READ: - case REQ_OP_WRITE: - return sd_setup_read_write_cmnd(cmd); case REQ_OP_ZONE_REPORT: return sd_zbc_setup_report_cmnd(cmd); case REQ_OP_ZONE_RESET: @@ -1973,30 +1978,12 @@ static int sd_done(struct scsi_cmnd *SCpnt) int sense_valid = 0; int sense_deferred = 0; - switch (req_op(req)) { - case REQ_OP_DISCARD: - case REQ_OP_WRITE_ZEROES: - case REQ_OP_WRITE_SAME: - case REQ_OP_ZONE_RESET: - if (!result) { - good_bytes = blk_rq_bytes(req); - scsi_set_resid(SCpnt, 0); - } else { - good_bytes = 0; - scsi_set_resid(SCpnt, blk_rq_bytes(req)); - } - break; - case REQ_OP_ZONE_REPORT: - if (!result) { - good_bytes = scsi_bufflen(SCpnt) - - scsi_get_resid(SCpnt); - scsi_set_resid(SCpnt, 0); - } else { - good_bytes = 0; - scsi_set_resid(SCpnt, blk_rq_bytes(req)); - } - break; - default: + /* + * Assumption that REQ_OP_READ and REQ_OP_WRITE are 0 and 1 is + * checked in init_sd(). The following "if" is done to expedite + * REQ_OP_READ and REQ_OP_WRITE. + */ + if (likely(req_op(req) < 2)) { /* * In case of bogus fw or device, we could end up having * an unaligned partial completion. Check this here and force @@ -2011,6 +1998,36 @@ static int sd_done(struct scsi_cmnd *SCpnt) round_up(resid, sector_size)); scsi_set_resid(SCpnt, resid); } + } else { + switch (req_op(req)) { + case REQ_OP_DISCARD: + case REQ_OP_WRITE_ZEROES: + case REQ_OP_WRITE_SAME: + case REQ_OP_ZONE_RESET: + if (!result) { + good_bytes = blk_rq_bytes(req); + scsi_set_resid(SCpnt, 0); + } else { + good_bytes = 0; + scsi_set_resid(SCpnt, blk_rq_bytes(req)); + } + break; + case REQ_OP_ZONE_REPORT: + if (!result) { + good_bytes = scsi_bufflen(SCpnt) + - scsi_get_resid(SCpnt); + scsi_set_resid(SCpnt, 0); + } else { + good_bytes = 0; + scsi_set_resid(SCpnt, blk_rq_bytes(req)); + } + break; + default: + sd_printk(KERN_NOTICE, sdkp, "unexpected REQ_OP=%u\n", + (unsigned int)req_op(req)); + WARN_ON_ONCE(true); + break; + } } if (result) { @@ -3597,6 +3614,17 @@ static int __init init_sd(void) int majors = 0, i, err; SCSI_LOG_HLQUEUE(3, printk("init_sd: sd driver entry point\n")); + /* + * The sd_init_command() and sd_done() assume REQ_OP_READ and + * REQ_OP_WRITE are 0 and 1 and will fail if they are not. If they + * are not, would prefer a compile failure but the preprocessor can't + * use enum constants. Place check here because only need to check + * early and once. + */ + if (REQ_OP_READ + REQ_OP_WRITE > 1) + pr_err("%s: REQ_OP_READ=%d REQ_OP_WRITE=%d %s.\n", __func__, + REQ_OP_READ, REQ_OP_WRITE, + "expected 0 and 1. Logic ERROR"); for (i = 0; i < SD_MAJORS; i++) { if (register_blkdev(sd_major(i), "sd") != 0) From patchwork Mon Aug 6 04:51:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10556483 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5EB99174A for ; Mon, 6 Aug 2018 04:51:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4A47328FFB for ; Mon, 6 Aug 2018 04:51:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3E5EA29006; Mon, 6 Aug 2018 04:51: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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 D3F3528FFE for ; Mon, 6 Aug 2018 04:51:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726942AbeHFG6q (ORCPT ); Mon, 6 Aug 2018 02:58:46 -0400 Received: from smtp.infotech.no ([82.134.31.41]:35032 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726276AbeHFG6o (ORCPT ); Mon, 6 Aug 2018 02:58:44 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 07817204247; Mon, 6 Aug 2018 06:51:27 +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 8vytXK1Q9OGm; Mon, 6 Aug 2018 06:51:25 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-45-58-245-67.dyn.295.ca [45.58.245.67]) by smtp.infotech.no (Postfix) with ESMTPA id 7B16E20423B; Mon, 6 Aug 2018 06:51:23 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [RFC PATCH 4/5] streamline some logical operations Date: Mon, 6 Aug 2018 00:51:14 -0400 Message-Id: <20180806045115.7725-5-dgilbert@interlog.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180806045115.7725-1-dgilbert@interlog.com> References: <20180806045115.7725-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 Re-arrange some logic to lessen the number of checks. With logical ANDs put the least likely first, with logical ORs put the most likely first. Also add conditional hints on the assumed fastpath. Signed-off-by: Douglas Gilbert --- drivers/scsi/sd.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 9f047fd3c92d..05014054e357 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1171,9 +1171,9 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) fua = (rq->cmd_flags & REQ_FUA) ? 0x8 : 0; dix = scsi_prot_sg_count(cmd); - dif = scsi_host_dif_capable(cmd->device->host, sdkp->protection_type); + dif = scsi_host_dif_capable(sdp->host, sdkp->protection_type); - if (write && dix) + if (dix && write) sd_dif_prepare(cmd); if (dif || dix) @@ -1181,19 +1181,27 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *cmd) else protect = 0; - if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) { + if (unlikely(protect && + sdkp->protection_type == T10_PI_TYPE2_PROTECTION)) ret = sd_setup_read_write_32_cmnd(cmd, write, lba, nr_blocks, protect | fua); - } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) { + else if (sdp->use_16_for_rw) ret = sd_setup_read_write_16_cmnd(cmd, write, lba, nr_blocks, protect | fua); - } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) || sdp->use_10_for_rw - || protect) { - ret = sd_setup_read_write_10_cmnd(cmd, write, lba, nr_blocks, - protect | fua); - } else { - ret = sd_setup_read_write_6_cmnd(cmd, write, lba, nr_blocks, - protect | fua); + else if (likely(nr_blocks < 0x100)) { + if (sdp->use_10_for_rw || (lba > 0x1fffff) || protect) + ret = sd_setup_read_write_10_cmnd(cmd, write, lba, + nr_blocks, protect | fua); + else + ret = sd_setup_read_write_6_cmnd(cmd, write, lba, + nr_blocks, protect | fua); + } else { /* not already done and nr_blocks > 0xff */ + if (unlikely(nr_blocks > 0xffff)) + ret = sd_setup_read_write_16_cmnd(cmd, write, lba, + nr_blocks, protect | fua); + else + ret = sd_setup_read_write_10_cmnd(cmd, write, lba, + nr_blocks, protect | fua); } if (ret != BLKPREP_OK) @@ -1976,7 +1984,6 @@ static int sd_done(struct scsi_cmnd *SCpnt) struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk); struct request *req = SCpnt->request; int sense_valid = 0; - int sense_deferred = 0; /* * Assumption that REQ_OP_READ and REQ_OP_WRITE are 0 and 1 is @@ -2030,16 +2037,14 @@ static int sd_done(struct scsi_cmnd *SCpnt) } } - if (result) { + if (unlikely(result)) { sense_valid = scsi_command_normalize_sense(SCpnt, &sshdr); - if (sense_valid) - sense_deferred = scsi_sense_is_deferred(&sshdr); + if (driver_byte(result) == DRIVER_SENSE || + (sense_valid && (!scsi_sense_is_deferred(&sshdr)))) + good_bytes = sd_done_sense(SCpnt, good_bytes, &sshdr); } - sdkp->medium_access_timed_out = 0; - if (unlikely(driver_byte(result) == DRIVER_SENSE || - (sense_valid && !sense_deferred))) - good_bytes = sd_done_sense(SCpnt, good_bytes, &sshdr); + sdkp->medium_access_timed_out = 0; if (sd_is_zoned(sdkp)) sd_zbc_complete(SCpnt, good_bytes, &sshdr); From patchwork Mon Aug 6 04:51:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 10556481 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1B56314E2 for ; Mon, 6 Aug 2018 04:51:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0B4AB28FFB for ; Mon, 6 Aug 2018 04:51:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F380729007; Mon, 6 Aug 2018 04:51:30 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, 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 A2E9828FFB for ; Mon, 6 Aug 2018 04:51:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726931AbeHFG6q (ORCPT ); Mon, 6 Aug 2018 02:58:46 -0400 Received: from smtp.infotech.no ([82.134.31.41]:35036 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726894AbeHFG6o (ORCPT ); Mon, 6 Aug 2018 02:58:44 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 54CB220423B; Mon, 6 Aug 2018 06:51:27 +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 AMPzTpQjtWMp; Mon, 6 Aug 2018 06:51:26 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-45-58-245-67.dyn.295.ca [45.58.245.67]) by smtp.infotech.no (Postfix) with ESMTPA id BA07F20423F; Mon, 6 Aug 2018 06:51:24 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, hare@suse.de, bart.vanassche@wdc.com, jthumshirn@suse.de Subject: [RFC PATCH 5/5] make sd_done() REQ_OP_FLUSH handling explicit Date: Mon, 6 Aug 2018 00:51:15 -0400 Message-Id: <20180806045115.7725-6-dgilbert@interlog.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180806045115.7725-1-dgilbert@interlog.com> References: <20180806045115.7725-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 REQ_OP_FLUSH appears in the submission path processing but not in the response path processing. Hence it takes the "default" case which seems to be designed for REQ_OP_READ and REQ_OP_WRITE. So add a REQ_OP_FLUSH case to the response path. Signed-off-by: Douglas Gilbert --- Is it correct to treat REQ_OP_FLUSH like the other OPs that may send but never return data (as in a READ's data-in)? Should this be fixed separately in the mainline? drivers/scsi/sd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 05014054e357..9cf88392e994 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2007,6 +2007,7 @@ static int sd_done(struct scsi_cmnd *SCpnt) } } else { switch (req_op(req)) { + case REQ_OP_FLUSH: case REQ_OP_DISCARD: case REQ_OP_WRITE_ZEROES: case REQ_OP_WRITE_SAME: