> -----Original Message----- > From: Mel Gorman [mailto:[email protected]] > Sent: Wednesday, July 15, 2015 7:51 AM > To: Waiman Long <[email protected]> > Cc: Alex Ng (LIS) <[email protected]>; Robin Holt > <[email protected]>; Nate Zimmer <[email protected]>; Dave Hansen > <[email protected]>; Scott Norton <[email protected]>; Luck, > Tony <[email protected]>; Ingo Molnar <[email protected]>; H. Peter Anvin > <[email protected]>; Thomas Gleixner <[email protected]> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
