| Message ID | 1227557235.17746.25.camel@localhost.localdomain (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote: > On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote: > > > > If this is all too much, then I'm close to giving up and burning a > > 64KB page, which requires only ALIGN_DOWN() in the kernel. > > ppc: force memory size to be a multiple of PAGE_SIZE > > Ensure that total memory size is page-aligned, because otherwise > bootmem.c gets upset. > > This error case was triggered by using 64 KiB pages in the kernel while > arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by 4096 (to > work around the "CHIP11" errata which affects the last 256 bytes of physical memory). > > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> > --- > This is on a common code path, and lmb_enforce_memory_limit() will now > always take action, so wider testing would be good. > > This patch supercedes http://patchwork.ozlabs.org/patch/8211/ . > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par > early_reserve_mem(); > phyp_dump_reserve_mem(); > > + /* Ensure that total memory size is page-aligned, because otherwise > + * bootmem.c gets upset. */ > + lmb_analyze(); > + memory_limit = lmb_phys_mem_size() & PAGE_MASK; All of the current code using memory_limit looks like it'll be safe with this change, although there are several cases of this we could remove: if (memory_limit && <some other condition>) Because memory_limit will now always be true. Still, I think it would be better to only set memory_limit when the mem size is not a multiple of the PAGE_SIZE - so that memory_limit retains it's function as both the value of the limit and a boolean. cheers
On Nov 24, 2008, at 6:10 PM, Michael Ellerman wrote: > On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote: >> On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote: >>> >>> If this is all too much, then I'm close to giving up and burning a >>> 64KB page, which requires only ALIGN_DOWN() in the kernel. >> >> ppc: force memory size to be a multiple of PAGE_SIZE >> >> Ensure that total memory size is page-aligned, because otherwise >> bootmem.c gets upset. >> >> This error case was triggered by using 64 KiB pages in the kernel >> while >> arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by >> 4096 (to >> work around the "CHIP11" errata which affects the last 256 bytes of >> physical memory). >> >> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> >> --- >> This is on a common code path, and lmb_enforce_memory_limit() will now >> always take action, so wider testing would be good. >> >> This patch supercedes http://patchwork.ozlabs.org/patch/8211/ . >> >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >> --- a/arch/powerpc/kernel/prom.c >> +++ b/arch/powerpc/kernel/prom.c >> @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par >> early_reserve_mem(); >> phyp_dump_reserve_mem(); >> >> + /* Ensure that total memory size is page-aligned, because otherwise >> + * bootmem.c gets upset. */ >> + lmb_analyze(); >> + memory_limit = lmb_phys_mem_size() & PAGE_MASK; > > All of the current code using memory_limit looks like it'll be safe > with > this change, although there are several cases of this we could remove: > > if (memory_limit && <some other condition>) > > Because memory_limit will now always be true. memory_limit was the result of parsing mem= from the command line. Does this break that? > > Still, I think it would be better to only set memory_limit when the mem > size is not a multiple of the PAGE_SIZE - so that memory_limit retains > it's function as both the value of the limit and a boolean. I would have expected this trimming to occur where we actually transfer the memory from lmb to bootmem, since it is bootmem that has the aligned size requirement. milton
On Tue, 2008-11-25 at 11:10 -0600, Milton Miller wrote: > On Nov 24, 2008, at 6:10 PM, Michael Ellerman wrote: > > On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote: > >> On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote: > >>> > >>> If this is all too much, then I'm close to giving up and burning a > >>> 64KB page, which requires only ALIGN_DOWN() in the kernel. > >> > >> ppc: force memory size to be a multiple of PAGE_SIZE > >> > >> Ensure that total memory size is page-aligned, because otherwise > >> bootmem.c gets upset. > >> > >> This error case was triggered by using 64 KiB pages in the kernel > >> while > >> arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by > >> 4096 (to > >> work around the "CHIP11" errata which affects the last 256 bytes of > >> physical memory). > >> > >> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> > >> --- > >> This is on a common code path, and lmb_enforce_memory_limit() will now > >> always take action, so wider testing would be good. > >> > >> This patch supercedes http://patchwork.ozlabs.org/patch/8211/ . > >> > >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > >> --- a/arch/powerpc/kernel/prom.c > >> +++ b/arch/powerpc/kernel/prom.c > >> @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par > >> early_reserve_mem(); > >> phyp_dump_reserve_mem(); > >> > >> + /* Ensure that total memory size is page-aligned, because otherwise > >> + * bootmem.c gets upset. */ > >> + lmb_analyze(); > >> + memory_limit = lmb_phys_mem_size() & PAGE_MASK; > > > > All of the current code using memory_limit looks like it'll be safe > > with > > this change, although there are several cases of this we could remove: > > > > if (memory_limit && <some other condition>) > > > > Because memory_limit will now always be true. > > memory_limit was the result of parsing mem= from the command line. > Does this break that? If you're asking me, no, the patch doesn't break that. early_parse_mem() already aligns memory_limit, so the additional mask does nothing. > > Still, I think it would be better to only set memory_limit when the mem > > size is not a multiple of the PAGE_SIZE - so that memory_limit retains > > it's function as both the value of the limit and a boolean. > > I would have expected this trimming to occur where we actually transfer > the memory from lmb to bootmem, since it is bootmem that has the > aligned size requirement. No good; since LMB knows the memory exists, it allocates space for the unflattened device tree in the last page, and then uses reserve_bootmem() for that space and again, bootmem BUG()s. Anyways, the LMB interface is wide, with lots of users directly accessing LMB data structures. There are enough permutations of highmem, crashdump kernels, et al that I don't feel comfortable trying to audit and test all the LMB users.
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par early_reserve_mem(); phyp_dump_reserve_mem(); + /* Ensure that total memory size is page-aligned, because otherwise + * bootmem.c gets upset. */ + lmb_analyze(); + memory_limit = lmb_phys_mem_size() & PAGE_MASK; + lmb_enforce_memory_limit(memory_limit); lmb_analyze();