* get_order() broken !
@ 2007-03-06 17:02 Benjamin Herrenschmidt
2007-03-06 17:13 ` Linus Torvalds
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2007-03-06 17:02 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: Andrew Morton, Linus Torvalds
Ok, maybe I have lots of shit in my eyes.... but it looks like get_order
in asm-generic/page.h is broken.
AFAIK, get_order() should round up (and we have code here or there
assuming such).
However, it seems to assume that ilog2 rounds up, which it doesn't. It
rounds down. At least the generic implementation seems to do so and the
powerpc optimized one too.
So I end up for example with get_order(0x1020) returning ... 0 (which
explains some memory corruption problems I'm having on ppc32 among
others0.
So who is right and who is wrong ? Should we audit all users of these
things ?
Cheers,
Ben.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: get_order() broken !
2007-03-06 17:02 get_order() broken ! Benjamin Herrenschmidt
@ 2007-03-06 17:13 ` Linus Torvalds
2007-03-06 17:43 ` David Howells
0 siblings, 1 reply; 3+ messages in thread
From: Linus Torvalds @ 2007-03-06 17:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt, David Howells
Cc: Linux Kernel Mailing List, Andrew Morton
On Tue, 6 Mar 2007, Benjamin Herrenschmidt wrote:
>
> Ok, maybe I have lots of shit in my eyes.... but it looks like get_order
> in asm-generic/page.h is broken.
>
> AFAIK, get_order() should round up (and we have code here or there
> assuming such).
>
> However, it seems to assume that ilog2 rounds up, which it doesn't. It
> rounds down. At least the generic implementation seems to do so and the
> powerpc optimized one too.
Yeah, I think the recent get_order() changes to change it to use ilog2()
are just crap. I actually looked at it when it went in, but I assumed that
it had been tested. I don't think it has.
Let's just revert that entirely.
David, quick comments before I undo commit 39d61db0?
Linus
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: get_order() broken !
2007-03-06 17:13 ` Linus Torvalds
@ 2007-03-06 17:43 ` David Howells
0 siblings, 0 replies; 3+ messages in thread
From: David Howells @ 2007-03-06 17:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Benjamin Herrenschmidt, Linux Kernel Mailing List, Andrew Morton
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> Yeah, I think the recent get_order() changes to change it to use ilog2()
> are just crap. I actually looked at it when it went in, but I assumed that
> it had been tested. I don't think it has.
It had, it's just that the testing totally failed to show up the problem; that
and I think my userspace testing went wrong because I made the wrong
assumptions about what the answer should be.
However, having discussed it with Ben, HPA and David Woodhouse and others I've
come up with a patch (sent separately) that should fix it by using:
ilog2((n) - 1) + 1
as the basis for the calculation as this should round up the result. I've
encapsulated this as a function called ilog2_up() and fixed the docs to make
it cleared how ilog2() and co actually work.
David
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-03-06 17:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-06 17:02 get_order() broken ! Benjamin Herrenschmidt
2007-03-06 17:13 ` Linus Torvalds
2007-03-06 17:43 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox