[PATCH] bootmem.c clean up bad pfn convertions

[PATCH] bootmem.c clean up bad pfn convertions

[Franck Bui-Huu]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

Hi,

As I described in my previous email, bootmem.c does improper
pfn convertions into phys addr. This simple patch fixes that.

Regards,

       Franck.



-------------------------------------------------------------------------------
This message contains confidential information and is intended only for the
individual named. If you are not the named addressee you should not
disseminate, distribute or copy this e-mail. Please notify the sender
immediately by e-mail if you have received this e-mail by mistake and delete
this e-mail from your system. E-mail transmission cannot be guaranteed to be
secure or error-free as information could be intercepted, corrupted, lost,
destroyed, arrive late or incomplete, or contain viruses. The sender therefore
does not accept liability for any errors or omissions in the contents of this
message, which arise as a result of e-mail transmission. If verification is
required please request a hard-copy version.
INNOVA CARD will not therefore be liable for the message if modified.
-------------------------------------------------------------------------------

[-- Attachment #2: bootmem.c.patch --]
[-- Type: text/x-patch, Size: 2530 bytes --]

--- bootmem.c	7 Apr 2005 16:39:24 -0000	1.2
+++ bootmem.c	8 Apr 2005 13:20:02 -0000
@@ -58,8 +58,8 @@ static unsigned long __init init_bootmem
 	pgdat_list = pgdat;
 
 	mapsize = (mapsize + (sizeof(long) - 1UL)) & ~(sizeof(long) - 1UL);
-	bdata->node_bootmem_map = phys_to_virt(mapstart << PAGE_SHIFT);
-	bdata->node_boot_start = (start << PAGE_SHIFT);
+	bdata->node_bootmem_map = phys_to_virt(pfn_to_phys(mapstart));
+	bdata->node_boot_start = pfn_to_phys(start);
 	bdata->node_low_pfn = end;
 
 	/*
@@ -86,11 +86,11 @@ static void __init reserve_bootmem_core(
 	unsigned long sidx = (addr - bdata->node_boot_start)/PAGE_SIZE;
 	unsigned long eidx = (addr + size - bdata->node_boot_start + 
 							PAGE_SIZE-1)/PAGE_SIZE;
-	unsigned long end = (addr + size + PAGE_SIZE-1)/PAGE_SIZE;
+	unsigned long end = phys_to_pfn(PAGE_ALIGN(addr + size));
 
 	BUG_ON(!size);
 	BUG_ON(sidx >= eidx);
-	BUG_ON((addr >> PAGE_SHIFT) >= bdata->node_low_pfn);
+	BUG_ON(phys_to_pfn(addr) >= bdata->node_low_pfn);
 	BUG_ON(end > bdata->node_low_pfn);
 
 	for (i = sidx; i < eidx; i++)
@@ -111,7 +111,7 @@ static void __init free_bootmem_core(boo
 	 */
 	unsigned long sidx;
 	unsigned long eidx = (addr + size - bdata->node_boot_start)/PAGE_SIZE;
-	unsigned long end = (addr + size)/PAGE_SIZE;
+	unsigned long end = phys_to_pfn(addr + size);
 
 	BUG_ON(!size);
 	BUG_ON(end > bdata->node_low_pfn);
@@ -260,6 +260,7 @@ static unsigned long __init free_all_boo
 	unsigned long i, count, total = 0;
 	unsigned long idx;
 	unsigned long *map; 
+	unsigned long mapsize;
 	int gofast = 0;
 
 	BUG_ON(!bdata->node_bootmem_map);
@@ -267,7 +268,7 @@ static unsigned long __init free_all_boo
 	count = 0;
 	/* first extant page of the node */
 	page = virt_to_page(phys_to_virt(bdata->node_boot_start));
-	idx = bdata->node_low_pfn - (bdata->node_boot_start >> PAGE_SHIFT);
+	idx = bdata->node_low_pfn - phys_to_pfn(bdata->node_boot_start);
 	map = bdata->node_bootmem_map;
 	/* Check physaddr is O(LOG2(BITS_PER_LONG)) page aligned */
 	if (bdata->node_boot_start == 0 ||
@@ -313,7 +314,9 @@ static unsigned long __init free_all_boo
 	 */
 	page = virt_to_page(bdata->node_bootmem_map);
 	count = 0;
-	for (i = 0; i < ((bdata->node_low_pfn-(bdata->node_boot_start >> PAGE_SHIFT))/8 + PAGE_SIZE-1)/PAGE_SIZE; i++,page++) {
+	mapsize = (bdata->node_low_pfn - phys_to_pfn(bdata->node_boot_start)+7)/8;
+	mapsize = (mapsize + PAGE_SIZE-1)/PAGE_SIZE;
+	for (i = 0; i < mapsize; i++, page++) {
 		count++;
 		__ClearPageReserved(page);
 		set_page_count(page, 1);

Re: [PATCH] bootmem.c clean up bad pfn convertions

[Dave Hansen]

On Fri, 2005-04-08 at 15:32 +0200, Franck Bui-Huu wrote:
> As I described in my previous email, bootmem.c does improper
> pfn convertions into phys addr. This simple patch fixes that.
...
> -       bdata->node_bootmem_map = phys_to_virt(mapstart << PAGE_SHIFT);
> -       bdata->node_boot_start = (start << PAGE_SHIFT);
> +       bdata->node_bootmem_map = phys_to_virt(pfn_to_phys(mapstart));
> +       bdata->node_boot_start = pfn_to_phys(start);

The only arch with phys_to_pfn() defined is UML, so the patch simply
won't compile anything but UML on current kernels (unless I'm missing
something).

Could you try to give us a more complete description of your problem?  I
know your memory doesn't start at 0x0, but what problems does that
cause?  Does the mem_map[] allocation blow up, etc...  

If it's just mem_map[], That calculation could be fixed pretty easily.
Something like

+#ifdef CONFIG_CRAZY_MIPS_FOO_MEM_MAP_START... 
+extern unsigned long mem_map_start_pfn
+#else
+#define mem_map_start_pfn 0UL
+#endif
-#define pfn_to_page(pfn)        (mem_map + (pfn))
+#define pfn_to_page(pfn)        (mem_map + (pfn) - mem_map_start_pfn)

(those names are horrid, please improve them, if you plan to do this)

All of the zone (and allocator) calculations should be just fine,
because it already has a zone_start_pfn.

-- Dave

Re: [PATCH] bootmem.c clean up bad pfn convertions

[Franck Bui-Huu]

Dave Hansen wrote:

>The only arch with phys_to_pfn() defined is UML, so the patch simply
>won't compile anything but UML on current kernels (unless I'm missing
>something).
>  
>
oops, I forgot to send the part of the patch that defines these macros, 
sorry.

>Could you try to give us a more complete description of your problem?  I
>know your memory doesn't start at 0x0, but what problems does that
>cause?  Does the mem_map[] allocation blow up, etc...  
>  
>
yes, it actually intends to solve that.

>If it's just mem_map[], That calculation could be fixed pretty easily.
>Something like
>
>+#ifdef CONFIG_CRAZY_MIPS_FOO_MEM_MAP_START... 
>+extern unsigned long mem_map_start_pfn
>+#else
>+#define mem_map_start_pfn 0UL
>+#endif
>-#define pfn_to_page(pfn)        (mem_map + (pfn))
>+#define pfn_to_page(pfn)        (mem_map + (pfn) - mem_map_start_pfn)
>
>
>  
>
This is another solution indeed...But you will have to define another 
constant which
will be used for virtual to physical address convertion. In my case I have

#define phys_to_pfn(x)        (((x) - PHYS_START) >> PAGE_SHIFT)
#define pa(x)       ((x) - PAGE_OFFSET + PHYS_START)

and "pfn_to_page" stays untouch.
But I expect your solution easier to implement since as you said 
previously no arch
defines "phys_to_pfn".

Thanks

          Franck.

-------------------------------------------------------------------------------
Come to visit Innova Card at CTST (Las Vegas, April 11-14, 2005) on booth 559 
and Cards Asia (Singapore, April 27-29, 2005) on booth 4A04 !

links:
 - www.ctst.com
 - www.worldofcards.biz/2005/cardsa_SG/

-------------------------------------------------------------------------------
This message contains confidential information and is intended only for the
individual named. If you are not the named addressee you should not
disseminate, distribute or copy this e-mail. Please notify the sender
immediately by e-mail if you have received this e-mail by mistake and delete
this e-mail from your system.
Innova Card will not therefore be liable for the message if modified.