Messages in this thread Patch in this message | | | From | Christoph Rohland <> | Subject | Re: [Patch] sysinfo compatibility | Date | 21 Aug 2001 19:30:04 +0200 |
| |
Hi Hugh,
On Tue, 21 Aug 2001, Hugh Dickins wrote: > On 21 Aug 2001, Christoph Rohland wrote: >> >> sysinfo does use a new mem_unit field if ram+swap > MAX_ULONG. That >> breaks 2.2 compatibility for a lot machines. >> >> I think it is more resonable to use the mem_unit field only if one >> of ram or swap is bigger than MAX_ULONG. (And 2.2 was only broken >> in that case) > > It's arguable. When I went there a few months back, I was a little > surprised by the way it adds ram+swap (it mistakenly added in more > before) to make that decision; but concluded it was helping callers > who might well go on to add ram+swap, and felt no overriding reason > to change that. But you can certainly argue that's inappropriate > for the kernel to do, that it should only guard the validity of > the actual numbers it returns to the caller. No strong feelings.
I think it's not the kernels task to fix simple user space errors by breaking compatibility.
And I have somewhat harder feelings since we get a lot of bug reports that our installer only detects 0M RAM when running on a 2.4 machine while it works with the 2.2 kernel. We are talking about an ABI which is directly imported into user space programs.
>> The appended patch implements that (and makes the logic a little >> bit easier) > > Alan, please don't apply. The patch made the logic a lot easier, > but sadly wrong: try what happens to totalswap 0x00120000 with > mem_unit 0x1000 - wrong decision since 0x20000000 > 0x00120000.
Uh, oh. Try this one instead:
Greetings Christoph
--- 8-ac8/kernel/info.c Tue Aug 21 09:54:02 2001 +++ u8-ac8/kernel/info.c Tue Aug 21 13:51:42 2001 @@ -16,6 +16,7 @@ asmlinkage long sys_sysinfo(struct sysinfo *info) { struct sysinfo val; + unsigned int mem_unit; memset((char *)&val, 0, sizeof(struct sysinfo)); @@ -32,47 +33,36 @@ si_meminfo(&val); si_swapinfo(&val); - { - unsigned long mem_total, sav_total; - unsigned int mem_unit, bitcount; - - /* If the sum of all the available memory (i.e. ram + swap) - * is less than can be stored in a 32 bit unsigned long then - * we can be binary compatible with 2.2.x kernels. If not, - * well, in that case 2.2.x was broken anyways... - * - * -Erik Andersen <andersee@debian.org> */ - - mem_total = val.totalram + val.totalswap; - if (mem_total < val.totalram || mem_total < val.totalswap) - goto out; - bitcount = 0; - mem_unit = val.mem_unit; - while (mem_unit > 1) { - bitcount++; - mem_unit >>= 1; - sav_total = mem_total; - mem_total <<= 1; - if (mem_total < sav_total) - goto out; - } - - /* If mem_total did not overflow, multiply all memory values by - * val.mem_unit and set it to 1. This leaves things compatible - * with 2.2.x, and also retains compatibility with earlier 2.4.x - * kernels... */ + /* + * If the the available memory or swap is less than can be + * stored in a 32 bit unsigned long then we can be binary + * compatible with 2.2.x kernels. If not, well, in that case + * 2.2.x was broken anyways... + * + * -Erik Andersen <andersee@debian.org> + */ + + mem_unit = val.mem_unit; + if (val.totalram <= ULONG_MAX / mem_unit && + val.totalswap <= ULONG_MAX / mem_unit) { + + /* + * If mem_total did not overflow, multiply all memory + * values by val.mem_unit and set it to 1. This + * leaves things compatible with 2.2.x, and also + * retains compatibility with earlier 2.4.x kernels... + */ val.mem_unit = 1; - val.totalram <<= bitcount; - val.freeram <<= bitcount; - val.sharedram <<= bitcount; - val.bufferram <<= bitcount; - val.totalswap <<= bitcount; - val.freeswap <<= bitcount; - val.totalhigh <<= bitcount; - val.freehigh <<= bitcount; + val.totalram *= mem_unit; + val.freeram *= mem_unit; + val.sharedram *= mem_unit; + val.bufferram *= mem_unit; + val.totalswap *= mem_unit; + val.freeswap *= mem_unit; + val.totalhigh *= mem_unit; + val.freehigh *= mem_unit; } -out: if (copy_to_user(info, &val, sizeof(struct sysinfo))) return -EFAULT; return 0; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |