From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.migadu.com (out-199.mta1.migadu.com [95.215.58.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 41B6A4398E4 for ; Fri, 14 Aug 2026 12:31:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786710699; cv=none; b=RYG3ZAurFDjC5Yn4mENTScu9Mrd/O3Vj+tcsyv/6rnfYvAAbxqebw3YaxjJB3PT00JLSHQw429TZ1i388L9F3SF7sza3aD0hh4AOF5bAEdM1uy1S+fAsf7OEsnJXWFIbXy0l9N5pZ+JwwpbvYiS/Z2c9TavXHd40jLbwx7j4cB0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786710699; c=relaxed/simple; bh=rnrRWzbRgf3HtNAFJ/URJsD3/yE/SVL7fKG74rTrmYI=; h=Mime-Version:Content-Type:Date:Message-Id:To:Cc:Subject:From: References:In-Reply-To; b=RLGel47I9CkBJ3JSRzBq8LbqThbRALEFeJCx+1mk6UbqptwzJl5tCBkliRpBcrBOy88I0mHw3bd6jdME3L9PWIm8RxOjgTxolEe9Ax9hNj823SqIkkhdIs6/6mLZALEa9CE9HaWoFOtXXR1YmBu4jGYwQWTFZZioK4T+eGgnxuM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=GZQDidMr; arc=none smtp.client-ip=95.215.58.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="GZQDidMr" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=rnrRWzbRgf3HtNAFJ/URJsD3/yE/SVL7fKG74rTrmYI=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1786710694; v=1; x=1787315494; b=GZQDidMrBpQDxONUJhyKGagb1b2Xv+6Z4/fru282pXRH5g671Bo9IbKcieI31XaW/p/SI5Xw +nxbKZZYTKmxy611GsRq7grESrKS46TD8EXcV3ZWetyWXa8LoqDE0SxAUTgEY5RgL+cG6+RlgVl Kl8U0Uh2wOy/Kml8fA9kqiO8= X-Envelope-To: linux-kernel@vger.kernel.org Received: from localhost (77.97.51.77) by smtp.migadu.com with ESMTPS id 2309f6b155db51af; Fri, 14 Aug 2026 12:31:24 +0000 X-Migadu-Flow: FLOW_OUT Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 14 Aug 2026 13:31:23 +0100 Message-Id: To: "Yosry Ahmed" Cc: "Borislav Petkov" , "Dave Hansen" , "Peter Zijlstra" , "Andrew Morton" , "David Hildenbrand" , "Vlastimil Babka" , "Mike Rapoport" , "Wei Xu" , "Johannes Weiner" , "Zi Yan" , "Lorenzo Stoakes" , , , , "Sumit Garg" , "Will Deacon" , , , "Itazuri, Takahiro" , "Andy Lutomirski" , "David Kaplan" , "Thomas Gleixner" , "Patrick Bellasi" , "Reiji Watanabe" , "Sean Christopherson" Subject: Re: [PATCH v3 21/26] mm/page_alloc: implement FREETYPE_UNMAPPED allocations From: "Brendan Jackman" X-Mailer: aerc 0.21.0 References: <20260726-page_alloc-unmapped-v3-0-6f5729aa9832@google.com> <20260726-page_alloc-unmapped-v3-21-6f5729aa9832@google.com> In-Reply-To: On Wed Aug 5, 2026 at 12:41 AM BST, Yosry Ahmed wrote: > On Sun, Jul 26, 2026 at 10:22:54PM +0000, Brendan Jackman wrote: >> Currently FREETYPE_UNMAPPED allocs will always fail because, although th= e >> lists exist to hold them, there is no way to actually create an unmapped >> page block. This commit adds one, and also the logic to map it back >> again when that's needed. >>=20 >> Doing this at pageblock granularity ensures that the pageblock flags can >> be used to infer which freetype a page belongs to. It also provides nice >> batching of TLB flushes, and also avoids creating too much unnecessary >> TLB fragmentation in the physmap. >>=20 >> There are some functional requirements for flipping a block: >>=20 >> - Unmapping requires a TLB shootdown, meaning IRQs must be enabled. >>=20 >> - Updating the pagetables might require allocating a pagetable to break >> down a huge page. This would deadlock if the zone lock was held. > > We also need to zero unmapped/sensitive pages before mapping them again, > but seems like the current approach is to differ this to the caller, > which makes sense. The only annoying part is that > want_init_on_{free/alloc}() now silently skip the zeroing for those > pages. This does look annoying but I think it's actually all fine and correct. - It's forbidden to allocate with ALLOC_UNMAPPED and __GFP_ZERO, so the __GFP_ZERO part of want_init_on_alloc() is still correct under ALLOC_UNMAPPED. - I think init_on_alloc's job is kernel hardening, i.e. it's a roadbump for kernel exploit authors. If you have a bug that lets you read some uninitialised memory via the kernel then init_on_alloc means you see zeroes. But those vulns are automatically mitigated by ALLOC_UNMAPPED anyway so this is fine. - There is a very strong ambient rule that you must ensure memory is zeroed before mapping it into userspace/VMs, but this is totally separate from init_on_alloc. And it's fine for filesystems or whatever to implement this however they want, __GFP_ZERO is just one way they can do it. This rule is totally separate from init_on_alloc, if you break it you are immediately creating a vulnerability instead of just a second- order weakness. > We should document somewhere that the users of ALLOC_UNMAPPED are > responsible for zeroing memory before freeing it? So, no I don't think we need to document that, we just need to make sure that the __GFP_ZERO restriction is clear. > The current users currently always zero the pages on allocation as well, > I am not sure if this should also be a general requirement, or perhaps > only if want_init_on_alloc() is set? >> This makes allocations that need to change sensitivity _somewhat_ > > s/sensitivity/direct mapping status (or sth)? Oops thanks. >> similar to those that need to fallback to a different migratetype. But, >> the locking requirements mean that this can't just be squashed into the >> existing "fallback" allocator logic, instead a new allocator path just >> for this purpose is needed. >>=20 >> The new path is assumed to be much cheaper than the really heavyweight >> stuff like compaction and reclaim. But at present it is treated as less >> desirable than the mobility-related "fallback" and "stealing" logic. >> This might turn out to need revision (in particular, maybe it's a >> problem that __rmqueue_steal(), which causes fragmentation, happens >> before __rmqueue_direct_map()), but that should be treated as a subseque= nt >> optimisation project. >>=20 >> Adding alloc_flags to gfp_freetype() requires moving it to >> mm/page_alloc.h so it can refer to ALLOC_UNMAPPED. It was already only >> used in internal mm code. >>=20 >> Now that unmapped pageblocks actually exist, exclude them from >> migration. Migrating unmapped pages via the mermap should be possible >> but that's something to be added later when needed. >>=20 >> Signed-off-by: Brendan Jackman > [..] >> @@ -3400,6 +3426,127 @@ static inline void zone_statistics(struct zone *= preferred_zone, struct zone *z, >> #endif >> } >> =20 >> +#ifdef CONFIG_PAGE_ALLOC_UNMAPPED >> +/* Try to allocate a page by mapping/unmapping a block from the direct = map. */ >> +static inline struct page * >> +__rmqueue_direct_map(struct zone *zone, unsigned int request_order, >> + unsigned int alloc_flags, freetype_t freetype) >> +{ >> + unsigned int ft_flags_other =3D freetype_flags(freetype) ^ FREETYPE_UN= MAPPED; >> + freetype_t ft_other =3D migrate_to_freetype(free_to_migratetype(freety= pe), >> + ft_flags_other); >> + bool want_mapped =3D !(freetype_flags(freetype) & FREETYPE_UNMAPPED); >> + enum rmqueue_mode rmqm =3D RMQUEUE_NORMAL; >> + unsigned long irq_flags; >> + int nr_pageblocks, nr_freed; >> + struct page *page; >> + int alloc_order; >> + int err; >> + >> + if (freetype_idx(ft_other) < 0) >> + return NULL; >> + >> + /* >> + * Might need a TLB shootdown. Even if IRQs are on this isn't >> + * safe if the caller holds a lock (in case the other CPUs need that >> + * lock to handle the shootdown IPI). >> + */ >> + if (alloc_flags & ALLOC_NOBLOCK) >> + return NULL; > > Should we only check this if !want_mapped? IIUC we only need a TLB > shootdown when unmapping. Hm, I don't think we wanna zero a pageblock with IRQs off. The comment should reflect that though. >> + >> + if (!can_set_direct_map() || alloc_flags & ALLOC_NOLOCK) >> + return NULL; >> + >> + lockdep_assert(!irqs_disabled() || unlikely(early_boot_irqs_disabled))= ; >> + >> + /* >> + * Need to [un]map a whole pageblock (otherwise it might require >> + * allocating pagetables). First allocate it. >> + */ >> + alloc_order =3D max(request_order, pageblock_order); >> + nr_pageblocks =3D 1 << (alloc_order - pageblock_order); >> + spin_lock_irqsave(&zone->lock, irq_flags); >> + /* First try a block that already has the right migratetype. */ >> + page =3D __rmqueue(zone, alloc_order, ft_other, alloc_flags, &rmqm); > > IIUC, this is called after __rmqueue() will have already failed in the > caller with request_order (a potentially smaller order), so why are we > trying this again here? The __rmqueue() that failed was with the opposite value of FREETYPE_UNMAPPED. >> + if (!page) { >> + /* Fallback to changing a block's migratetype. */ >> + rmqm =3D RMQUEUE_CLAIM; >> + page =3D __rmqueue(zone, alloc_order, ft_other, alloc_flags, &rmqm); >> + } >> + spin_unlock_irqrestore(&zone->lock, irq_flags); >> + if (!page) >> + return NULL; >> + >> + /* >> + * Now that IRQs are on it's safe to do a TLB shootdown, and now that = we >> + * released the zone lock it's possible to allocate a pagetable if >> + * needed to split up a huge page. >> + * >> + * Note that modifying the direct map may need to allocate pagetables. >> + * What about unbounded recursion? Here are the assumptions that make = it >> + * safe: >> + * >> + * - The direct map starts out fully mapped at boot. (This is not real= ly >> + * an "assumption" as it's in direct control of page_alloc.c). >> + * >> + * - Once pages in the direct map are broken down, they are not >> + * re-aggregated into larger pages again. >> + * >> + * - Pagetables are never allocated with ALLOC_UNMAPPED. >> + * >> + * Under these assumptions, a pagetable might need to be allocated whi= le >> + * _unmapping_ stuff from the direct map during an ALLOC_UNMAPPED >> + * allocation. But, the allocation of that pagetable never requires >> + * allocating a further pagetable. >> + */ >> + err =3D set_direct_map_valid_noflush(page, >> + nr_pageblocks << pageblock_order, want_mapped); >> + if (err =3D=3D -ENOMEM || WARN_ONCE(err, "err=3D%d\n", err)) { >> + set_direct_map_valid_noflush(page, >> + nr_pageblocks << pageblock_order, !want_mapped); >> + spin_lock_irqsave(&zone->lock, irq_flags); >> + /* Important: free using _old_ freetype. */ >> + __free_one_page(page, page_to_pfn(page), zone, >> + alloc_order, ft_other, FPI_SKIP_REPORT_NOTIFY); >> + spin_unlock_irqrestore(&zone->lock, irq_flags); >> + return NULL; >> + } >> + >> + if (want_mapped) { >> + /* Exposing formerly-protected data; scrub it. */ >> + clear_highpages_kasan_tagged(page, nr_pageblocks << pageblock_order); > > Shouldn't all unmapped memory be zeroed on free? If we solidify this > assumption we can probably drop this here (and maybe replace it with an > assertion)? Hm, that's true. I guess just a question of whether we do indeed want to make that a hard rule for ALLOC_UNMAPPED. I'm not too sure about that, I only really added that unconditional zeroing because I wanted to keep the prior behaviour of secretmem/GUEST_MEMFD_FLAG_NO_DIRECT_MAP, but maybe it's undesirable to place such a big burden on ALLOC_UNMAPPED? It would be nice to be able to easily expand this into more direct-map-killing behaviour... >> + } else { >> + unsigned long start =3D (unsigned long)page_address(page); >> + unsigned long end =3D start + (nr_pageblocks << (pageblock_order + PA= GE_SHIFT)); >> + >> + flush_tlb_kernel_range(start, end); >> + } >> + >> + for (int i =3D 0; i < nr_pageblocks; i++) { >> + struct page *block_page =3D page + (pageblock_nr_pages * i); >> + >> + set_pageblock_freetype_flags(block_page, freetype_flags(freetype)); >> + } >> + >> + if (request_order >=3D alloc_order) >> + return page; >> + >> + /* Free any remaining pages in the block. */ >> + spin_lock_irqsave(&zone->lock, irq_flags); >> + nr_freed =3D expand(zone, page, request_order, alloc_order, freetype); >> + account_freepages(zone, nr_freed, free_to_migratetype(freetype)); >> + spin_unlock_irqrestore(&zone->lock, irq_flags); >> + >> + return page; >> +} >> +#else /* CONFIG_PAGE_ALLOC_UNMAPPED */ >> +static inline struct page *__rmqueue_direct_map(struct zone *zone, unsi= gned int request_order, >> + unsigned int alloc_flags, freetype_t freetype) >> +{ >> + return NULL; >> +} >> +#endif /* CONFIG_PAGE_ALLOC_UNMAPPED */ >> + >> static __always_inline >> struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zo= ne, >> unsigned int order, unsigned int alloc_flags, >> @@ -3433,13 +3580,15 @@ struct page *rmqueue_buddy(struct zone *preferre= d_zone, struct zone *zone, >> */ >> if (!page && (alloc_flags & (ALLOC_OOM|ALLOC_HARDER))) >> page =3D __rmqueue_smallest(zone, order, ft_high); >> - >> - if (!page) { >> - spin_unlock_irqrestore(&zone->lock, flags); >> - return NULL; >> - } >> } >> spin_unlock_irqrestore(&zone->lock, flags); >> + >> + /* Try changing direct map, now we've released the zone lock */ >> + if (!page) >> + page =3D __rmqueue_direct_map(zone, order, alloc_flags, freetype); >> + if (!page) >> + return NULL; >> + >> } while (check_new_pages(page, order)); >> =20 >> /* >> @@ -3660,6 +3809,8 @@ static void reserve_highatomic_pageblock(struct pa= ge *page, int order, >> return; >> =20 >> ft_high =3D freetype_with_migrate(ft, MIGRATE_HIGHATOMIC); >> + if (freetype_idx(ft_high) < 0) >> + return; > > Does this belong in "mm/page_alloc: add support for freetypes with no > freelist"? > >> if (order < pageblock_order) { >> if (move_freepages_block(zone, page, ft, ft_high) =3D=3D -1) >> return; >> @@ -3975,13 +4126,15 @@ alloc_flags_nofragment(struct zone *zone, gfp_t = gfp_mask) >> } >> =20 >> /* Must be called after current_gfp_context() which can change gfp_mask= */ >> -static inline unsigned int alloc_flags_cma(gfp_t gfp_mask) >> +static inline unsigned int alloc_flags_cma(gfp_t gfp_mask, unsigned int= alloc_flags) >> { >> #ifdef CONFIG_CMA >> - if (free_to_migratetype(gfp_freetype(gfp_mask)) =3D=3D MIGRATE_MOVABLE= ) >> - return ALLOC_CMA; >> + if (free_to_migratetype(gfp_freetype(gfp_mask, alloc_flags)) =3D=3D MI= GRATE_MOVABLE) >> + alloc_flags |=3D ALLOC_CMA; >> #endif >> - return ALLOC_DEFAULT; >> + alloc_flags |=3D ALLOC_DEFAULT; >> + >> + return alloc_flags; >> } >> =20 >> /* >> @@ -4770,7 +4923,7 @@ alloc_flags_slowpath(gfp_t gfp_mask, unsigned int = order) >> } else if (unlikely(rt_or_dl_task(current)) && in_task()) >> alloc_flags |=3D ALLOC_MIN_RESERVE; >> =20 >> - alloc_flags |=3D alloc_flags_cma(gfp_mask); >> + alloc_flags =3D alloc_flags_cma(gfp_mask, alloc_flags); >> =20 >> if (defrag_mode) >> alloc_flags |=3D ALLOC_NOFRAGMENT; >> @@ -5085,7 +5238,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned in= t order, >> =20 >> reserve_flags =3D __gfp_pfmemalloc_flags(gfp_mask); >> if (reserve_flags) >> - alloc_flags =3D alloc_flags_cma(gfp_mask) | reserve_flags | >> + alloc_flags =3D alloc_flags_cma(gfp_mask, alloc_flags) | reserve_flag= s | >> ac->alloc_flags | (alloc_flags & ALLOC_KSWAPD); > > Should we pass in ac->alloc_flags here to maintain equivalent > functionality, maybe this: > > alloc_flags =3D alloc_flags_cma(gfp_mask, ac->alloc_flags) | > reserve_flags | (alloc_flags & ALLOC_KSWAPD); Um, what's the difference? >> =20 >> /* >> @@ -5307,7 +5460,11 @@ static inline bool prepare_alloc_pages(gfp_t gfp_= mask, unsigned int order, >> ac->highest_zoneidx =3D gfp_zone(gfp_mask); >> ac->zonelist =3D node_zonelist(preferred_nid, gfp_mask); >> ac->nodemask =3D nodemask; >> - ac->freetype =3D gfp_freetype(gfp_mask); >> + ac->freetype =3D gfp_freetype(gfp_mask, *alloc_flags); >> + >> + /* Not implemented yet. */ >> + if (freetype_flags(ac->freetype) & FREETYPE_UNMAPPED && gfp_mask & __G= FP_ZERO) > > Nit: Add more parentheses for readability? Yup sounds good.