Re: [PATCH 2/3] Setup min_low_pfn/max_low_pfn correctly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/11/06, Ralf Baechle <ralf@xxxxxxxxxxxxxx> wrote:
On Wed, Dec 06, 2006 at 04:48:29PM +0100, Franck Bui-Huu wrote:

> The old code was assuming that min_low_pfn was always 0. This
> means that we can't handle platforms with a big hole at start
> of memory since mem_map[] size would blew up.

It was - and IP22 was paying a high price for that.  It's currently 1MB
on 32-bit and 1.75MB on 64-bit kernels - but used to be much higher in
the past.

Btw, I think you're confusing the terms here; your patch description reads
wrong while the patch looks fine.  PFN is page frame number, not physical
frame number.  To avoid wasting dead mem_map[] entries idealy the pfn for
the first allocatable page should be zero.
So ignoring NUMA and discontig memory for a moment (those make everything
more complicated ...) PFN 0 is the first entry in mem_map[] - which usually
but not necessarily is physical address 0x0.


well if you look at the generic definition of pfn_to_page(), you find:

       #define __pfn_to_page(pfn)      (mem_map + ((pfn) - ARCH_PFN_OFFSET))

With the current implementation, PFN 0 is something that does not
exist. PFN ARCH_PFN_OFFSET is the first entry in mem_map[]...

I think I'm missing your point....

> This patch does not relax this constraint but it's a first
> step to achieve that.

> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 89440a0..8e58d7f 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -271,7 +271,6 @@ static void __init bootmem_init(void)
>  static void __init bootmem_init(void)
>  {
>       unsigned long reserved_end;
> -     unsigned long highest = 0;
>       unsigned long mapstart = -1UL;

Assigning a negative number to an unsigned long variable ...


Well, it's a convenient way to set mapstart to 0xffffffff on both
32bits and 64bits kernels as you guessed. I think it's quite readable
thanks to the 'UL' suffix. But I'll change the whole line with:

       unsigned long mapstart = ~(0UL);

>       unsigned long bootmap_size;
>       int i;
> @@ -284,6 +283,13 @@ static void __init bootmem_init(void)
>       reserved_end = max(init_initrd(), PFN_UP(__pa_symbol(&_end)));
>
>       /*
> +      * max_low_pfn is not a number of pages. The number of pages
> +      * of the system is given by 'max_low_pfn - min_low_pfn'.
> +      */
> +     min_low_pfn = -1UL;

Assigning a negative number to an unsigned long variable ...

ditto

thanks
--
              Franck


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux