From: Vlastimil Babka <vbabka@suse.cz>
To: David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
Alexander Potapenko <glider@google.com>,
Michal Hocko <mhocko@suse.com>,
Mike Kravetz <mike.kravetz@oracle.com>,
Mike Rapoport <rppt@linux.ibm.com>,
Oscar Salvador <osalvador@suse.de>,
Kees Cook <keescook@chromium.org>,
Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH v2] mm/page_alloc: clear all pages in post_alloc_hook() with init_on_alloc=1
Date: Wed, 25 Nov 2020 17:16:03 +0100 [thread overview]
Message-ID: <645f62ce-18fc-1586-7cd8-737924d919ba@suse.cz> (raw)
In-Reply-To: <20201120180452.19071-1-david@redhat.com>
On 11/20/20 7:04 PM, 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.
>
> To achieve that, let's move clearing into post_alloc_hook(). This will not
> only affect alloc_contig_pages() allocations but also any pages used as
> migration target in compaction code via compaction_alloc().
>
> While this sounds sub-optimal, it's the very same handling as when
> allocating migration targets via alloc_migration_target() - pages will
> get properly cleared with init_on_free=1. In case we ever want to optimize
> migration in that regard, we should tackle all such migration users - if we
> believe migration code can be fully trusted.
>
> 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.
> - Pages isolated for compaction will be cleared
>
> 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>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>
> This is the follow-up of:
> "[PATCH v1] mm/page_alloc: clear pages in alloc_contig_pages() with
> init_on_alloc=1 or __GFP_ZERO"
>
> v1 -> v2:
> - Let's clear anything that leaves the buddy, also affecting compaction.
> - Don't implement __GFP_ZERO support for now
>
> ---
> mm/page_alloc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index eaa227a479e4..108b81c0dfa8 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2275,6 +2275,9 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
> kasan_alloc_pages(page, order);
> kernel_poison_pages(page, 1 << order, 1);
> set_page_owner(page, order, gfp_flags);
> +
> + if (!free_pages_prezeroed() && want_init_on_alloc(gfp_flags))
> + kernel_init_free_pages(page, 1 << order);
> }
>
> static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
> @@ -2282,9 +2285,6 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
> {
> post_alloc_hook(page, order, gfp_flags);
>
> - if (!free_pages_prezeroed() && want_init_on_alloc(gfp_flags))
> - kernel_init_free_pages(page, 1 << order);
> -
> if (order && (gfp_flags & __GFP_COMP))
> prep_compound_page(page, order);
>
>
next prev parent reply other threads:[~2020-11-25 16:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-20 18:04 [PATCH v2] mm/page_alloc: clear all pages in post_alloc_hook() with init_on_alloc=1 David Hildenbrand
2020-11-25 16:16 ` Vlastimil Babka [this message]
2020-11-27 12:23 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=645f62ce-18fc-1586-7cd8-737924d919ba@suse.cz \
--to=vbabka@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=david@redhat.com \
--cc=glider@google.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=mike.kravetz@oracle.com \
--cc=mpe@ellerman.id.au \
--cc=osalvador@suse.de \
--cc=rppt@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).