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-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
