| Message ID | 20210329222555.3077928-1-slyfox@gentoo.org |
|---|---|
| State | Accepted |
| Commit | 9df65f522536719682bccd24245ff94db956256c |
| Headers | show |
| Series | [v3] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1 | expand |
On 30.03.21 00:25, Sergei Trofimovich wrote: > On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1 > implies page_poison=on: > > if (page_poisoning_enabled() || > (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && > debug_pagealloc_enabled())) > static_branch_enable(&_page_poisoning_enabled); > > page_poison=on needs to override init_on_free=1. > > Before the change it did not work as expected for the following case: > - have PAGE_POISONING=y > - have page_poison unset > - have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64) > - have init_on_free=1 > - have debug_pagealloc=1 > > That way we get both keys enabled: > - static_branch_enable(&init_on_free); > - static_branch_enable(&_page_poisoning_enabled); > > which leads to poisoned pages returned for __GFP_ZERO pages. > > After the change we execute only: > - static_branch_enable(&_page_poisoning_enabled); > and ignore init_on_free=1. > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > Fixes: 8db26a3d4735 ("mm, page_poison: use static key more efficiently") > Cc: <stable@vger.kernel.org> > CC: Andrew Morton <akpm@linux-foundation.org> > CC: linux-mm@kvack.org > CC: David Hildenbrand <david@redhat.com> > CC: Andrey Konovalov <andreyknvl@gmail.com> > Link: https://lkml.org/lkml/2021/3/26/443 > > Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> > --- > Change since v2: > - Added 'Fixes:' and 'CC: stable@' suggested by Vlastimil and David > - Renamed local variable to 'page_poisoning_requested' for > consistency suggested by David > - Simplified initialization of page_poisoning_requested suggested > by David > - Added 'Acked-by: Vlastimil' > > mm/page_alloc.c | 30 +++++++++++++++++------------- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index cfc72873961d..4bb3cdfc47f8 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone, struct page *page, > */ > void init_mem_debugging_and_hardening(void) > { > + bool page_poisoning_requested = false; > + > +#ifdef CONFIG_PAGE_POISONING > + /* > + * Page poisoning is debug page alloc for some arches. If > + * either of those options are enabled, enable poisoning. > + */ > + if (page_poisoning_enabled() || > + (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && > + debug_pagealloc_enabled())) { > + static_branch_enable(&_page_poisoning_enabled); > + page_poisoning_requested = true; > + } > +#endif > + > if (_init_on_alloc_enabled_early) { > - if (page_poisoning_enabled()) > + if (page_poisoning_requested) > pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, " > "will take precedence over init_on_alloc\n"); > else > static_branch_enable(&init_on_alloc); > } > if (_init_on_free_enabled_early) { > - if (page_poisoning_enabled()) > + if (page_poisoning_requested) > pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, " > "will take precedence over init_on_free\n"); > else > static_branch_enable(&init_on_free); > } > > -#ifdef CONFIG_PAGE_POISONING > - /* > - * Page poisoning is debug page alloc for some arches. If > - * either of those options are enabled, enable poisoning. > - */ > - if (page_poisoning_enabled() || > - (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && > - debug_pagealloc_enabled())) > - static_branch_enable(&_page_poisoning_enabled); > -#endif > - > #ifdef CONFIG_DEBUG_PAGEALLOC > if (!debug_pagealloc_enabled()) > return; > Thanks! Reviewed-by: David Hildenbrand <david@redhat.com> -- Thanks, David / dhildenb
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index cfc72873961d..4bb3cdfc47f8 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone, struct page *page, */ void init_mem_debugging_and_hardening(void) { + bool page_poisoning_requested = false; + +#ifdef CONFIG_PAGE_POISONING + /* + * Page poisoning is debug page alloc for some arches. If + * either of those options are enabled, enable poisoning. + */ + if (page_poisoning_enabled() || + (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && + debug_pagealloc_enabled())) { + static_branch_enable(&_page_poisoning_enabled); + page_poisoning_requested = true; + } +#endif + if (_init_on_alloc_enabled_early) { - if (page_poisoning_enabled()) + if (page_poisoning_requested) pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, " "will take precedence over init_on_alloc\n"); else static_branch_enable(&init_on_alloc); } if (_init_on_free_enabled_early) { - if (page_poisoning_enabled()) + if (page_poisoning_requested) pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, " "will take precedence over init_on_free\n"); else static_branch_enable(&init_on_free); } -#ifdef CONFIG_PAGE_POISONING - /* - * Page poisoning is debug page alloc for some arches. If - * either of those options are enabled, enable poisoning. - */ - if (page_poisoning_enabled() || - (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && - debug_pagealloc_enabled())) - static_branch_enable(&_page_poisoning_enabled); -#endif - #ifdef CONFIG_DEBUG_PAGEALLOC if (!debug_pagealloc_enabled()) return;
On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1 implies page_poison=on: if (page_poisoning_enabled() || (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) && debug_pagealloc_enabled())) static_branch_enable(&_page_poisoning_enabled); page_poison=on needs to override init_on_free=1. Before the change it did not work as expected for the following case: - have PAGE_POISONING=y - have page_poison unset - have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64) - have init_on_free=1 - have debug_pagealloc=1 That way we get both keys enabled: - static_branch_enable(&init_on_free); - static_branch_enable(&_page_poisoning_enabled); which leads to poisoned pages returned for __GFP_ZERO pages. After the change we execute only: - static_branch_enable(&_page_poisoning_enabled); and ignore init_on_free=1. Acked-by: Vlastimil Babka <vbabka@suse.cz> Fixes: 8db26a3d4735 ("mm, page_poison: use static key more efficiently") Cc: <stable@vger.kernel.org> CC: Andrew Morton <akpm@linux-foundation.org> CC: linux-mm@kvack.org CC: David Hildenbrand <david@redhat.com> CC: Andrey Konovalov <andreyknvl@gmail.com> Link: https://lkml.org/lkml/2021/3/26/443 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> --- Change since v2: - Added 'Fixes:' and 'CC: stable@' suggested by Vlastimil and David - Renamed local variable to 'page_poisoning_requested' for consistency suggested by David - Simplified initialization of page_poisoning_requested suggested by David - Added 'Acked-by: Vlastimil' mm/page_alloc.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)