On Fri, Aug 05, 2016 at 09:41:15AM +0100, Mel Gorman wrote:
> On Thu, Aug 04, 2016 at 09:59:17PM +0100, James Hogan wrote:
> > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > 
> > This breaks boot on metag architecture:
> > Oops: err 0007 (Data access general read/write fault) addr 00233008 [#1]
> > 
> > It appears to be in node_page_state_snapshot() (via
> > pgdat_reclaimable()), and have come via mm_init. Here's the relevant
> > bit of the backtrace:
> > 
> >     node_page_state_snapshot@0x4009c884(enum node_stat_item item =
> > ???, struct pglist_data * pgdat = ???) + 0x48
> >     pgdat_reclaimable(struct pglist_data * pgdat = 0x402517a0)
> >     show_free_areas(unsigned int filter = 0) + 0x2cc
> >     show_mem(unsigned int filter = 0) + 0x18
> >     mm_init@0x4025c3d4()
> >     start_kernel() + 0x204
> > 
> > __per_cpu_offset[0] == 0x233000 (close to bad addr),
> > pgdat->per_cpu_nodestats = NULL. and setup_per_cpu_pageset()
> > definitely hasn't been called yet (mm_init is called before
> > setup_per_cpu_pageset()).
> > 
> > Any ideas what the correct solution is (and why presumably others
> > haven't seen the same issue on other architectures?).
> > 
> 
> metag calls show_mem in mem_init() before the pagesets are initialised.

Indeed, I didn't spot yesterday evening that this appears to be
different to other arches.

> What's surprising is that it worked for the zone stats as it appears
> that calling zone_reclaimable() from that context should also have
> broken. Did anything change recently that would have avoided the
> zone->pageset dereference in zone_reclaimable() before?

It appears that zone_pcp_init() was already setting zone->pageset to
&boot_pageset, via paging_init():

zone_pcp_init@0x40265d54(struct zone * zone = ???)
free_area_init_core@0x40265c18(struct pglist_data * pgdat = ???) + 0x138
free_area_init_node(int nid = 0, unsigned long * zones_size = ???, unsigned long node_start_pfn = ???, unsigned long * zholes_size = ???) + 0x1a0
free_area_init_nodes(unsigned long * max_zone_pfn = ???) + 0x440
paging_init(unsigned long mem_end = 0x4fe00000) + 0x378
setup_arch(char ** cmdline_p = 0x4024e038) + 0x2b8
start_kernel() + 0x54

setup_arch() is called prior to mm_init(), which explains why it wasn't
crashing before.

> The easiest option would be to not call show_mem from arch code until
> after the pagesets are setup.

Since no other arches seem to do show_mem earily during boot like metag,
and doing so doesn't really add much value, I'm happy to remove it
anyway.

However could your change break other things and need fixing anyway?

Thanks!
James