Re: weirdness in bootmem_init(), arch/mips64/kernel/setup.c

[Juan Quintela <quintela@mandrakesoft.com>]

>>>>> "andrew" == Andrew Clausen <clausen@melbourne.sgi.com> writes:

andrew> Hi all,
andrew> This code isn't really relevant to what I'm working on (it isn't compiled
andrew> in to kernels for the ip27), but I just noticed it, and it looks broken:

andrew> /* Find the highest page frame number we have available.  */
andrew> max_pfn = 0;
andrew> for (i = 0; i < boot_mem_map.nr_map; i++) {
andrew> unsigned long start, end;

andrew> if (boot_mem_map.map[i].type != BOOT_MEM_RAM)
andrew> continue;

andrew> *****           start = PFN_UP(boot_mem_map.map[i].addr);
andrew> *****           end = PFN_DOWN(boot_mem_map.map[i].addr
andrew> + boot_mem_map.map[i].size);

andrew> *****           if (start >= end)
andrew> continue;
andrew> if (end > max_pfn)
andrew> max_pfn = end;
andrew> }


andrew> That test looks like it will always succeed... and it looks like the
andrew> author wanted it to be a sanity check.

andrew> Why all this business with PFN_UP and PFN_DOWN?  (They are bit
andrew> shifts... PFN_UP shifts left, PFN_DOWN shifts right)

Not completely sure, but I think that it is related with the weird
discontig memory that Origins (and I think other MIPS machines) have.

(Just having put the file where that thing are, will have saved me a
grep :)

1st- Looking at the code, both of them shift right:

#define PFN_UP(x)	(((x) + PAGE_SIZE - 1) >> PAGE_SHIFT)
#define PFN_DOWN(x)	((x) >> PAGE_SHIFT)

PFN_UP -> page frame of next page
PFN_DOWN -> page frame of this page

2nd - if the region is empty (size = 0), start will be == end, which
      means that we don't considerd that area for checking what memory
      are available.

Standard disclaimer: That is my reading/things that I remind about
   that, any resemblance with reality can be pure coincidence :p  I am
   not an expert in SGI machines lowlevel details but any mean.

Later, Juan.

-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy