linux-kernel - RE: Regarding patch "mm: meminit: make __early_pfn_to_nid SMP-safe and introduce meminit_pfn_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <BY2PR0301MB1655CB50F3539863FF5DC252D89A0@BY2PR0301MB1655.namprd03.prod.outlook.com>
Date:	Wed, 15 Jul 2015 22:45:11 +0000
From:	"Alex Ng (LIS)" <alexng@...rosoft.com>
To:	Mel Gorman <mgorman@...e.de>, Waiman Long <waiman.long@...com>
CC:	Robin Holt <robinmholt@...il.com>, Nate Zimmer <nzimmer@....com>,
	"Dave Hansen" <dave.hansen@...el.com>,
	Scott Norton <scott.norton@...com>,
	"Luck, Tony" <tony.luck@...el.com>, Ingo Molnar <mingo@...e.hu>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Regarding patch "mm: meminit: make __early_pfn_to_nid SMP-safe
 and introduce meminit_pfn_in_nid"

> -----Original Message-----
> From: Mel Gorman [mailto:mgorman@...e.de]
> Sent: Wednesday, July 15, 2015 7:51 AM
> To: Waiman Long <waiman.long@...com>
> Cc: Alex Ng (LIS) <alexng@...rosoft.com>; Robin Holt
> <robinmholt@...il.com>; Nate Zimmer <nzimmer@....com>; Dave Hansen
> <dave.hansen@...el.com>; Scott Norton <scott.norton@...com>; Luck,
> Tony <tony.luck@...el.com>; Ingo Molnar <mingo@...e.hu>; H. Peter Anvin
> <hpa@...or.com>; Thomas Gleixner <tglx@...utronix.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/