Hello, Jens, Boaz. Jens Axboe wrote: >>>> From: Boaz Harrosh <bharrosh@xxxxxxxxxxx> >>>> Date: Wed, 5 Mar 2008 12:07:12 +0200 >>>> Subject: [PATCH] blk: missing add of padded bytes to io completion byte count >>>> >>>> the commit e97a294ef6938512b655b1abf17656cf2b26f709 was very wrong. This is >>>> because scsi-ml supports the ability to split a request into smaller chunks, >>>> in which case scsi_bufflen() is smaller then request length. Then at completion >>>> time the remainder can be issued as a new scsi command. In that case the above >>>> commit is a data corruption. Thanks for catching the stupidity. Did it actually happen? PC commands are not completed in pieces and padding / draining should only happen for those. qc->extra_len should be zero where commands can be splitted for all current cases. >>> We needed something for -rc4, so it had to be rushed a bit... >>> >>>> Also in this fix all users of block layer are taken care of, and not only >>>> scsi devices. >>>> >>>> Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx> >>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx> >>>> --- >>>> block/blk-core.c | 4 ++++ >>>> drivers/scsi/scsi.c | 2 +- >>>> 2 files changed, 5 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/block/blk-core.c b/block/blk-core.c >>>> index 2a438a9..37fcccc 100644 >>>> --- a/block/blk-core.c >>>> +++ b/block/blk-core.c >>>> @@ -1549,6 +1549,9 @@ static int __end_that_request_first(struct request *req, int error, >>>> nr_bytes >> 9, req->sector); >>>> } >>>> >>>> + if (nr_bytes >= blk_rq_bytes(req)) >>>> + nr_bytes += req->extra_len; >>>> + This is getting insanely subtle. Let's say there's PIO driver which transfer certain sized chunks at a time and completes request partially after completing each chunk and the driver uses draining to eat up whatever excess data, which seems like a legit use case to me. But it won't work because __end_that_request_first() will terminate when it reaches reaches the 'true' transfer size. That's just broken API. FWIW, Nacked-by: Tejun Heo <htejun@xxxxxxxxx> -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
