On Tue, Nov 01, 2016 at 09:11:43AM +0000, Paul Burton wrote: > On Tuesday, 1 November 2016 00:40:29 GMT James Hogan wrote: > > When low memory doesn't reach HIGHMEM_START (e.g. up to 256MB at PA=0 is > > common) and highmem is present above HIGHMEM_START (e.g. on Malta the > > RAM overlayed by the IO region is aliased at PA=0x90000000), max_low_pfn > > will be initially calculated very large and then clipped down to > > HIGHMEM_START. > > > > This causes crashes when reading /sys/kernel/mm/page_idle/bitmap > > (i.e. CONFIG_IDLE_PAGE_TRACKING=y) when highmem is disabled. pfn_valid() > > will compare against max_mapnr which is derived from max_low_pfn when > > there is no highend_pfn set up, and will return true for PFNs right up > > to HIGHMEM_START, even though they are beyond the end of low memory and > > no page structs will actually exist for these PFNs. > > > > This is fixed by skipping high memory regions when initially calculating > > max_low_pfn if highmem is disabled, so it doesn't get clipped too high. > > > > Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx> > > Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx> > > Cc: linux-mips@xxxxxxxxxxxxxx > > --- > > arch/mips/kernel/setup.c | 9 +++++++++ > > 1 file changed, 9 insertions(+), 0 deletions(-) > > > > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c > > index 0d57909d9026..cb6e5895bb7e 100644 > > --- a/arch/mips/kernel/setup.c > > +++ b/arch/mips/kernel/setup.c > > @@ -368,6 +368,15 @@ static void __init bootmem_init(void) > > end = PFN_DOWN(boot_mem_map.map[i].addr > > + boot_mem_map.map[i].size); > > > > +#ifndef CONFIG_HIGHMEM > > + /* > > + * Skip highmem here so we get an accurate max_low_pfn if low > > + * memory stops short of high memory. > > + */ > > + if (start >= PFN_DOWN(HIGHMEM_START)) > > + continue; > > +#endif > > + > > if (end > max_low_pfn) > > max_low_pfn = end; > > if (start < min_low_pfn) > > Hi James, > > Shouldn't this also clip any region which crosses the boundary from lowmem to > highmem? (ie. start < PFN_DOWN(HIGHMEM_START) < end) I did do this at first, but then I realised it was redundant since max_low_pfn already gets clipped to PFN_DOWN(HIGHMEM_START) after the loop, i.e. it already does the right thing in that case since memory reaches up to HIGHMEM_START. The only difference would be max_pfn: 1) At the moment max_pfn includes highmem even when highmem is disabled. 2) With this patch max_pfn would only include highmem when highmem is disabled if a memory region crosses the highmem boundary. 3) Clipping end to PFN_DOWN(HIGHMEM_START) in the #ifndef above would cause max_pfn to exclude highmem when highmem is disabled. Individual pfns are usually checked with pfn_valid() anyway, however (3) still sounds more correct (and possibly more efficient when pfns are iterated over). It would also correct the number of pfns that can be read in /sys/kernel/mm/page_idle/bitmap to exclude highmem when it does overlap the boundary. Sound about right? I'll do a v2. Thanks for reviewing, Cheers James
Attachment:
signature.asc
Description: Digital signature
