From: Yu Kuai yukuai3@huawei.com
This reverts commit 84f7a9de0602704bbec774a6c7f7c8c4994bee9c.
Because it introduces a problem that rq->__data_len is set to the wrong value.
before this patch: 1) nr_bytes = rq->__data_len 2) rq->__data_len = sdp->sector_size 3) scsi_init_io() 4) rq->__data_len = nr_bytes
after this patch: 1) rq->__data_len = sdp->sector_size 2) scsi_init_io() 3) rq->__data_len = rq->__data_len -> __data_len is wrong
It will cause that io can only complete one segment each time, and the io will requeue in scsi_io_completion_action(), which will cause severe performance degradation.
Fixes: 84f7a9de0602 ("scsi: sd: Remove a local variable") Signed-off-by: Yu Kuai yukuai3@huawei.com --- drivers/scsi/sd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index de6640ad1943..1e887c11e83d 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1072,6 +1072,7 @@ static blk_status_t sd_setup_write_same_cmnd(struct scsi_cmnd *cmd) struct bio *bio = rq->bio; u64 lba = sectors_to_logical(sdp, blk_rq_pos(rq)); u32 nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq)); + unsigned int nr_bytes = blk_rq_bytes(rq); blk_status_t ret;
if (sdkp->device->no_write_same) @@ -1108,7 +1109,7 @@ static blk_status_t sd_setup_write_same_cmnd(struct scsi_cmnd *cmd) */ rq->__data_len = sdp->sector_size; ret = scsi_alloc_sgtables(cmd); - rq->__data_len = blk_rq_bytes(rq); + rq->__data_len = nr_bytes;
return ret; }
On 10/28/22 19:58, Yu Kuai wrote:
This reverts commit 84f7a9de0602704bbec774a6c7f7c8c4994bee9c.
Because it introduces a problem that rq->__data_len is set to the wrong value.
before this patch:
- nr_bytes = rq->__data_len
- rq->__data_len = sdp->sector_size
- scsi_init_io()
- rq->__data_len = nr_bytes
after this patch:
- rq->__data_len = sdp->sector_size
- scsi_init_io()
- rq->__data_len = rq->__data_len -> __data_len is wrong
It will cause that io can only complete one segment each time, and the io will requeue in scsi_io_completion_action(), which will cause severe performance degradation.
It's probably worth mentioning that the code affected by this patch has been removed from the master branch and hence that this patch is only needed for stable kernels. Anyway:
Reviewed-by: Bart Van Assche bvanassche@acm.org
On Sat, Oct 29, 2022 at 04:10:47PM -0700, Bart Van Assche wrote:
On 10/28/22 19:58, Yu Kuai wrote:
This reverts commit 84f7a9de0602704bbec774a6c7f7c8c4994bee9c.
Because it introduces a problem that rq->__data_len is set to the wrong value.
before this patch:
- nr_bytes = rq->__data_len
- rq->__data_len = sdp->sector_size
- scsi_init_io()
- rq->__data_len = nr_bytes
after this patch:
- rq->__data_len = sdp->sector_size
- scsi_init_io()
- rq->__data_len = rq->__data_len -> __data_len is wrong
It will cause that io can only complete one segment each time, and the io will requeue in scsi_io_completion_action(), which will cause severe performance degradation.
It's probably worth mentioning that the code affected by this patch has been removed from the master branch and hence that this patch is only needed for stable kernels. Anyway:
Yes, that needs to be in the changelog in lots of detail.
Yu, please fix this up and resend.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org