Re: memblock and bootmem problems if start + size = 4GB

[Michal Simek]

Tejun Heo wrote:
> Hello,
>
> On Thu, Dec 29, 2011 at 05:46:18PM +0100, Michal Simek wrote:
> > First of all I don't like to use your term "extend range coverages".
> > We don't want to extend any ranges - we just wanted to place memory to the end
> > of address space and be able to work with.
>
> It is, as long as we use address ranges.  Either we can express length
> of zero or include the last address.
>
> > It is limitation which should be fixed somehow.
> > And I would expect that PFN_XX(base + size) will be in u32 range.
> >
> > Probably the best solution will be to use PFN macro in one place and
> > do not covert addresses in common code.
> >
> > + change parameters in bootmem code because some arch do
> > free_bootmem_node(..., PFN_PHYS(), ...)
> > and
> > reserve_bootmem_node(..., PFN_PHYS(), ...)
>
> So now we're talking about a lot of code just for ONE page and
> regardless of the representation in the memblock or other memory
> management code, I think trying to use that page is fundamentally a
> bad idea.  There are a lot of places in the kernel where phys_addr_t
> is used. 

I haven't said to replace phys_addr_t!
My point was something like this (just as example on parisc and free_bootmem_node).
The problematic part is kmemleak code which could be good reason not to change it.

Thanks,
Michal


diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index 82f364e..b83ee32 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -291,8 +291,8 @@ static void __init setup_bootmem(void)
                                                start_pfn,
                                                (start_pfn + npages) );
                free_bootmem_node(NODE_DATA(i),
-                                 (start_pfn << PAGE_SHIFT),
-                                 (npages << PAGE_SHIFT) );
+                                 start_pfn,
+                                 npages);
                bootmap_pfn += (bootmap_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
                if ((start_pfn + npages) > max_pfn)
                        max_pfn = start_pfn + npages;



diff --git a/mm/bootmem.c b/mm/bootmem.c
index 45a691a..dfbfc47 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -363,17 +363,12 @@ static int __init mark_bootmem(unsigned long start, unsigned long end,
  *
  * The range must reside completely on the specified node.
  */
-void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
-                             unsigned long size)
+void __init free_bootmem_node(pg_data_t *pgdat, unsigned long startpfn,
+                             unsigned long endpfn)
 {
-       unsigned long start, end;
-
-       kmemleak_free_part(__va(physaddr), size);
+       kmemleak_free_part(__va(startpfn << PAGE_SHIFT), (endpfn - startpfn) << PAGE_SHIFT);

-       start = PFN_UP(physaddr);
-       end = PFN_DOWN((u64)physaddr + (u64)size);
-
-       mark_bootmem_node(pgdat->bdata, start, end, 0, 0);
+       mark_bootmem_node(pgdat->bdata, startpfn, endpfn, 0, 0);
 }




--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/