Messages in this thread | | | Date | Wed, 10 Dec 2008 14:15:15 +0100 (CET) | From | Geert Uytterhoeven <> | Subject | Re: Re[2]: [PATCH] fork_init: fix division by zero |
| |
On Wed, 10 Dec 2008, David Howells wrote: > Yuri Tikhonov <yur@emcraft.com> wrote: > > In any case, adding braces as follows probably would be better: > > > > + max_threads = mempages * (PAGE_SIZE / (8 * THREAD_SIZE)); > > I think you mean brackets, not braces '{}'. > > > Right ? > > Definitely not. > > I added this function to the above: > > unsigned long alt(void) > { > unsigned long max_threads; > max_threads = mempages * (PAGE_SIZE / (8 * THREAD_SIZE)); > return max_threads; > } > > and ran it through "gcc -S -O2" for x86_64: > > jump: > movq mempages(%rip), %rax > salq $12, %rax > shrq $16, %rax > ret > alt: > xorl %eax, %eax > ret > > Note the difference? In jump(), x86_64 first multiplies mempages by 4096, and > _then_ divides by 8*8192. > > In alt(), it just returns 0 because the compiler realised that you're > multiplying by 0.
The case were the multiplier is 0 (actually smaller than 1, but not integer) is handled by
#if (8 * THREAD_SIZE) > PAGE_SIZE max_threads = mempages / (8 * THREAD_SIZE / PAGE_SIZE); #else ...
> If you're going to bracket the expression, it must be: > > max_threads = (mempages * PAGE_SIZE) / (8 * THREAD_SIZE); > > which should be superfluous.
No, `mempages * PAGE_SIZE' may overflow.
With kind regards,
Geert Uytterhoeven Software Architect
Sony Techsoft Centre Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: Geert.Uytterhoeven@sonycom.com Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V. VAT BE 0413.825.160 · RPR Brussels Fortis · BIC GEBABEBB · IBAN BE41293037680010 -- 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/
| |