[patch 07/30] Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

[patch 07/30] Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

[akpm]

From: Jeremy Linton <jli@greshamstorage.com>

Any function which use scsi_execute_async() and transfers "odd" sized data
that doesn't align correctly with the segment sizes may have its transfer
length padded out to the closest segment size.

For writes, this results in unnecessary data being transfered to the SCSI
target.  For reads, it affects the residual data length being returned to
the application since the residual length will be based on the padded
transfer size rather than the actual request size.

The easiest way to see this is by trying to read using the SG_IO ioctl a
large (>32k) buffer size from a tape device that only has a few bytes of
data stored for the current block.  The resulting resid will generally be
incorrect.

I've fixed this simply by changing scsi_req_map_sg() so that it places the
requested transfer length in rq->data_len rather than the sum of all the sg
segments.

Signed-off-by: Jeremy Linton <jli@greshamstorage.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/scsi/scsi_lib.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/scsi/scsi_lib.c~incorrect-scsi-transfer-length-computation-from-odd drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c~incorrect-scsi-transfer-length-computation-from-odd
+++ a/drivers/scsi/scsi_lib.c
@@ -355,7 +355,7 @@ static int scsi_req_map_sg(struct reques
 	}
 
 	rq->buffer = rq->data = NULL;
-	rq->data_len = data_len;
+	rq->data_len = bufflen;
 	return 0;
 
 free_bios:
_

Re: [patch 07/30] Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

[Jeremy Linton]

akpm@linux-foundation.org wrote:
> From: Jeremy Linton <jli@greshamstorage.com>
> 
> Any function which use scsi_execute_async() and transfers "odd" sized data
> that doesn't align correctly with the segment sizes may have its transfer
> length padded out to the closest segment size.
	I would like to strongly suggest that Mike Christie's patch be used instead.
http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg06032.html


I finally hit the case he was talking about (the block layer retries 0 length commands caused by a size mismatch) and 
its ugly. I'm not really sure why my initial tests weren't hitting that case, I was trying to understand why some blocks 
were getting an extra command generated while others weren't.  Sufficient to say, his patch fixes both problems, the 
incorrect transfer lengths and the extra 0 length transfer being generated.

Re: [patch 07/30] Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

[Mike Christie]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

Jeremy Linton wrote:
> akpm@linux-foundation.org wrote:
>> From: Jeremy Linton <jli@greshamstorage.com>
>>
>> Any function which use scsi_execute_async() and transfers "odd" sized 
>> data
>> that doesn't align correctly with the segment sizes may have its transfer
>> length padded out to the closest segment size.
>     I would like to strongly suggest that Mike Christie's patch be used 
> instead.
> http://www.mail-archive.com/linux-scsi@vger.kernel.org/msg06032.html
> 
> 
> I finally hit the case he was talking about (the block layer retries 0 
> length commands caused by a size mismatch) and its ugly. I'm not really 
> sure why my initial tests weren't hitting that case, I was trying to 
> understand why some blocks were getting an extra command generated while 
> others weren't.  Sufficient to say, his patch fixes both problems, the 
> incorrect transfer lengths and the extra 0 length transfer being generated.
> 

I am having trouble inlining the patch properly with my mailer. Here is 
the patch from that thread attached if James or Andrew needs it.

[-- Attachment #2: usebufflen.patch --]
[-- Type: text/x-patch, Size: 1465 bytes --]

sg's may have setup a the buffer with a different length than
the transfer length so we should be using the bufflen passed
in as the request's data len.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>


diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5f95570..30ae831 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -301,7 +301,7 @@ static int scsi_req_map_sg(struct reques
 {
 	struct request_queue *q = rq->q;
 	int nr_pages = (bufflen + sgl[0].offset + PAGE_SIZE - 1) >> PAGE_SHIFT;
-	unsigned int data_len = 0, len, bytes, off;
+	unsigned int data_len = bufflen, len, bytes, off;
 	struct page *page;
 	struct bio *bio = NULL;
 	int i, err, nr_vecs = 0;
@@ -310,10 +310,15 @@ static int scsi_req_map_sg(struct reques
 		page = sgl[i].page;
 		off = sgl[i].offset;
 		len = sgl[i].length;
-		data_len += len;
 
-		while (len > 0) {
+		while (len > 0 && data_len > 0) {
+			/*
+			 * sg sends a scatterlist that is larger than
+			 * the data_len it wants transferred for certain
+			 * IO sizes
+			 */
 			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
+			bytes = min(bytes, data_len);
 
 			if (!bio) {
 				nr_vecs = min_t(int, BIO_MAX_PAGES, nr_pages);
@@ -345,12 +350,13 @@ static int scsi_req_map_sg(struct reques
 
 			page++;
 			len -= bytes;
+			data_len -=bytes;
 			off = 0;
 		}
 	}
 
 	rq->buffer = rq->data = NULL;
-	rq->data_len = data_len;
+	rq->data_len = bufflen;
 	return 0;
 
 free_bios: