* RE: Regarding patch "mm: meminit: make __early_pfn_to_nid SMP-safe and introduce meminit_pfn_in_nid"
[not found] ` <20150715145041.GE6812@suse.de>
@ 2015-07-15 22:45 ` Alex Ng (LIS)
2015-07-16 12:47 ` Mel Gorman
0 siblings, 1 reply; 2+ messages in thread
From: Alex Ng (LIS) @ 2015-07-15 22:45 UTC (permalink / raw)
To: Mel Gorman, Waiman Long
Cc: Robin Holt, Nate Zimmer, Dave Hansen, Scott Norton, Luck, Tony,
Ingo Molnar, H. Peter Anvin, Thomas Gleixner,
linux-kernel@vger.kernel.org
> -----Original Message-----
> From: Mel Gorman [mailto:mgorman@suse.de]
> Sent: Wednesday, July 15, 2015 7:51 AM
> To: Waiman Long <waiman.long@hp.com>
> Cc: Alex Ng (LIS) <alexng@microsoft.com>; Robin Holt
> <robinmholt@gmail.com>; Nate Zimmer <nzimmer@sgi.com>; Dave Hansen
> <dave.hansen@intel.com>; Scott Norton <scott.norton@hp.com>; Luck,
> Tony <tony.luck@intel.com>; Ingo Molnar <mingo@elte.hu>; H. Peter Anvin
> <hpa@zytor.com>; Thomas Gleixner <tglx@linutronix.de>
> Subject: Re: Regarding patch "mm: meminit: make __early_pfn_to_nid SMP-
> safe and introduce meminit_pfn_in_nid"
>
> On Wed, Jul 15, 2015 at 10:07:00AM -0400, Waiman Long wrote:
> > On 07/15/2015 09:40 AM, Mel Gorman wrote:
> > >On Wed, Jul 15, 2015 at 01:02:34AM +0000, Alex Ng (LIS) wrote:
> > >>Hello,
> > >>
> > >>Just wanted to bring to your attention an issue regarding commit
> 8a942fdea560d4ac0e9d9fabcd5201ad20e0c382<https://git.kernel.org/cgit/lin
> ux/kernel/git/torvalds/linux.git/commit/mm/page_alloc.c?id=8a942fdea560
> d4ac0e9d9fabcd5201ad20e0c382>. After this commit, calls to
> early_pfn_to_nid() are resulting in panic due to checking whether system is
> in early boot state. Such calls are happening after system has booted; and
> were working before this commit.
> > >>
> > >>One such case is in memory hot plug scenario; which results in below call
> trace. The function vmemmap_verify() calls early_pfn_to_nid() after system
> has booted. Should these calls be updated to use the SMP-safe version?
> > >>
> > >Yes but if there is more than one I missed then it's awkward. I missed the
> > >hotplug case when making early_pfn_to_nid() crash if it was not during
> > >boot. I suspect that hotplug just happened to work because it's under a
> > >giant mutex and cannot race.
> > >
> > >I didn't even build test this but can you try it? It makes early_pfn_to_nid()
> > >usable when the system is up and running by using a lock once races are
> > >possible.
> > >
> > >
> > >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > >index 94e2599830c2..91dd0c78a427 100644
> > >--- a/mm/page_alloc.c
> > >+++ b/mm/page_alloc.c
> > >@@ -982,21 +982,26 @@ static void __init
> __free_pages_boot_core(struct page *page,
> > >
> > > #if defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID) || \
> > > defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP)
> > >-/* Only safe to use early in boot when initialisation is single-threaded */
> > >+
> > > static struct mminit_pfnnid_cache early_pfnnid_cache __meminitdata;
> > >+DEFINE_SPINLOCK(early_pfn_lock);
> >
> > I think the lock should be static and preferably moved inside
> > early_pfn_to_nid() if it is the only function that uses it.
>
> Agreed. Lets hear first if it addresses Alex problem first and if so,
> I'll apply your feedback and put a changelog on it. There are now at least
> three follow-on patches that I'm waiting for feedback on.
>
> --
> Mel Gorman
> SUSE Labs
Tried this patch and it addresses the hotplug issue. It's no longer crashing.
Thanks,
Alex Ng
^ permalink raw reply [flat|nested] 2+ messages in thread