Re: [PATCH] x86/mm/init: respect memblock reserved regions whendestroying mappings

[Stefano Stabellini]

On Thu, 3 Feb 2011, H. Peter Anvin wrote:
> On 02/03/2011 03:25 AM, Stefano Stabellini wrote:
> >>
> >> How on Earth would you end up with a reserved region *inside the BRK*?
> > 
> > I think in practice you cannot, but you can have reserved regions at
> > _end, that is the main problem I am trying to solve.
> > If we have a reserved region at _end and _end is not PMD aligned, then
> > we have a problem.
> > 
> > I thought that checking for reserved regions before destroying the
> > mapping would be a decent solution (because it wouldn't affect the
> > normal case); so I ended up checking between _brk_end and _end too.
> > 
> > Other alternative solutions I thought about but that I discarded because
> > they also affect the normal case are:
> > 
> > - never destroy mappings that could go over _end;
> > - always PMD align _end.
> > 
> > If none of the above are acceptable, I welcome other suggestions :-)
> > 
> 
> Sounds like the code does the right thing, but the description needs to
> be improved.
> 

I tried to improve both the commit message and the comments within the
code, this is the result:


commit d0136be7b48953f27202dbde285a7379d06cfe98
Author: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date:   Tue Jan 25 12:05:11 2011 +0000

    x86/mm/init: respect memblock reserved regions when destroying mappings
    
    In init_memory_mapping we destroy the mappings between _brk_end and
    _end, but if _end is not PMD aligned we also destroy mappings for
    potentially reserved regions between _end and the following PMD.
    
    In order to avoid this problem, before clearing any PMDs we check if the
    corresponding memory area has been reserved and we only destroy the
    mapping if hasn't.
    
    We found this issue because under Xen we have a valid mapping at _end,
    and if _end is not PMD aligned the current code destroys the initial
    part of it.
    
    In practice this fix does not have any impact on native.
    
    Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 947f42a..65c34f4 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -283,6 +283,8 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
 	if (!after_bootmem && !start) {
 		pud_t *pud;
 		pmd_t *pmd;
+		unsigned long addr;
+		u64 size, memblock_addr;
 
 		mmu_cr4_features = read_cr4();
 
@@ -291,11 +293,22 @@ unsigned long __init_refok init_memory_mapping(unsigned long start,
 		 * located on different 2M pages. cleanup_highmap(), however,
 		 * can only consider _end when it runs, so destroy any
 		 * mappings beyond _brk_end here.
+		 *
+		 * If _end is not PMD aligned, we also destroy the mapping of
+		 * the memory area between _end the next PMD, so before clearing
+		 * the PMD we make sure that the corresponding memory region has
+		 * not been reserved.
 		 */
 		pud = pud_offset(pgd_offset_k(_brk_end), _brk_end);
 		pmd = pmd_offset(pud, _brk_end - 1);
-		while (++pmd <= pmd_offset(pud, (unsigned long)_end - 1))
-			pmd_clear(pmd);
+		addr = (_brk_end + PMD_SIZE - 1) & PMD_MASK;
+		while (++pmd <= pmd_offset(pud, (unsigned long)_end - 1)) {
+			memblock_addr = memblock_x86_find_in_range_size(__pa(addr),
+					&size, PMD_SIZE);
+			if (memblock_addr == (u64) __pa(addr) && size >= PMD_SIZE)
+				pmd_clear(pmd);
+			addr += PMD_SIZE;
+		}
 	}
 #endif
 	__flush_tlb_all();
--
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/