diff mbox

[v2,2/7] sd: Improve sd_completed_bytes

Message ID 20170424075115.30337-3-damien.lemoal@wdc.com (mailing list archive)
State Accepted
Headers show

Commit Message

Damien Le Moal April 24, 2017, 7:51 a.m. UTC 
From: Damien Le Moal <damien.lemoal@wdc.com>

Re-shuffle the code to be more efficient by not initializing
variables upfront (i.e. do it only when necessary).
Also replace the do_div calls with calls to sectors_to_logical().

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/sd.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

Comments

Christoph Hellwig April 24, 2017, 3:31 p.m. UTC | #1 
On Mon, Apr 24, 2017 at 04:51:10PM +0900, damien.lemoal@wdc.com wrote:
> From: Damien Le Moal <damien.lemoal@wdc.com>
> 
> Re-shuffle the code to be more efficient by not initializing
> variables upfront (i.e. do it only when necessary).
> Also replace the do_div calls with calls to sectors_to_logical().
> 
> No functional change is introduced by this patch.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox

Patch

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ce62d2c..10c7657 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1758,41 +1758,45 @@  static int sd_eh_action(struct scsi_cmnd *scmd, int eh_disp)
 
 static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd)
 {
-	u64 start_lba = blk_rq_pos(scmd->request);
-	u64 end_lba = blk_rq_pos(scmd->request) + (scsi_bufflen(scmd) / 512);
-	u64 factor = scmd->device->sector_size / 512;
-	u64 bad_lba;
-	int info_valid;
+	struct request *req = scmd->request;
+	struct scsi_device *sdev = scmd->device;
+	unsigned int transferred, good_bytes;
+	u64 start_lba, end_lba, bad_lba;
+
 	/*
-	 * resid is optional but mostly filled in.  When it's unused,
-	 * its value is zero, so we assume the whole buffer transferred
+	 * Some commands have a payload smaller than the device logical
+	 * block size (e.g. INQUIRY on a 4K disk).
 	 */
-	unsigned int transferred = scsi_bufflen(scmd) - scsi_get_resid(scmd);
-	unsigned int good_bytes;
-
-	info_valid = scsi_get_sense_info_fld(scmd->sense_buffer,
-					     SCSI_SENSE_BUFFERSIZE,
-					     &bad_lba);
-	if (!info_valid)
+	if (scsi_bufflen(scmd) <= sdev->sector_size)
 		return 0;
 
-	if (scsi_bufflen(scmd) <= scmd->device->sector_size)
+	/* Check if we have a 'bad_lba' information */
+	if (!scsi_get_sense_info_fld(scmd->sense_buffer,
+				     SCSI_SENSE_BUFFERSIZE,
+				     &bad_lba))
 		return 0;
 
-	/* be careful ... don't want any overflows */
-	do_div(start_lba, factor);
-	do_div(end_lba, factor);
-
-	/* The bad lba was reported incorrectly, we have no idea where
+	/*
+	 * If the bad lba was reported incorrectly, we have no idea where
 	 * the error is.
 	 */
-	if (bad_lba < start_lba  || bad_lba >= end_lba)
+	start_lba = sectors_to_logical(sdev, blk_rq_pos(req));
+	end_lba = sectors_to_logical(sdev,
+				blk_rq_pos(req) + (scsi_bufflen(scmd) >> 9));
+	if (bad_lba < start_lba || bad_lba >= end_lba)
 		return 0;
 
+	/*
+	 * resid is optional but mostly filled in.  When it's unused,
+	 * its value is zero, so we assume the whole buffer transferred
+	 */
+	transferred = scsi_bufflen(scmd) - scsi_get_resid(scmd);
+
 	/* This computation should always be done in terms of
 	 * the resolution of the device's medium.
 	 */
-	good_bytes = (bad_lba - start_lba) * scmd->device->sector_size;
+	good_bytes = logical_to_bytes(sdev, bad_lba - start_lba);
+
 	return min(good_bytes, transferred);
 }