Messages in this thread | | | Subject | Re: [PATCH v1] mm/page_alloc: clear pages in alloc_contig_pages() with init_on_alloc=1 or __GFP_ZERO | From | David Hildenbrand <> | Date | Wed, 11 Nov 2020 11:06:45 +0100 |
| |
On 11.11.20 10:59, Mike Rapoport wrote: > On Tue, Nov 10, 2020 at 08:32:40PM +0100, David Hildenbrand wrote: >> commit 6471384af2a6 ("mm: security: introduce init_on_alloc=1 and >> init_on_free=1 boot options") resulted with init_on_alloc=1 in all pages >> leaving the buddy via alloc_pages() and friends to be >> initialized/cleared/zeroed on allocation. >> >> However, the same logic is currently not applied to >> alloc_contig_pages(): allocated pages leaving the buddy aren't cleared >> with init_on_alloc=1 and init_on_free=0. Let's also properly clear >> pages on that allocation path and add support for __GFP_ZERO. >> >> With this change, we will see double clearing of pages in some >> cases. One example are gigantic pages (either allocated via CMA, or >> allocated dynamically via alloc_contig_pages()) - which is the right >> thing to do (and to be optimized outside of the buddy in the callers) as >> discussed in: >> https://lkml.kernel.org/r/20201019182853.7467-1-gpiccoli@canonical.com >> >> This change implies that with init_on_alloc=1 >> - All CMA allocations will be cleared >> - Gigantic pages allocated via alloc_contig_pages() will be cleared >> - virtio-mem memory to be unplugged will be cleared. While this is >> suboptimal, it's similar to memory balloon drivers handling, where >> all pages to be inflated will get cleared as well. >> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Alexander Potapenko <glider@google.com> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Mike Kravetz <mike.kravetz@oracle.com> >> Cc: Vlastimil Babka <vbabka@suse.cz> >> Cc: Mike Rapoport <rppt@linux.ibm.com> >> Cc: Oscar Salvador <osalvador@suse.de> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> mm/page_alloc.c | 24 +++++++++++++++++++++--- >> 1 file changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index eed4f4075b3c..0361b119b74e 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -8453,6 +8453,19 @@ static int __alloc_contig_migrate_range(struct compact_control *cc, >> return 0; >> } >> >> +static void __alloc_contig_clear_range(unsigned long start_pfn, >> + unsigned long end_pfn) > > Maybe clear_contig_range() ?
I chose the naming to match "__alloc_contig_migrate_range", but I agree that your version sounds better.
> >> +{ >> + unsigned long pfn; >> + >> + for (pfn = start_pfn; pfn < end_pfn; pfn += MAX_ORDER_NR_PAGES) { >> + cond_resched(); >> + kernel_init_free_pages(pfn_to_page(pfn), >> + min_t(unsigned long, end_pfn - pfn, >> + MAX_ORDER_NR_PAGES)); >> + } >> +} >> + >> /** >> * alloc_contig_range() -- tries to allocate given range of pages >> * @start: start PFN to allocate >> @@ -8461,7 +8474,8 @@ static int __alloc_contig_migrate_range(struct compact_control *cc, >> * #MIGRATE_MOVABLE or #MIGRATE_CMA). All pageblocks >> * in range must have the same migratetype and it must >> * be either of the two. >> - * @gfp_mask: GFP mask to use during compaction >> + * @gfp_mask: GFP mask to use during compaction. __GFP_ZERO clears allocated >> + * pages. > > "__GFP_ZERO is not passed to compaction but rather clears allocated pages"
Bought! Thanks :)
-- Thanks,
David / dhildenb
| |