Re: [PATCH v2] MIPS: Fix max_low_pfn with disabled highmem

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 2701 bytes --]

On Tue, Nov 01, 2016 at 12:50:51PM -0700, Florian Fainelli wrote:
> On 11/01/2016 06:59 AM, 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.
> > We also clip regions which overlap the highmem boundary when highmem is
> > disabled, so that max_pfn doesn't extend into highmem either.
> > 
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Paul Burton <paul.burton@imgtec.com>
> > Cc: linux-mips@linux-mips.org
> 
> Should this also go to -stable, if so, which kernels would be affected?

Well idle page tracking is quite a recent addition, commit 33c3fc71c8cf
("mm: introduce idle page tracking") in v4.3, but there could be other
places where pfns are walked with pfn_valid() where this problem could
break things.

But the actual code setting up max_low_pfn hasn't handled this case
properly since it was added in commit db84dc61552a ("[MIPS] Setup
min_low_pfn/max_low_pfn correctly") in v2.6.21.

At least for Malta where I saw it on QEMU however it doesn't appear
(from brief digging in history) to read ememsize environment except for
EVA kernels, except possibly since v4.4 with the new dtshim or a newish
u-boot capable of passing RAM size via DT.

So since I wasn't sure whether it really affects anybody else, and how
important it is to them, nor how far back it really matters (I was only
playing with idle page tracking since it hits the KVM_CAP_SYNC_MMU
related KVM arch callbacks), I figured I'd play it by ear. Thoughts
welcome.

Reproducing is simply a case of a 32bit kernel with CONFIG_HIGHMEM=n,
CONFIG_IDLE_PAGE_TRACKING=y, a platform that reports high memory regions
but with < 512MB of lowmem, then this to see if you get a kernel splat:

hexdump /sys/kernel/mm/page_idle/bitmap

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]