Skip to contentSkip to conversation

Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

PATCH In review Jeremy Linton 3 participants 3 messages Jul 12, 2007 SCSI

lore
Share & tools
lore

Jeremy Linton

author
#0 lore raw

PATCH 1/1 Incorrect SCSI transfer length computation from odd sized scsi_execute_async() transfers.

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.

This patch applies against scsi_lib.c in 2.6.22.

Signed-off-by Jeremy Linton <[email protected]>
drivers/scsi/scsi_lib.c 2007-07-11 18:43:36.000000000 -0500 renamed from drivers/scsi/scsi_lib.c.orig 2007-07-11 19:07:06.000000000 -0500 +1 -1
@@ -350,7 +350,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:
#1 lore raw
Quoted 18 lines · Jeremy Linton

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.

This patch applies against scsi_lib.c in 2.6.22.

Signed-off-by Jeremy Linton <[email protected]>

--- linux-2.6.22/drivers/scsi/scsi_lib.c.orig 2007-07-11
19:07:06.000000000 -0500
+++ linux-2.6.22/drivers/scsi/scsi_lib.c 2007-07-11
18:43:36.000000000 -0500

drivers/scsi/scsi_lib.c 2007-07-11 18:43:36.000000000 -0500 static int scsi_req_map_sg(struct reques · line 355

Quoted 7 lines
drivers/scsi/scsi_lib.c 2007-07-11 18:43:36.000000000 -0500 renamed from drivers/scsi/scsi_lib.c.orig 2007-07-11 19:07:06.000000000 -0500 +1 -1
@@ -350,7 +350,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;

I think you needed some other bits in there. See this patch http://marc.info/?l=linux-scsi&m=117392208211297&w=2

I tried just setting the bufflen first, and that still had problems. Could you try the patch here

http://marc.info/?l=linux-scsi&m=117392208211297&w=2

#2 lore raw
Quoted 3 lines · Mike Christie

I think you needed some other bits in there. See this patch I tried just setting the bufflen first, and that still had problems. Could you try the patch here

http://marc.info/?l=linux-scsi&m=117392208211297&w=2

I just read the thread.. I didn't see any strange retries with my test case. I will try duplicating the problem tomorrow. Then I will apply your patch and rerun my test.

I'm curious if this has been known since 2.6.19 why the patch hasn't propagated to the main kernel tree?