On 2018/08/06 13:51, Douglas Gilbert wrote: > 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 <dgilbert@xxxxxxxxxxxx> > --- > 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); So here, without use_16_for_rw being forced on (which is the case for most disks I think, except ZBC disks which mandate it) or most disks, all read/write to low LBAs will have to go through a longer chain of if/else if... Is this change really such a gain in average ? It looks like this will be a loss for the first small partition at the beginning of the disk. > - } 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); > -- Damien Le Moal Western Digital Research