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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 30 Dec 2011 08:58:26 +0100
From:	Michal Simek <monstr@...str.eu>
To:	Tejun Heo <tj@...nel.org>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Yinghai Lu <yinghai@...nel.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Sam Ravnborg <sam@...nborg.org>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: memblock and bootmem problems if start + size = 4GB

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/