Re: bug in lmb_enforce_memory_limit()
On Fri, 2008-08-15 at 19:57 -0700, David Miller wrote: From: Michael Ellerman [EMAIL PROTECTED] Date: Sat, 16 Aug 2008 10:46:22 +1000 On Fri, 2008-08-15 at 15:25 -0700, David Miller wrote: Sounds great. Mind if I push the following to Linus? Looks good to me. I'll test it on Monday. I don't know if I have a system with memory holes to test on, but I take it you do? Works for me. Although I can't find a system with holes (the newer hypervisors make things look contiguous I think), so I can't really test the interesting case. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: bug in lmb_enforce_memory_limit()
From: Michael Ellerman [EMAIL PROTECTED] Date: Mon, 18 Aug 2008 12:00:32 +1000 Although I can't find a system with holes (the newer hypervisors make things look contiguous I think), so I can't really test the interesting case. I have several so I'll make sure it works in such cases :) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: bug in lmb_enforce_memory_limit()
From: Michael Ellerman [EMAIL PROTECTED]
Date: Thu, 14 Aug 2008 21:26:53 +1000
Perhaps after the first loop we should set memory_limit to equal
lmb_end_of_DRAM(), then the second loop should work as it is.
Sounds great. Mind if I push the following to Linus?
lmb: Fix reserved region handling in lmb_enforce_memory_limit().
The idea of the implementation of this fix is from Michael Ellerman.
This function has two loops, but they each interpret the memory_limit
value differently. The first loop interprets it as a size limit
whereas the second loop interprets it as an address limit.
Before the second loop runs, reset memory_limit to lmb_end_of_DRAM()
so that it all works out.
Signed-off-by: David S. Miller [EMAIL PROTECTED]
diff --git a/lib/lmb.c b/lib/lmb.c
index 5d7b928..97e5470 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -462,6 +462,8 @@ void __init lmb_enforce_memory_limit(u64 memory_limit)
if (lmb.memory.region[0].size lmb.rmo_size)
lmb.rmo_size = lmb.memory.region[0].size;
+ memory_limit = lmb_end_of_DRAM();
+
/* And truncate any reserves above the limit also. */
for (i = 0; i lmb.reserved.cnt; i++) {
p = lmb.reserved.region[i];
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: bug in lmb_enforce_memory_limit()
On Fri, 2008-08-15 at 15:25 -0700, David Miller wrote: From: Michael Ellerman [EMAIL PROTECTED] Date: Thu, 14 Aug 2008 21:26:53 +1000 Perhaps after the first loop we should set memory_limit to equal lmb_end_of_DRAM(), then the second loop should work as it is. Sounds great. Mind if I push the following to Linus? Looks good to me. I'll test it on Monday. I don't know if I have a system with memory holes to test on, but I take it you do? I notice some of our 32-bit code is using lmb_enforce_memory_limit() to enforce an address limit, which is technically broken, but is probably fine because it doesn't need to worry about holes. lmb: Fix reserved region handling in lmb_enforce_memory_limit(). The idea of the implementation of this fix is from Michael Ellerman. This function has two loops, but they each interpret the memory_limit value differently. The first loop interprets it as a size limit whereas the second loop interprets it as an address limit. Before the second loop runs, reset memory_limit to lmb_end_of_DRAM() so that it all works out. Signed-off-by: David S. Miller [EMAIL PROTECTED] Acked-by: Michael Ellerman [EMAIL PROTECTED] cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: bug in lmb_enforce_memory_limit()
On Thu, 2008-08-14 at 01:20 -0700, David Miller wrote: I just mentioned this to Ben H. on IRC and promised I would report it here. :-) The first loop over lmb.memory in this function interprets the memory_limit as a raw size limit, and that's fine so far. But the second loop over lmb.reserved interprets this value instead as an address limit. I haven't cobbled together a fix myself, but probably the way to do this is, when we're about break out of the first loop over lmb.memory, walk through the now-trimmed memory blobs and trim those from lmb.reserved, one by one. Perhaps after the first loop we should set memory_limit to equal lmb_end_of_DRAM(), then the second loop should work as it is. I think that actually makes memory_limit (the variable) more useful, and avoids more code like we have in numa_enforce_memory_limit(), which doesn't use memory_limit exactly because it isn't the value we're actually interested in (because of holes). This bug got introduced by: commit 2babf5c2ec2f2d5de3e38d20f7df7fd815fd10c9 Author: Michael Ellerman [EMAIL PROTECTED] Date: Wed May 17 18:00:46 2006 +1000 [PATCH] powerpc: Unify mem= handling back when LMB was still a powerpc local item. :-) Guilty as charged. I have some tests for that code, but clearly not enough - and it gets very little exercise otherwise. This led me to another bug which probably a lot of platforms are effected by. If you do this command line memory limiting, and the kernel was placed by the boot loader into physical ram (say at the end of the available physical memory) that gets trimmed out by the command line option, we hang or crash right as we boot into userspace because freeing up initmem ends up freeing invalid page structs. I think, on sparc64, instead of adding all kinds of complicated logic to free_initmem() I'm simply going to only poison the pages and not free them at all if cmdline_memory_size has been set. Would it be that much extra logic to check that the address is less than the limit? Especially if we changed memory_limit to incorporate holes. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
