Bart,

On 2/16/17 01:42, Bart Van Assche wrote:
> On Wed, 2017-02-15 at 11:12 +0900, Damien Le Moal wrote:
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 1f5d92a..d05a328 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -1790,6 +1790,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>>  {
>>      int result = SCpnt->result;
>>      unsigned int good_bytes = result ? 0 : scsi_bufflen(SCpnt);
>> +    unsigned int sector_size = SCpnt->device->sector_size;
>> +    unsigned int resid;
>>      struct scsi_sense_hdr sshdr;
>>      struct scsi_disk *sdkp = scsi_disk(SCpnt->request->rq_disk);
>>      struct request *req = SCpnt->request;
>> @@ -1820,6 +1822,24 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>>                      scsi_set_resid(SCpnt, blk_rq_bytes(req));
>>              }
>>              break;
>> +    default:
>> +            /*
>> +             * In case of bogus fw or device, we could end up having
>> +             * an unaligned partial completion. Check this here and force
>> +             * alignment.
>> +             */
>> +            resid = scsi_get_resid(SCpnt);
>> +            if (resid & (sector_size - 1)) {
>> +                    SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
>> +                            "Unaligned partial completion (resid=%u, 
>> sector_sz=%u)\n",
>> +                            resid, sector_size));
>> +                    resid = round_up(resid, sector_size);
>> +                    if (resid < good_bytes)
>> +                            good_bytes -= resid;
>> +                    else
>> +                            good_bytes = 0;
>> +                    scsi_set_resid(SCpnt, resid);
>> +            }
>>      }
>>  
>>      if (result) {
> 
> An additional concern: what if the size of the Data-Out buffer is not a
> multiple of the logical block size? Shouldn't we round down (good_bytes -
> resid) instead of rounding up resid?

The only REQ_TYPE_FS request that may not operate on LBA size aligned
payloads is REQ_OP_ZONE_REPORT which is handles in a different case of
the switch statement. So I think it is safe.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
damien.lem...@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

Reply via email to