linux-kernel - Re: [PATCH v3 07/26] mm: drop CONFIG_HAVE_ARCH_NODEDATA

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240803115813.809f808f1afbe9f9feaae129@linux-foundation.org>
Date: Sat, 3 Aug 2024 11:58:13 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Mike Rapoport <rppt@...nel.org>, <linux-kernel@...r.kernel.org>,
 Alexander Gordeev <agordeev@...ux.ibm.com>, Andreas Larsson
 <andreas@...sler.com>, Arnd Bergmann <arnd@...db.de>, "Borislav Petkov"
 <bp@...en8.de>, Catalin Marinas <catalin.marinas@....com>, Christophe Leroy
 <christophe.leroy@...roup.eu>, Dan Williams <dan.j.williams@...el.com>,
 Dave Hansen <dave.hansen@...ux.intel.com>, David Hildenbrand
 <david@...hat.com>, "David S. Miller" <davem@...emloft.net>, Davidlohr
 Bueso <dave@...olabs.net>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, Heiko Carstens <hca@...ux.ibm.com>, Huacai
 Chen <chenhuacai@...nel.org>, Ingo Molnar <mingo@...hat.com>, Jiaxun Yang
 <jiaxun.yang@...goat.com>, "John Paul Adrian Glaubitz"
 <glaubitz@...sik.fu-berlin.de>, Jonathan Corbet <corbet@....net>, Michael
 Ellerman <mpe@...erman.id.au>, Palmer Dabbelt <palmer@...belt.com>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Rob Herring <robh@...nel.org>,
 Samuel Holland <samuel.holland@...ive.com>, Thomas Bogendoerfer
 <tsbogend@...ha.franken.de>, Thomas Gleixner <tglx@...utronix.de>,
 "Vasily Gorbik" <gor@...ux.ibm.com>, Will Deacon <will@...nel.org>, Zi Yan
 <ziy@...dia.com>, <devicetree@...r.kernel.org>,
 <linux-acpi@...r.kernel.org>, <linux-arch@...r.kernel.org>,
 <linux-arm-kernel@...ts.infradead.org>, <linux-cxl@...r.kernel.org>,
 <linux-doc@...r.kernel.org>, <linux-mips@...r.kernel.org>,
 <linux-mm@...ck.org>, <linux-riscv@...ts.infradead.org>,
 <linux-s390@...r.kernel.org>, <linux-sh@...r.kernel.org>,
 <linuxppc-dev@...ts.ozlabs.org>, <loongarch@...ts.linux.dev>,
 <nvdimm@...ts.linux.dev>, <sparclinux@...r.kernel.org>, <x86@...nel.org>
Subject: Re: [PATCH v3 07/26] mm: drop CONFIG_HAVE_ARCH_NODEDATA_EXTENSION

On Fri, 2 Aug 2024 10:49:22 +0100 Jonathan Cameron <Jonathan.Cameron@...wei.com> wrote:

> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -1838,11 +1838,10 @@ void __init free_area_init(unsigned long *max_zone_pfn)
> >  
> >  		if (!node_online(nid)) {
> >  			/* Allocator not initialized yet */
> > -			pgdat = arch_alloc_nodedata(nid);
> > +			pgdat = memblock_alloc(sizeof(*pgdat), SMP_CACHE_BYTES);
> >  			if (!pgdat)
> >  				panic("Cannot allocate %zuB for node %d.\n",
> >  				       sizeof(*pgdat), nid);
> > -			arch_refresh_nodedata(nid, pgdat);
> 
> This allocates pgdat but never sets node_data[nid] to it
> and promptly leaks it on the line below. 
> 
> Just to sanity check this I spun up a qemu machine with no memory
> initially present on some nodes and it went boom as you'd expect.
> 
> I tested with addition of
> 			NODE_DATA(nid) = pgdat;
> and it all seems to work as expected.

Thanks, I added that.  It blew up on x86_64 allnoconfig because
node_data[] (and hence NODE_DATA()) isn't an lvalue when CONFIG_NUMA=n.

I'll put some #ifdef CONFIG_NUMAs in there for now but

a) NODE_DATA() is upper-case. Implies "constant".  Shouldn't be assigned to.

b) NODE_DATA() should be non-lvalue when CONFIG_NUMA=y also.  But no,
   we insist on implementing things in cpp instead of in C.

c) In fact assigning to anything which ends in "()" is nuts.  Please
   clean up my tempfix.

c) Mike, generally I'm wondering if there's a bunch of code here
   which isn't needed on CONFIG_NUMA=n.  Please check all of this for
   unneeded bloatiness.