Address issues slowing memory init
Subsystems (5)
v2 cover Address issues slowing memory init
This patch series is meant to address some issues I consider to be low-hanging fruit in regards to memory initialization optimization.
With these two changes I am able to cut the hot-plug memory initialization times in my environment in half.
v2: Added comments about why we are using __SetPageReserved
Added new config and updated approach used for page init poisoning
---
Alexander Duyck (2):
mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
mm: Create non-atomic version of SetPageReserved for init use
include/linux/page-flags.h | 9 +++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/page_alloc.c | 13 +++++++++++-- mm/sparse.c | 4 +--- 5 files changed, 37 insertions(+), 8 deletions(-)
--
PATCH v2 1/2 mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the PAGE_POISON_PATTERN value. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.
In order to work around the issue I had to disable CONFIG_DEBUG_VM and then the boot time returned to something much more reasonable as the arch_add_memory call completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.
Instead of keeping the value in CONFIG_DEBUG_VM I am adding a new CONFIG value called CONFIG_DEBUG_VM_PAGE_INIT_POISON that will control the page poisoning independent of the CONFIG_DEBUG_VM option.
--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)
include/linux/page-flags.h +8 -0
@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits:
@@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages *
lib/Kconfig.debug +14 -0
@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISON + bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory. + + If unsure, say Y. + config ARCH_HAS_DEBUG_VIRTUAL bool
mm/memblock.c +2 -3
@@ -1444,10 +1444,9 @@ void * __init memblock_virt_alloc_try_nid_raw( ptr =memblock_virt_alloc_internal (size, align, min_addr, max_addr, nid); -#ifdef CONFIG_DEBUG_VM if (ptr && size >0 ) -memset (ptr,PAGE_POISON_PATTERN , size); -#endif +page_init_poison (ptr, size); +return ptr; }
mm/sparse.c +1 -3
@@ -696,13 +696,11 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, goto out; } -#ifdef CONFIG_DEBUG_VM /* * Poison uninitialized struct pages in order to catch invalid flags * combinations. */ -memset (memmap,PAGE_POISON_PATTERN ,sizeof (struct page ) *PAGES_PER_SECTION ); -#endif +page_init_poison (memmap,sizeof (struct page ) *PAGES_PER_SECTION );section_mark_present (ms);sparse_init_one_section (ms, section_nr, memmap, usemap);
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 59 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the
PAGE_POISON_PATTERNvalue. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.In order to work around the issue I had to disable
CONFIG_DEBUG_VMand then the boot time returned to something much more reasonable as thearch_add_memorycall completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.Signed-off-by Alexander Duyck <[email protected]>--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)include/linux/page-flags.h +8
-0@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits: @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages * lib/Kconfig.debug +1
-0@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISON
lib/Kconfig.debug config DEBUG_VM_PGFLAGS · line 650
Quoted 10 lines
lib/Kconfig.debug +10
-0+ bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory.
What happens when DEBUG_VM_PGFLAGS = y andDEBUG_VM_PAGE_INIT_POISON = n ?
We are testing for pattern that was not set?
I think DEBUG_VM_PAGE_INIT_POISON must depend on DEBUG_VM_PGFLAGS instead.
Looks good otherwise.
Thank you,
Pavel
Quoted 43 lines
lib/Kconfig.debug +3
-0+ + If unsure, say Y. + config ARCH_HAS_DEBUG_VIRTUAL boolmm/memblock.c +2
-3@@ -1444,10 +1444,9 @@ void * __init memblock_virt_alloc_try_nid_raw( ptr =memblock_virt_alloc_internal (size, align, min_addr, max_addr, nid); -#ifdef CONFIG_DEBUG_VM if (ptr && size >0 ) -memset (ptr,PAGE_POISON_PATTERN , size); -#endif +page_init_poison (ptr, size); +return ptr; }mm/sparse.c +1
-3@@ -696,13 +696,11 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, goto out; } -#ifdef CONFIG_DEBUG_VM /* * Poison uninitialized struct pages in order to catch invalid flags * combinations. */ -memset (memmap,PAGE_POISON_PATTERN ,sizeof (struct page ) *PAGES_PER_SECTION ); -#endif +page_init_poison (memmap,sizeof (struct page ) *PAGES_PER_SECTION );section_mark_present (ms);sparse_init_one_section (ms, section_nr, memmap, usemap);
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 77 lines · Pasha Tatashin
Quoted 59 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the
PAGE_POISON_PATTERNvalue. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.In order to work around the issue I had to disable
CONFIG_DEBUG_VMand then the boot time returned to something much more reasonable as thearch_add_memorycall completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.Signed-off-by Alexander Duyck <[email protected]>--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)include/linux/page-flags.h +8
-0@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits: @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages * lib/Kconfig.debug +1
-0@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISONlib/Kconfig.debug config DEBUG_VM_PGFLAGS · line 650
Quoted 10 lines
lib/Kconfig.debug +10
-0+ bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory.What happens when
DEBUG_VM_PGFLAGS= y andDEBUG_VM_PAGE_INIT_POISON= n ?
Quoted 8 lines
We are testing for pattern that was not set?
I think
DEBUG_VM_PAGE_INIT_POISONmust depend onDEBUG_VM_PGFLAGSinstead.Looks good otherwise.
Thank you,
Pavel
The problem is that I then end up in the same situation I had in the last patch where you have to have DEBUG_VM_PGFLAGS on in order to do the seeding with poison.
I can wrap the bit of code in PagePoisoned to just always return false if we didn't set the pattern. I figure there is value to be had for running DEBUG_VM_PGFLAGS regardless of the poison check, or DEBUG_VM_PAGE_INIT_POISON without the PGFLAGS check. That is why I wanted to leave them independent.
- Alex
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 83 lines · Alexander Duyck
Quoted 81 lines · Pasha Tatashin
Quoted 59 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the
PAGE_POISON_PATTERNvalue. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.In order to work around the issue I had to disable
CONFIG_DEBUG_VMand then the boot time returned to something much more reasonable as thearch_add_memorycall completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.Signed-off-by Alexander Duyck <[email protected]>--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)include/linux/page-flags.h +8
-0@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits: @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages * lib/Kconfig.debug +1
-0@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISONlib/Kconfig.debug config DEBUG_VM_PGFLAGS · line 650
Quoted 10 lines
lib/Kconfig.debug +10
-0+ bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory.What happens when
DEBUG_VM_PGFLAGS= y andDEBUG_VM_PAGE_INIT_POISON= n ?We are testing for pattern that was not set?
I think
DEBUG_VM_PAGE_INIT_POISONmust depend onDEBUG_VM_PGFLAGSinstead.
Quoted 8 lines
Quoted 4 lines
Looks good otherwise.
Thank you,
PavelThe problem is that I then end up in the same situation I had in the last patch where you have to have
DEBUG_VM_PGFLAGSon in order to do the seeding with poison.
OK
Quoted 6 lines
I can wrap the bit of code in PagePoisoned to just always return false if we didn't set the pattern. I figure there is value to be had for running
DEBUG_VM_PGFLAGSregardless of the poison check, orDEBUG_VM_PAGE_INIT_POISONwithout the PGFLAGS check. That is why I wanted to leave them independent.
How about:
Remove "depends on DEBUG_VM", but make DEBUG_VM_PGFLAGS to depend on both DEBUG_VM and DEBUG_VM_PAGE_INIT_POISON?
DEBUG_VM_PGFLAGS is already extremely slow, so having this extra dependency is OK.
Thank you,
Pavel
Quoted 3 lines
- Alex
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 107 lines · Pasha Tatashin
Quoted 83 lines · Alexander Duyck
Quoted 81 lines · Pasha Tatashin
Quoted 59 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the
PAGE_POISON_PATTERNvalue. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.In order to work around the issue I had to disable
CONFIG_DEBUG_VMand then the boot time returned to something much more reasonable as thearch_add_memorycall completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.Signed-off-by Alexander Duyck <[email protected]>--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)include/linux/page-flags.h +8
-0@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits: @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages * lib/Kconfig.debug +1
-0@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISONlib/Kconfig.debug config DEBUG_VM_PGFLAGS · line 650
Quoted 10 lines
lib/Kconfig.debug +10
-0+ bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory.What happens when
DEBUG_VM_PGFLAGS= y andDEBUG_VM_PAGE_INIT_POISON= n ?We are testing for pattern that was not set?
I think
DEBUG_VM_PAGE_INIT_POISONmust depend onDEBUG_VM_PGFLAGSinstead.Quoted 8 lines
Quoted 4 lines
Looks good otherwise.
Thank you,
PavelThe problem is that I then end up in the same situation I had in the last patch where you have to have
DEBUG_VM_PGFLAGSon in order to do the seeding with poison.OK
Quoted 6 lines
I can wrap the bit of code in PagePoisoned to just always return false if we didn't set the pattern. I figure there is value to be had for running
DEBUG_VM_PGFLAGSregardless of the poison check, orDEBUG_VM_PAGE_INIT_POISONwithout the PGFLAGS check. That is why I wanted to leave them independent.How about:
Quoted 8 lines
Remove "depends on
DEBUG_VM", but makeDEBUG_VM_PGFLAGSto depend on bothDEBUG_VMandDEBUG_VM_PAGE_INIT_POISON?
DEBUG_VM_PGFLAGSis already extremely slow, so having this extra dependency is OK.Thank you,
Pavel
Why create the extra dependency though? In the case of DEBUG_VM I am doing it so that we can retain the original behavior where enabling DEBUG_VM should enable the poisoning. I want to avoid this just getting disabled by default and want to try to stick to the original behavior as closely as possible unless we want to go in and explicitly turn it off.
Quoted 1 line
From what I can tell the original code prior to your changes didn't
bother checking for the poison value when testing the page flags. The poison value only applies prior to the page being initialized, so the value add for having it is only so much. It makes more sense to me to have these as two separate config options where enabling both would give you the maximum benefit, but you could test with either one or the other if you so desired.
- Alex
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
In reply to Alexander DuyckQuoted 3 lines · Alexander Duyck
Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.
I guess this is a reasonable compromise.
If folks see odd 'struct page' corruption, they'll have to know to go turn this on and reboot, though.
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
In reply to Alexander DuyckQuoted 7 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
On systems with a large amount of memory it can take a significant amount of time to initialize all of the page structs with the
PAGE_POISON_PATTERNvalue. I have seen it take over 2 minutes to initialize a system with over 12GB of RAM.
Quoted 8 lines
In order to work around the issue I had to disable
CONFIG_DEBUG_VMand then the boot time returned to something much more reasonable as thearch_add_memorycall completed in milliseconds versus seconds. However in doing that I had to disable all of the other VM debugging on the system.Instead of keeping the value in
CONFIG_DEBUG_VMI am adding a new CONFIG value calledCONFIG_DEBUG_VM_PAGE_INIT_POISONthat will control the page poisoning independent of theCONFIG_DEBUG_VMoption.
As explained in other thread (please slow down when posting a new version until the previous discussion reaches a consensus next time), I really detest a new config. We have way too many of those. If you really have to then make sure to describe _why_ it is needed. Sure initialization takes some time and that is one-off overhead. So why does it matter at all for somebody willing to pat runtime overhead all over the MM code paths. In other words, why do you have to keep DEBUG_VM enabled for workloads where the boot time matters so much that few seconds matter?
Quoted 96 lines
Signed-off-by Alexander Duyck <[email protected]>--- include/linux/page-flags.h | 8 ++++++++ lib/Kconfig.debug | 14 ++++++++++++++ mm/memblock.c | 5 ++--- mm/sparse.c | 4 +--- 4 files changed, 25 insertions(+), 6 deletions(-)include/linux/page-flags.h +8
-0@@ -13,6 +13,7 @@#include <linux/mm_types.h> #include <generated/bounds.h> #endif /* !__GENERATING_BOUNDS_H */ +#include <linux/string.h> /* * Various page->flags bits: @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)return page->flags ==PAGE_POISON_PATTERN ; } +static inline void page_init_poison (struct page *page,size_t size) +{ +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON +memset (page,PAGE_POISON_PATTERN , size); +#endif +} +/* * Page flags policies wrt compound pages * lib/Kconfig.debug +14
-0@@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS If unsure, say N. +config DEBUG_VM_PAGE_INIT_POISON + bool "Enable early page metadata poisoning" + default y + depends on DEBUG_VM + help + Seed the page metadata with a poison pattern to improve the + likelihood of detecting attempts to access the page prior to + initialization by the memory subsystem. + + This initialization can result in a longer boot time for systems + with a large amount of memory. + + If unsure, say Y. + config ARCH_HAS_DEBUG_VIRTUAL boolmm/memblock.c +2
-3@@ -1444,10 +1444,9 @@ void * __init memblock_virt_alloc_try_nid_raw( ptr =memblock_virt_alloc_internal (size, align, min_addr, max_addr, nid); -#ifdef CONFIG_DEBUG_VM if (ptr && size >0 ) -memset (ptr,PAGE_POISON_PATTERN , size); -#endif +page_init_poison (ptr, size); +return ptr; }mm/sparse.c +1
-3@@ -696,13 +696,11 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, goto out; } -#ifdef CONFIG_DEBUG_VM /* * Poison uninitialized struct pages in order to catch invalid flags * combinations. */ -memset (memmap,PAGE_POISON_PATTERN ,sizeof (struct page ) *PAGES_PER_SECTION ); -#endif +page_init_poison (memmap,sizeof (struct page ) *PAGES_PER_SECTION );section_mark_present (ms);sparse_init_one_section (ms, section_nr, memmap, usemap);
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?
There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECK
under which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 8 lines · Dave Hansen
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
Quoted 8 lines
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option. Something that takes few cycles but it is called often or one time stuff that takes quite a long but less than aggregated overhead of the former?
Just consider this particular case. It basically re-adds an overhead that has always been there before the struct page init optimization went it. The poisoning just returns it in a different form to catch potential left overs. And we would like to have as many people willing to running in debug mode to test for those paths because they are basically impossible to review by the code inspection. More importantnly the major overhead is boot time so my question still stands. Is this worth a separate config option almost nobody is going to enable?
Enabling DEBUG_VM by Fedora and others serves us a very good testing coverage and I appreciate that because it has generated some useful bug reports. Those people are paying quite a lot of overhead in runtime which can aggregate over time is it so much to ask about one time boot overhead?
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 33 lines · Michal Hocko
Quoted 8 lines · Dave Hansen
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
Quoted 8 lines
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option. Something that takes few cycles but it is called often or one time stuff that takes quite a long but less than aggregated overhead of the former?
Just consider this particular case. It basically re-adds an overhead that has always been there before the struct page init optimization went it. The poisoning just returns it in a different form to catch potential left overs. And we would like to have as many people willing to running in debug mode to test for those paths because they are basically impossible to review by the code inspection. More importantnly the major overhead is boot time so my question still stands. Is this worth a separate config option almost nobody is going to enable?
Quoted 5 lines
Enabling
DEBUG_VMby Fedora and others serves us a very good testing coverage and I appreciate that because it has generated some useful bug reports. Those people are paying quite a lot of overhead in runtime which can aggregate over time is it so much to ask about one time boot overhead?
The kind of boot time add-on I saw as a result of this was about 170 seconds, or 2 minutes and 50 seconds on a 12TB system. I spent a couple minutes wondering if I had built a bad kernel or not as I was staring at a dead console the entire time after the grub prompt since I hit this so early in the boot. That is the reason why I am so eager to slice this off and make it something separate. I could easily see this as something that would get in the way of other debugging that is going on in a system.
If we don't want to do a config option, then what about adding a kernel parameter to put a limit on how much memory we will initialize like this before we just start skipping it. We could put a default limit on it like 256GB and then once we cross that threshold we just don't bother poisoning any more memory. With that we would probably be able to at least cover most of the early memory init, and that value should cover most systems without getting into delays on the order of minutes.
- Alex
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 49 lines · Alexander Duyck
Quoted 33 lines · Michal Hocko
Quoted 8 lines · Dave Hansen
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
Quoted 8 lines
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option. Something that takes few cycles but it is called often or one time stuff that takes quite a long but less than aggregated overhead of the former?
Just consider this particular case. It basically re-adds an overhead that has always been there before the struct page init optimization went it. The poisoning just returns it in a different form to catch potential left overs. And we would like to have as many people willing to running in debug mode to test for those paths because they are basically impossible to review by the code inspection. More importantnly the major overhead is boot time so my question still stands. Is this worth a separate config option almost nobody is going to enable?
Quoted 5 lines
Enabling
DEBUG_VMby Fedora and others serves us a very good testing coverage and I appreciate that because it has generated some useful bug reports. Those people are paying quite a lot of overhead in runtime which can aggregate over time is it so much to ask about one time boot overhead?The kind of boot time add-on I saw as a result of this was about 170 seconds, or 2 minutes and 50 seconds on a 12TB system. I spent a couple minutes wondering if I had built a bad kernel or not as I was staring at a dead console the entire time after the grub prompt since I hit this so early in the boot. That is the reason why I am so eager to slice this off and make it something separate. I could easily see this as something that would get in the way of other debugging that is going on in a system.
Quoted 8 lines
If we don't want to do a config option, then what about adding a kernel parameter to put a limit on how much memory we will initialize like this before we just start skipping it. We could put a default limit on it like 256GB and then once we cross that threshold we just don't bother poisoning any more memory. With that we would probably be able to at least cover most of the early memory init, and that value should cover most systems without getting into delays on the order of minutes.
I am OK with a boot parameter to optionally disable it when DEBUG_VM is enabled. But, I do not think it is a good idea to make that parameter "smart" basically always poison memory with DEBUG_VM unless bootet with a parameter that tells not to poison memory.
CONFIG_DEBUG_VM is disbled on:
RedHat, Oracle Linux, CentOS, Ubuntu, Arch Linux, SUSE
Enabled on:
Fedora
Are there other distros where it is enabled? I think, this could be filed as a performance bug against Fedora distro, and let the decide what to do about it.
I do not want to make this feature less tested. Poisoning memory allowed us to catch corner case bugs like these:
ab1e8d8960b68f54af42b6484b5950bd13a4054b
mm: don't allow deferred pages with NEED_PER_CPU_KM
e181ae0c5db9544de9c53239eb22bc012ce75033
mm: zero unavailable pages before memmap init
And several more that were fixed by other people.
For a very long linux relied on assumption that boot memory is zeroed, and I am sure we will continue detect more bugs over time.
Thank you,
Pavel
Quoted 3 lines
- Alex
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 11 lines · Pasha Tatashin
I do not want to make this feature less tested. Poisoning memory allowed us to catch corner case bugs like these:
ab1e8d8960b68f54af42b6484b5950bd13a4054b
mm: don't allow deferred pages withNEED_PER_CPU_KM
e181ae0c5db9544de9c53239eb22bc012ce75033
mm: zero unavailable pages before memmap initAnd several more that were fixed by other people.
Just curious: were these found in the wild, or by a developer doing normal development having turned on lots of debug options?
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 8 lines · Dave Hansen
Quoted 7 lines · Pasha Tatashin
I do not want to make this feature less tested. Poisoning memory allowed us to catch corner case bugs like these:
ab1e8d8960b68f54af42b6484b5950bd13a4054b
mm: don't allow deferred pages withNEED_PER_CPU_KM
Quoted 7 lines
Quoted 4 lines
e181ae0c5db9544de9c53239eb22bc012ce75033
mm: zero unavailable pages before memmap initAnd several more that were fixed by other people.
Just curious: were these found in the wild, or by a developer doing normal development having turned on lots of debug options?
Some of those were 0day AFAIR but my memory is quite dim. Pavel will know better. The bottom line is, however, that those bugs depend on strange or unexpected memory configurations or HW which is usually deployed outside of developers machine pool. So more people have this enabled the more likely we hit all those strange corner cases nobody even thought of.
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
In reply to Alexander DuyckQuoted 34 lines · Alexander Duyck
Quoted 33 lines · Michal Hocko
Quoted 8 lines · Dave Hansen
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
Quoted 8 lines
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option. Something that takes few cycles but it is called often or one time stuff that takes quite a long but less than aggregated overhead of the former?
Just consider this particular case. It basically re-adds an overhead that has always been there before the struct page init optimization went it. The poisoning just returns it in a different form to catch potential left overs. And we would like to have as many people willing to running in debug mode to test for those paths because they are basically impossible to review by the code inspection. More importantnly the major overhead is boot time so my question still stands. Is this worth a separate config option almost nobody is going to enable?
Quoted 8 lines
Quoted 5 lines
Enabling
DEBUG_VMby Fedora and others serves us a very good testing coverage and I appreciate that because it has generated some useful bug reports. Those people are paying quite a lot of overhead in runtime which can aggregate over time is it so much to ask about one time boot overhead?The kind of boot time add-on I saw as a result of this was about 170 seconds, or 2 minutes and 50 seconds on a 12TB system.
Just curious. How long does it take to get from power on to even reaach boot loader on that machine... ;)
Quoted 7 lines
I spent a
couple minutes wondering if I had built a bad kernel or not as I was staring at a dead console the entire time after the grub prompt since I hit this so early in the boot. That is the reason why I am so eager to slice this off and make it something separate. I could easily see this as something that would get in the way of other debugging that is going on in a system.
But you would get the same overhead a kernel release ago when the memmap init optimization was merged. So you are basically back to what we used to have for years. Unless I misremember.
Quoted 8 lines
If we don't want to do a config option, then what about adding a kernel parameter to put a limit on how much memory we will initialize like this before we just start skipping it. We could put a default limit on it like 256GB and then once we cross that threshold we just don't bother poisoning any more memory. With that we would probably be able to at least cover most of the early memory init, and that value should cover most systems without getting into delays on the order of minutes.
No, this will defeat the purpose of the check.
Signature · 2 lines
Michal Hocko SUSE Labs
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
Quoted 55 lines · Michal Hocko
Quoted 34 lines · Alexander Duyck
Quoted 33 lines · Michal Hocko
Quoted 8 lines · Dave Hansen
Quoted 2 lines · Michal Hocko
why do you have to keep
DEBUG_VMenabled for workloads where the boot time matters so much that few seconds matter?There are a number of distributions that run with it enabled in the default build. Fedora, for one. We've basically assumed for a while that we have to live with it in production environments.
Quoted 8 lines
So, where does leave us? I think we either need a _generic_ debug option like:
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option. Something that takes few cycles but it is called often or one time stuff that takes quite a long but less than aggregated overhead of the former?
Just consider this particular case. It basically re-adds an overhead that has always been there before the struct page init optimization went it. The poisoning just returns it in a different form to catch potential left overs. And we would like to have as many people willing to running in debug mode to test for those paths because they are basically impossible to review by the code inspection. More importantnly the major overhead is boot time so my question still stands. Is this worth a separate config option almost nobody is going to enable?
Quoted 8 lines
Quoted 5 lines
Enabling
DEBUG_VMby Fedora and others serves us a very good testing coverage and I appreciate that because it has generated some useful bug reports. Those people are paying quite a lot of overhead in runtime which can aggregate over time is it so much to ask about one time boot overhead?The kind of boot time add-on I saw as a result of this was about 170 seconds, or 2 minutes and 50 seconds on a 12TB system.
Just curious. How long does it take to get from power on to even reaach boot loader on that machine... ;)
Quoted 7 lines
I spent a
couple minutes wondering if I had built a bad kernel or not as I was staring at a dead console the entire time after the grub prompt since I hit this so early in the boot. That is the reason why I am so eager to slice this off and make it something separate. I could easily see this as something that would get in the way of other debugging that is going on in a system.
Quoted 3 lines
But you would get the same overhead a kernel release ago when the memmap init optimization was merged. So you are basically back to what we used to have for years. Unless I misremember.
You remeber this correctly:
2f47a91f4dab19aaaa05cdcfced9dfcaf3f5257e has data before vs after zeroing memory in memblock allocator.
On SPARC with 15T we saved 55.4s, because SPARC has larger base pages, thus fewer struct pages.
On x86 with 1T saved 15.8s: which is 189.6s if it was 12T machine Alexander is using, close to 170s he is seeing, but CPU must be faster.
Pavel
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
In reply to Michal HockoQuoted 7 lines · Michal Hocko
Quoted 5 lines
CONFIG_DEBUG_VM_SLOW_AS_HECKunder which we can put this an other really slow VM debugging. Or, we need some kind of boot-time parameter to trigger the extra checking instead of a new CONFIG option.
I strongly suspect nobody will ever enable such a scary looking config TBH. Besides I am not sure what should go under that config option.
OK, so call it CONFIG_DEBUG_VM2, or CONFIG_DEBUG_VM_MORE. :)
What do we put under it? The things that folks complain about that get turned on with DEBUG_VM, like this.
Quoted 2 lines
Is this worth a separate config option almost nobody is going to enable?
Yes. We get basically *zero* debug checking from this option. We want it available to developers mucking with boot and hotplug, but it's honestly not worth it for normal users.
Has anyone ever seen a single in-the-wild report from this mechanism?
Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT_POISON
On Thu 06-09-18 09:09:46, Dave Hansen wrote:
[...]
Quoted 1 line
Has anyone ever seen a single in-the-wild report from this mechanism?
Yes. See the list from Pavel. And I wouldn't push for it otherwise. There are some questionable asserts with an overhead which is not directly visible but it just adds up. This is different that it is one time boot rare thing.
Anyway, I guess I have put all my arguments on the table. I will leave the decision to you guys. If there is a strong concensus about a config option, then I can live with that and will enable it.
Signature · 2 lines
Michal Hocko SUSE Labs
PATCH v2 2/2 mm: Create non-atomic version of SetPageReserved for init use
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
It doesn't make much sense to use the atomic SetPageReserved at init time when we are using memset to clear the memory and manipulating the page flags via simple "&=" and "|=" operations in __init_single_page.
This patch adds a non-atomic version __SetPageReserved that can be used during page init and shows about a 10% improvement in initialization times on the systems I have available for testing.
I tried adding a bit of documentation based on commit <f1dd2cd13c4> ("mm,memory_hotplug: do not associate hotadded memory to zones until online").
Ideally the reserved flag should be set earlier since there is a brief window where the page is initialization via __init_single_page and we have not set the PG_Reserved flag. I'm leaving that for a future patch set as that will require a more significant refactor.
--- include/linux/page-flags.h | 1 + mm/page_alloc.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-)
include/linux/page-flags.h +1 -0
@@ -300,6 +300,7 @@ static inline void page_init_poison(struct page *page, size_t size) PAGEFLAG(Reserved, reserved, PF_NO_COMPOUND) __CLEARPAGEFLAG(Reserved, reserved, PF_NO_COMPOUND) + __SETPAGEFLAG(Reserved, reserved, PF_NO_COMPOUND) PAGEFLAG(SwapBacked, swapbacked, PF_NO_TAIL) __CLEARPAGEFLAG(SwapBacked, swapbacked, PF_NO_TAIL)__SETPAGEFLAG (SwapBacked, swapbacked,PF_NO_TAIL )
mm/page_alloc.c +11 -2
@@ -1231,7 +1231,8 @@ void __meminit reserve_bootmem_region(phys_addr_t start, phys_addr_t end)/* Avoid false-positive PageTail() */ INIT_LIST_HEAD (&page->lru); -SetPageReserved (page); +/* no need for atomic set_bit at init time */ +__SetPageReserved (page); } } }
@@ -5517,8 +5518,16 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone, not_early: page =pfn_to_page (pfn);__init_single_page (page, pfn, zone, nid); + +/* +* Mark page reserved as it will need to wait for onlining +* phase for it to be fully associated with a zone. +* +* We can use the non-atomic __set_bit operation for setting +* the flag as we are still initializing the pages. +*/ if (context ==MEMMAP_HOTPLUG ) -SetPageReserved (page); +__SetPageReserved (page);/* * Mark the block movable so that blocks are reserved for
Re: [PATCH v2 2/2] mm: Create non-atomic version of SetPageReserved for init use
On Wed 05-09-18 14:13:34, Alexander Duyck wrote:
[...]
just a nit
mm/page_alloc.c void __meminit reserve_bootmem_region(phys_addr_t start, phys_addr_t end) · line 1235
Quoted 7 lines
mm/page_alloc.c +2
-1@@ -1231,7 +1231,8 @@ void __meminit reserve_bootmem_region(phys_addr_t start, phys_addr_t end)/* Avoid false-positive PageTail() */ INIT_LIST_HEAD (&page->lru); -SetPageReserved (page); +/* no need for atomic set_bit at init time */ +__SetPageReserved (page);
OK but I guess it would be much more clear to say
/*
* no need for atomic set_bit because the struct
* page is not visible yet so nobody should
* access it yet.
*/
Quoted 3 lines
(quoted diff) +0
-0} } }
Signature · 2 lines
Michal Hocko SUSE Labs