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