[PATCH] fix alloc_large_system_hash roundup

[PATCH] fix alloc_large_system_hash roundup

[hawkes]

The "rounded up to nearest power of 2 in size" algorithm in
alloc_large_system_hash is not correct.  As coded, it takes an otherwise
acceptable power-of-2 value and doubles it.  For example, we see the
error if we boot with   thash_entries=2097152   which produces a hash
table with 4194304 entries.

Signed-off-by: John Hawkes <hawkes@sgi.com>

Index: linux/mm/page_alloc.c
===================================================================
--- linux.orig/mm/page_alloc.c	2006-03-14 15:25:40.000000000 -0800
+++ linux/mm/page_alloc.c	2006-03-14 16:06:48.000000000 -0800
@@ -2686,7 +2686,7 @@ void *__init alloc_large_system_hash(con
 			numentries <<= (PAGE_SHIFT - scale);
 	}
 	/* rounded up to nearest power of 2 in size */
-	numentries = 1UL << (long_log2(numentries) + 1);
+	numentries = 1UL << (long_log2(2*numentries - 1));
 
 	/* limit allocation size to 1/16 total memory by default */
 	if (max == 0) {

Re: [PATCH] fix alloc_large_system_hash roundup

[Roland Dreier]

 >  	/* rounded up to nearest power of 2 in size */
 > -	numentries = 1UL << (long_log2(numentries) + 1);
 > +	numentries = 1UL << (long_log2(2*numentries - 1));

How about just using roundup_pow_of_two()?  You could kill the comment
too then.

 - R.

RE: [PATCH] fix alloc_large_system_hash roundup

[Chen, Kenneth W]

Roland Dreier wrote on Wednesday, March 15, 2006 9:41 AM
> >  	/* rounded up to nearest power of 2 in size */
> > -	numentries = 1UL << (long_log2(numentries) + 1);
> > +	numentries = 1UL << (long_log2(2*numentries - 1));
> 
> How about just using roundup_pow_of_two()?  You could kill the comment
> too then.

roundup_pow_of_two uses fls, but fls takes an "int" argument.  I think
that function is buggy on 64-bit arch. Is it an oversight or something?


static inline unsigned long roundup_pow_of_two(unsigned long x)
{
        return (1UL << fls(x - 1));
}

- Ken

Re: [PATCH] fix alloc_large_system_hash roundup

[Roland Dreier]

    Kenneth> roundup_pow_of_two uses fls, but fls takes an "int"
    Kenneth> argument.  I think that function is buggy on 64-bit
    Kenneth> arch. Is it an oversight or something?

Huh, looks like you're right.  I never looked inside fls() before.
Yes, roundup_pow_of_two() should probably be fixed, since a naive
person (like me) would look at it and think it works on longs.

 - R.