Hi Martin, thanks for your comments. On Wed, Jun 27, 2018 at 8:24 PM, Martin Steigerwald <martin@xxxxxxxxxxxx> wrote:
Thanks a lot again for your patch. schmitzmic@xxxxxxxxx - 27.06.18, 03:24:+ if (start_sect > INT_MAX || nr_sects > INT_MAX + || (start_sect + nr_sects) > INT_MAX) { + pr_err("%s: Warning: RDB partition overflow!\n", + bdevname(state->bdev,I´d word this: Warning: RDB partition 32-bit overflow AmigaOS developers can do 64 bit math on a 32 bit operating system. Just like Linux can.
Yes, I realize that. I hadn't gone back through all the mails on the subject to find out what the exact requrements are on the AmigaOS side. Just trying to be as terse as possible to keep checkpatch happy :-(
b)); + pr_err("%s: start 0x%llX size 0x%llX\n", + bdevname(state->bdev, b), start_sect, + nr_sects); + pr_err("%s: partition incompatible with 32 bit OS\n", + bdevname(state->bdev, b)); + }And as stated in my other reply to the patch: partition needs 64 bit disk device support in AmigaOS or AmigaOS like operating systems (NSD64, TD64 or SCSI direct)
I'd probably leave it at 'disk needs 64 bit disk device support on native OS', and only print that warning once. Geert has raised another important point about 64 bt device support - all this is moot when the Linux kernel wasn't built with large block device support enabled (you'd get the same buggy behaviour as before the patch there).
see my other reply to the patch and my other mails in the "Re: moving affs + RDB partition support to staging?" thread as to why. And for references.
Thanks for collating all the references. Please understand that I can't read all of that, and as a simple patch mechanic I won't even try to grasp all the subtleties of RDB (I don't even own an Amiga so I am quite unlikey to ever use this code path). But please also understand that for that reason, I take Joanne's advice about backwards compatibility very serious. My patch (actually Joanne's originally) changes kernel behaviour from what we consider broken (allowing 32 bit overflow in partition address calculations) to what we think is the right thing to do. But there might be someone out there who used the current behaviour to craft a RDB that aliows two separate sets of partitions to coexist on the same disk (one set visible to 32 bit disk drivers, before the 32 bit overflow mark, and a second set above that mark, visible only to 64 bit drivers. Silently changing our parser behaviour might cause said user to now trash data past the overflow mark.). This is a little contrived, and perhaps I am overcomplicating matters (again), but can't be ruled out. In the interest of least surprises, we have to fix the 32 bit overflow (so we can even detect that it would have happened), and give the user the chance to carefully consider whether to accept the new behaviour. That means refusing to make available any partition that would have been affected by such overflow. The user has then all options available - force old behaviour by using an older kernel, override the parser to force new behaviour (which we all assume is correct), or leave the disk well alone. Cheers, Michael
Thanks, -- Martin
-- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
