PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)

[Hans de Goede]

James Bottomley wrote:
>>> Plus what is the rq->nr_sectors > sdp->sector_size /
>>> 512 test supposed to be doing?  that being true is supposed to be a
>>> guarantee of the block layer (and if something goes wrong there's a
>>> check for this lower down).
>>>
>> It first is was just:
>> rq->nr_sectors > 1
>>
>> Then I changed it to also do the right thing for 1024 and larger sector disks. 
>> The whole purpose is to not read the last sector in a larger then one sector 
>> read, so the amount of sectors gets reduced by one if (block + rq->nr_sectors 
>> == get_capacity(disk)) but we do want still want to be able to read the last 
>> sector by itself, so we must not reduce the no sectors to be read by one if it 
>> is already one.
> 
> Yes, I know that.  What I mean is the block subsystem sends reads and
> writes down in increments of the sector size.  Checking if the current
> number of pending sectors is greater than the current block size is
> checking that guarantee.  The current code already has a check in it to
> see if this guarantee is observed ... you don't need to check it again.
> 

I'm not checking for the guarantee, I'm checking that the read > 1 
realsize_sector, before decrementing the number of _512_bytes_sectors by 
realsize_sector, this check must be there, as after reading all but the last 
sector, another request will be send with 1 realsize_sector size, for which
(block + rq->nr_sectors) == get_capacity(disk) will still hold true, in this 
case this_count must not be lowered, otherwise this_count will become 0 in this 
case and the last sector will never get read.

I hope that clarifies why that code is there, if not can you tell how you would 
want the code to look by just giving the full if statement as it should be, I 
think we are somehow misunderstanding each other.

Regards,

Hans