Alan Stern wrote:
> On Thu, 26 Apr 2012, Norman Diamond wrote:
>
>> OK, the new code for heuristics makes it PROBABLY good enough to
>> change the Prolific bridge from FIX_CAPACITY to CAPACITY_HEURISTICS.
>>
>>> Below are two patches that do pretty much what you are asking for.
>>> Please test just the first and then both together.
>>
>> Ouch. Yeah I should expect it to be my responsibility to test, but
>> rebuilding the live Linux system that I use is particularly painful.
>> (Of course I have to be grateful that it is even possible in the
>> first place, regardless of being painful.)
>>
>> Would it be acceptable for me to donate my USB-to-ATA cable to you?
>
> Let's just not worry about it. It seems pretty clear that the patches
> will fix your immediate problem.
Only partly, because an odd multiple of 63 is possible.
>>> + (sdp->guess_capacity && (n % 2 == 1 || n % 63 == 1))) {
>>
>> Imagine a device with 63 blocks. guess_capacity is true and n % 2 ==
>> 1 so this is going to truncate when we don't want to.
>
> Yes. As the comment at the start of this code says, we err on the side
> of truncating. Besides, that's what the current algorithm does.
>
>> How is one of these, either in my style:
>> (sdp->guess_capacity && (n % 2 != 0) && (n % 63 != 0))) {
>>
>> or a popular coding style:
>> (sdp->guess_capacity && n % 2 n % 63)) {
>
> (I have no idea what "popular coding style" that might be -- it
> contains a syntax error!)
I lost an && in editing. But it seems to be popular to omit redundant parentheses and to omit extra operations such as "!= 0" when, as in this case, they don't affect the result. Both of these kinds of redundancies only assist readability for a subset of human readers, not for compilers.
(sdp->guess_capacity && n % 2 && n % 63)) {
Anyway, the version with my coding style showed what I suggest and why.
> What about a device that has 6362 sectors but reports 6363? Then your
> test would fail to decrement the value when we do want to.
But we can't distinguish two versions of the device, one where 6363 is correct and one where 6363 is wrong. I prefer to take the device's word for it unless we know for sure that it is wrong. TEST_CAPACITY would still be a good idea. FIX_CAPACITY (where we know for sure that the device is wrong) will not suffer from this ambiguity and will always get a decrement.
> Anyway, why do you want to test for n % 63 != 0? You have no
> justification for decrementing the block count if the result would
> not be divisible by 63.
I sure do. You know as well as I do that for decades a lot of ATA disks reported capacities that were multiples of 63 and they had a reason for it.
> (Interestingly, I just found an email thread that appears to be the
> origin of the CAPACITY_HEURISTICS flag:
> http://marc.info/?l=linux-usb-devel&m=117025868105192&w=2
> Among other things, it suggests testing for divisibility by 255 * 63
> instead of by 63, which ought to help reduce the number of errors.
A few weeks ago I also had the idea of testing divisibility by 255 * 63, but very quickly disproved this idea. I still have a lot of drives that report multiples of 63 for the obvious reason, but they are not multiples of 255 * 63. BIOSes, and software that has to provide some amount of interoperability with BIOSes, do geometry translation to get a multiple of 255 * 63, and then they don't use the entire drive.
> Bear in mind that if anybody reports a problem caused by the new
> "divisible by 63" criterion, we will have to revert the changed test
> (the updated message could remain, since it doesn't affect
> functionality).
Mostly I agree. But if reversion causes a different known problem to be reinstated, then TEST_CAPACITY is really needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
