mm: drop superfluous section checks when onlining/offlining
Subsystems (5)
cover mm: drop superfluous section checks when onlining/offlining
Let's drop some superfluous section checks on the onlining/offlining path.
David Hildenbrand (3):
drivers/base/memory.c: drop section_count drivers/base/memory.c: drop pages_correctly_probed() mm/page_ext.c: drop pfn_present() check when onlining
drivers/base/memory.c | 59 +++--------------------------------------- include/linux/memory.h | 1 - mm/page_ext.c | 5 +--- 3 files changed, 4 insertions(+), 61 deletions(-)
Signature · 1 line
2.24.1
PATCH 1/3 drivers/base/memory.c: drop section_count
Since commit c5e79ef561b0 ("mm/memory_hotplug.c: don't allow to online/offline memory blocks with holes") we have a generic check in offline_pages() that disallows offlining memory blocks with holes.
Memory blocks with missing sections are just another variant of these type of blocks. We can stop checking (and especially storing) present sections. A proper error message is now printed why offlining failed.
section_count was initially introduced in commit 07681215975e ("Driver
core: Add section count to memory_block struct") in order to detect when it is okay to remove a memory block. It was used in commit 26bbe7ef6d5c ("drivers/base/memory.c: prohibit offlining of memory blocks with missing sections") to disallow offlining memory blocks with missing sections. As we refactored creation/removal of memory devices and have a proper check for holes in place, we can drop the section_count.
This also removes a leftover comment regarding the mem_sysfs_mutex, which was removed in commit 848e19ad3c33 ("drivers/base/memory.c: drop the mem_sysfs_mutex").
--- drivers/base/memory.c | 17 +++-------------- include/linux/memory.h | 1 - 2 files changed, 3 insertions(+), 15 deletions(-)
drivers/base/memory.c +3 -14
@@ -292,10 +292,6 @@ static int memory_subsys_offline(struct device *dev)if (mem->state ==MEM_OFFLINE )return 0 ; -/* Can't offline block with non-present sections */ -if (mem->section_count != sections_per_block) -return -EINVAL ; -return memory_block_change_state (mem,MEM_OFFLINE ,MEM_ONLINE ); }
@@ -660,7 +656,7 @@ static int init_memory_block(struct memory_block **memory,static int add_memory_block (unsigned long base_section_nr) { -int ret, section_count =0 ; +int section_count =0 ;struct memory_block *mem;unsigned long nr;
@@ -671,12 +667,8 @@ static int add_memory_block(unsigned long base_section_nr)if (section_count ==0 )return 0 ; - ret =init_memory_block (&mem,base_memory_block_id (base_section_nr), -MEM_ONLINE ); -if (ret) -return ret; - mem->section_count = section_count; -return 0 ; +return init_memory_block (&mem,base_memory_block_id (base_section_nr), +MEM_ONLINE ); }static void unregister_memory (struct memory_block *memory)
@@ -714,7 +706,6 @@ int create_memory_block_devices(unsigned long start, unsigned long size) ret =init_memory_block (&mem, block_id,MEM_OFFLINE );if (ret)break ; - mem->section_count = sections_per_block; }if (ret) { end_block_id = block_id;
@@ -723,7 +714,6 @@ int create_memory_block_devices(unsigned long start, unsigned long size) mem =find_memory_block_by_id (block_id);if (WARN_ON_ONCE (!mem))continue ; - mem->section_count =0 ;unregister_memory (mem); } }
@@ -752,7 +742,6 @@ void remove_memory_block_devices(unsigned long start, unsigned long size) mem =find_memory_block_by_id (block_id);if (WARN_ON_ONCE (!mem))continue ; - mem->section_count =0 ;unregister_memory_block_under_nodes (mem);unregister_memory (mem); }
include/linux/memory.h +0 -1
@@ -26,7 +26,6 @@struct memory_block {unsigned long start_section_nr;unsigned long state;/* serialized by the dev->lock */ -int section_count;/* serialized by mem_sysfs_mutex */ int online_type;/* for passing data to online routine */ int phys_device;/* to which fru does this belong? */ struct device dev;
Signature · 1 line
2.24.1
PATCH 2/3 drivers/base/memory.c: drop pages_correctly_probed()
In reply to David Hildenbrandpages_correctly_probed() is a leftover from ancient times. It dates back to commit 3947be1969a9 ("[PATCH] memory hotplug: sysfs and add/remove functions"), where Pg_reserved checks were added as a sfety net:
/*
* The probe routines leave the pages reserved, just
* as the bootmem code does. Make sure they're still
* that way.
*/
The checks were refactored quite a bit over the years, especially in commit b77eab7079d9 ("mm/memory_hotplug: optimize probe routine"), where checks for present, valid, and online sections were added.
Hotplugged memory is added via add_memory(), which will create the full memmap for the hotplugged memory, and mark all sections valid and present. Only full memory blocks are onlined/offlined, so we also cannot have an inconsistency in that regard (especially, memory blocks with some sections being online and some being offline).
1. Boot memory always starts online. Since commit c5e79ef561b0 ("mm/memory_hotplug.c: don't allow to online/offline memory blocks with holes") we disallow to offline any memory with holes. Therefore, we never online memory with holes. Present and validity checks are superfluous.
2. Only complete memory blocks are onlined/offlined (and especially, the state - online or offline - is stored for whole memory blocks). Besides the core, only arch/powerpc/platforms/powernv/memtrace.c manually calls offline_pages() and fiddels with memory block states. But it also only offlines complete memory blocks.
3. To make any of these conditions trigger, something would have to be terribly messed up in the core. (e.g., online/offline only some sections of a memory block).
4. Memory unplug properly makes sure that all sysfs attributes were removed (and therefore, that all threads left the sysfs handlers). We don't have to worry about zombie devices at this point.
5. The valid_section_nr(section_nr) check is actually dead code, as it would never have been reached due to the WARN_ON_ONCE(!pfn_valid(pfn)).
No wonder we haven't seen any of these errors in a long time (or even ever, according to my search). Let's just get rid of them. Now, all checks that could hinder onlining and offlining are completely contained in online_pages()/offline_pages().
--- drivers/base/memory.c | 42 ------------------------------------------ 1 file changed, 42 deletions(-)
drivers/base/memory.c +0 -42
@@ -169,45 +169,6 @@ int memory_notify(unsigned long val, void *v)return blocking_notifier_call_chain (&memory_chain, val, v); } -/* -* The probe routines leave the pages uninitialized, just as the bootmem code -* does. Make sure we do not access them, but instead use only information from -* within sections. -*/ -static bool pages_correctly_probed (unsigned long start_pfn) -{ -unsigned long section_nr =pfn_to_section_nr (start_pfn); -unsigned long section_nr_end = section_nr + sections_per_block; -unsigned long pfn = start_pfn; - -/* -* memmap between sections is not contiguous except with -* SPARSEMEM_VMEMMAP. We lookup the page once per section -* and assume memmap is contiguous within each section -*/ -for (; section_nr < section_nr_end; section_nr++) { -if (WARN_ON_ONCE (!pfn_valid (pfn))) -return false; - -if (!present_section_nr (section_nr)) { -pr_warn ("section %ld pfn[%lx, %lx) not present\n" , - section_nr, pfn, pfn +PAGES_PER_SECTION ); -return false; - }else if (!valid_section_nr (section_nr)) { -pr_warn ("section %ld pfn[%lx, %lx) no valid memmap\n" , - section_nr, pfn, pfn +PAGES_PER_SECTION ); -return false; - }else if (online_section_nr (section_nr)) { -pr_warn ("section %ld pfn[%lx, %lx) is already online\n" , - section_nr, pfn, pfn +PAGES_PER_SECTION ); -return false; - } - pfn +=PAGES_PER_SECTION ; - } - -return true; -} -/* * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is * OK to have direct references to sparsemem variables in here.
@@ -224,9 +185,6 @@ memory_block_action(unsigned long start_section_nr, unsigned long action,switch (action) {case MEM_ONLINE : -if (!pages_correctly_probed (start_pfn)) -return -EBUSY ; - ret =online_pages (start_pfn, nr_pages, online_type, nid);break ;case MEM_OFFLINE :
Signature · 1 line
2.24.1
PATCH 3/3 mm/page_ext.c: drop pfn_present() check when onlining
In reply to David HildenbrandSince commit c5e79ef561b0 ("mm/memory_hotplug.c: don't allow to online/offline memory blocks with holes") we disallow to offline any memory with holes. As all boot memory is online and hotplugged memory cannot contain holes, we never online memory with holes.
This present check can be dropped.
--- mm/page_ext.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
mm/page_ext.c +1 -4
@@ -303,11 +303,8 @@ static int __meminit online_page_ext(unsigned long start_pfn,VM_BUG_ON (!node_state (nid,N_ONLINE )); } -for (pfn = start; !fail && pfn < end; pfn +=PAGES_PER_SECTION ) { -if (!pfn_present (pfn)) -continue ; +for (pfn = start; !fail && pfn < end; pfn +=PAGES_PER_SECTION ) fail =init_section_page_ext (pfn, nid); - }if (!fail)return 0 ;
Signature · 1 line
2.24.1