linux-kernel - Re: [PATCH v4 10/25] mm/mm_init: Move p2pdma page refcount initialisation to p2pdma

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: <bk6cmiubwvor6gevit3wgl4k66xxpfcv6swmfrtvxnjnuojqkx@yow3pmyuxozw>
Date: Thu, 19 Dec 2024 09:49:18 +1100
From: Alistair Popple <apopple@...dia.com>
To: David Hildenbrand <david@...hat.com>
Cc: akpm@...ux-foundation.org, dan.j.williams@...el.com, 
	linux-mm@...ck.org, lina@...hilina.net, zhang.lyra@...il.com, 
	gerald.schaefer@...ux.ibm.com, vishal.l.verma@...el.com, dave.jiang@...el.com, 
	logang@...tatee.com, bhelgaas@...gle.com, jack@...e.cz, jgg@...pe.ca, 
	catalin.marinas@....com, will@...nel.org, mpe@...erman.id.au, npiggin@...il.com, 
	dave.hansen@...ux.intel.com, ira.weiny@...el.com, willy@...radead.org, djwong@...nel.org, 
	tytso@....edu, linmiaohe@...wei.com, peterx@...hat.com, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org, nvdimm@...ts.linux.dev, 
	linux-cxl@...r.kernel.org, linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org, 
	linux-xfs@...r.kernel.org, jhubbard@...dia.com, hch@....de, david@...morbit.com
Subject: Re: [PATCH v4 10/25] mm/mm_init: Move p2pdma page refcount
 initialisation to p2pdma

On Tue, Dec 17, 2024 at 11:14:42PM +0100, David Hildenbrand wrote:
> On 17.12.24 06:12, Alistair Popple wrote:
> > Currently ZONE_DEVICE page reference counts are initialised by core
> > memory management code in __init_zone_device_page() as part of the
> > memremap() call which driver modules make to obtain ZONE_DEVICE
> > pages. This initialises page refcounts to 1 before returning them to
> > the driver.
> > 
> > This was presumably done because it drivers had a reference of sorts
> > on the page. It also ensured the page could always be mapped with
> > vm_insert_page() for example and would never get freed (ie. have a
> > zero refcount), freeing drivers of manipulating page reference counts.
> 
> It probably dates back to copying that code from other zone-init code where
> we
> (a) Treat all available-at-boot memory as allocated before we release it to
> the buddy
> (b) Treat all hotplugged memory as allocated until we release it to the
> buddy
 
Argh, thanks for the background.

> As a side note, I'm working on converting (b) -- PageOffline pages -- to
> have a refcount of 0 ("frozen").

[...]

> > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > index 24b68b4..f021e63 100644
> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -1017,12 +1017,26 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
> >   	}
> >   	/*
> > -	 * ZONE_DEVICE pages are released directly to the driver page allocator
> > -	 * which will set the page count to 1 when allocating the page.
> > +	 * ZONE_DEVICE pages other than MEMORY_TYPE_GENERIC and
> > +	 * MEMORY_TYPE_FS_DAX pages are released directly to the driver page
> > +	 * allocator which will set the page count to 1 when allocating the
> > +	 * page.
> > +	 *
> > +	 * MEMORY_TYPE_GENERIC and MEMORY_TYPE_FS_DAX pages automatically have
> > +	 * their refcount reset to one whenever they are freed (ie. after
> > +	 * their refcount drops to 0).
> >   	 */
> > -	if (pgmap->type == MEMORY_DEVICE_PRIVATE ||
> > -	    pgmap->type == MEMORY_DEVICE_COHERENT)
> > +	switch (pgmap->type) {
> > +	case MEMORY_DEVICE_PRIVATE:
> > +	case MEMORY_DEVICE_COHERENT:
> > +	case MEMORY_DEVICE_PCI_P2PDMA:
> >   		set_page_count(page, 0);
> > +		break;
> > +
> > +	case MEMORY_DEVICE_FS_DAX:
> > +	case MEMORY_DEVICE_GENERIC:
> > +		break;
> > +	}
> >   }
> >   /*
> 
> 
> But that's a bit weird: we call __init_single_page()->init_page_count() to
> initialize it to 1, to then set it back to 0.
> 
> 
> Maybe we can just pass to __init_single_page() the refcount we want to have
> directly? Can be a patch on top of course.

Once the dust settles on this series we won't need the pgmap->type check at
all because all ZONE_DEVICE pages will get an initial count of 0. I have some
follow up clean-ups for after this series is applied (particularly with regards
to pgmap refcounts), so if it's ok I'd rather do this as a follow-up.

> Apart from that
> 
> Acked-by: David Hildenbrand <david@...hat.com>
> 
> -- 
> Cheers,
> 
> David / dhildenb
>