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

Hans de Goede j.w.r.degoede at hhs.nl
Sun Jan 20 10:12:16 UTC 2008


James Bottomley wrote:
> On Mon, 2008-01-14 at 20:27 +0100, Hans de Goede wrote:
>> 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.
> 
> Oh, right; it's > rather than >= ... sorry, yes that's fine.
> 

Ok,

I got swamped @work this week so it took a while, but I've made a new seperate 
(scsi-sd only) patch with the cleanups as discussed.

I'm sending this (to the full recipient list) in a new top level post titled:
PATCH: scsi-sd-last-sector-bug-flag.patch

Regards,

Hans




More information about the Fedora-kernel-list mailing list