Re: sd_mod or usb-storage fails to read a single good block (was: ehci_hcd fails to read a single good block)

[Norman Diamond <n0diamond@xxxxxxxxxxx>]

Alan Stern wrote:
> On Fri, 27 Apr 2012, Norman Diamond wrote:
>>> 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.
> 
> Look, I didn't say the changes would help with every possible disk.  
> Just that they would fix your immediate problem, i.e., the problem with
> your current disk.

"Look" ...
We already found a workaround for what you thought was my immediate problem, one of the bridges that can test with and one of the disks that I can test with.  A kernel boot parameter can remove all quirks flags for the Prolific bridge.

That is not a fix for what really is my immediate problem, which is that my users can have any kind of bridge and any kind of disk, and in an emergency I can tell them to add a kernel boot parameter, or see if they can use a different bridge, etc., but it's really better if we can avoid such emergencies.

>>> 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.
> 
> That's the whole point of CAPACITY_HEURISTICS -- we have two versions 
> of the device, of which one is buggy and the other isn't, and we can't 
> tell them apart.  If we preferred to take the device's word for it 
> whenever we didn't know for sure, there would be no need for 
> CAPACITY_HEURISTICS at all.

A preference for taking the device's word (in cases where we don't have a more reliable heuristic) is different from always taking the device's word (in all cases even when we have a more reliable heuristic).  CAPACITY_HEURISTICS will remain meaningful and will become more meaningful.

> For example, consider all those cameras sold by Nikon, Pentax, and so
> on.  They probably all used Symbian's firmware, which some years ago  
> had the capacity bug.  Maybe the firmware has been fixed since then; I
> don't know.  But under the reasonable assumption that the
> SD/CF/whatever memories used in these cameras will always have an even 
> number of blocks, the existing CAPACITY_HEURISTICS solution is 
> appropriate.

If you think that a revised CAPACITY_HEURISTICS incorporating my suggestion will become less appropriate than the existing CAPACITY_HEURISTICS then we need two separate heuristics, one for Symbian and similar, one for Prolific and similar.  And we even more surely do need TEST_CAPACITY.

>>> 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.
> 
> ?  That makes no sense.  Go back and reread what I wrote.  In 
> particular, I was asking why you changed the test from (n % 63 == 1) to 
> (n % 63 != 0).

My change was bigger than that.  Where you had ||, I had &&, though in one of my two codings I accidentally deleted one of the &&s.

6363 is an example.  Your code would cut it to 6362.  I think it should stay 6363, because for decates a lot of ATA disks reported capacities that were multiples of 63.

> You know that for decades, ATA disks have reported capacities that were
> multiples of 63.  So if a drive reports a capacity that is 5 larger
> than a multiple of 63, why would you want to decrement the value?

Where a report is 5 larger than a multiple of 63, I left the existing heuristic intact.  I suggested only modifying the heuristic to avoid decrementing from exact multiples of 63.

> The result still would not be a multiple of 63.  Didn't you say that you 
> preferred to take the device's word for it?

Only when we have unresolvable ambiguities.

>>> 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.
> 
> The same caveat applies to TEST_CAPACITY.
> 
> Also, the amount of effort required to write the TEST_CAPACITY code and 
> get it accepted into the kernel would be non-trivial.  I prefer not to 
> do it unless it turns out to be clearly necessary.

I think it is clearly necessary.  In the meantime a revised heuristic will help a lot.

By the way, I will not be the primary beneficiary of these revisions.  The primary beneficiaries will be owners of bridges and devices that you and I have learned to dislike, but we have to live with them.
--
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