From: Muchun Song <hidden> Date: 2021-02-19 10:52:40
Hi all,
This patch series will free some vmemmap pages(struct page structures)
associated with each hugetlbpage when preallocated to save memory.
In order to reduce the difficulty of the first version of code review.
From this version, we disable PMD/huge page mapping of vmemmap if this
feature was enabled. This accutualy eliminate a bunch of the complex code
doing page table manipulation. When this patch series is solid, we cam add
the code of vmemmap page table manipulation in the future.
The struct page structures (page structs) are used to describe a physical
page frame. By default, there is a one-to-one mapping from a page frame to
it's corresponding page struct.
The HugeTLB pages consist of multiple base page size pages and is supported
by many architectures. See hugetlbpage.rst in the Documentation directory
for more details. On the x86 architecture, HugeTLB pages of size 2MB and 1GB
are currently supported. Since the base page size on x86 is 4KB, a 2MB
HugeTLB page consists of 512 base pages and a 1GB HugeTLB page consists of
4096 base pages. For each base page, there is a corresponding page struct.
Within the HugeTLB subsystem, only the first 4 page structs are used to
contain unique information about a HugeTLB page. HUGETLB_CGROUP_MIN_ORDER
provides this upper limit. The only 'useful' information in the remaining
page structs is the compound_head field, and this field is the same for all
tail pages.
By removing redundant page structs for HugeTLB pages, memory can returned to
the buddy allocator for other uses.
When the system boot up, every 2M HugeTLB has 512 struct page structs which
size is 8 pages(sizeof(struct page) * 512 / PAGE_SIZE).
HugeTLB struct pages(8 pages) page frame(8 pages)
+-----------+ ---virt_to_page---> +-----------+ mapping to +-----------+
| | | 0 | -------------> | 0 |
| | +-----------+ +-----------+
| | | 1 | -------------> | 1 |
| | +-----------+ +-----------+
| | | 2 | -------------> | 2 |
| | +-----------+ +-----------+
| | | 3 | -------------> | 3 |
| | +-----------+ +-----------+
| | | 4 | -------------> | 4 |
| 2MB | +-----------+ +-----------+
| | | 5 | -------------> | 5 |
| | +-----------+ +-----------+
| | | 6 | -------------> | 6 |
| | +-----------+ +-----------+
| | | 7 | -------------> | 7 |
| | +-----------+ +-----------+
| |
| |
| |
+-----------+
The value of page->compound_head is the same for all tail pages. The first
page of page structs (page 0) associated with the HugeTLB page contains the 4
page structs necessary to describe the HugeTLB. The only use of the remaining
pages of page structs (page 1 to page 7) is to point to page->compound_head.
Therefore, we can remap pages 2 to 7 to page 1. Only 2 pages of page structs
will be used for each HugeTLB page. This will allow us to free the remaining
6 pages to the buddy allocator.
Here is how things look after remapping.
HugeTLB struct pages(8 pages) page frame(8 pages)
+-----------+ ---virt_to_page---> +-----------+ mapping to +-----------+
| | | 0 | -------------> | 0 |
| | +-----------+ +-----------+
| | | 1 | -------------> | 1 |
| | +-----------+ +-----------+
| | | 2 | ----------------^ ^ ^ ^ ^ ^
| | +-----------+ | | | | |
| | | 3 | ------------------+ | | | |
| | +-----------+ | | | |
| | | 4 | --------------------+ | | |
| 2MB | +-----------+ | | |
| | | 5 | ----------------------+ | |
| | +-----------+ | |
| | | 6 | ------------------------+ |
| | +-----------+ |
| | | 7 | --------------------------+
| | +-----------+
| |
| |
| |
+-----------+
When a HugeTLB is freed to the buddy system, we should allocate 6 pages for
vmemmap pages and restore the previous mapping relationship.
Apart from 2MB HugeTLB page, we also have 1GB HugeTLB page. It is similar
to the 2MB HugeTLB page. We also can use this approach to free the vmemmap
pages.
In this case, for the 1GB HugeTLB page, we can save 4094 pages. This is a
very substantial gain. On our server, run some SPDK/QEMU applications which
will use 1024GB hugetlbpage. With this feature enabled, we can save ~16GB
(1G hugepage)/~12GB (2MB hugepage) memory.
Because there are vmemmap page tables reconstruction on the freeing/allocating
path, it increases some overhead. Here are some overhead analysis.
1) Allocating 10240 2MB hugetlb pages.
a) With this patch series applied:
# time echo 10240 > /proc/sys/vm/nr_hugepages
real 0m0.166s
user 0m0.000s
sys 0m0.166s
# bpftrace -e 'kprobe:alloc_fresh_huge_page { @start[tid] = nsecs; }
kretprobe:alloc_fresh_huge_page /@start[tid]/ { @latency = hist(nsecs -
@start[tid]); delete(@start[tid]); }'
Attaching 2 probes...
@latency:
[8K, 16K) 5476 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[16K, 32K) 4760 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ |
[32K, 64K) 4 | |
b) Without this patch series:
# time echo 10240 > /proc/sys/vm/nr_hugepages
real 0m0.067s
user 0m0.000s
sys 0m0.067s
# bpftrace -e 'kprobe:alloc_fresh_huge_page { @start[tid] = nsecs; }
kretprobe:alloc_fresh_huge_page /@start[tid]/ { @latency = hist(nsecs -
@start[tid]); delete(@start[tid]); }'
Attaching 2 probes...
@latency:
[4K, 8K) 10147 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[8K, 16K) 93 | |
Summarize: this feature is about ~2x slower than before.
2) Freeing 10240 2MB hugetlb pages.
a) With this patch series applied:
# time echo 0 > /proc/sys/vm/nr_hugepages
real 0m0.213s
user 0m0.000s
sys 0m0.213s
# bpftrace -e 'kprobe:free_pool_huge_page { @start[tid] = nsecs; }
kretprobe:free_pool_huge_page /@start[tid]/ { @latency = hist(nsecs -
@start[tid]); delete(@start[tid]); }'
Attaching 2 probes...
@latency:
[8K, 16K) 6 | |
[16K, 32K) 10227 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[32K, 64K) 7 | |
b) Without this patch series:
# time echo 0 > /proc/sys/vm/nr_hugepages
real 0m0.081s
user 0m0.000s
sys 0m0.081s
# bpftrace -e 'kprobe:free_pool_huge_page { @start[tid] = nsecs; }
kretprobe:free_pool_huge_page /@start[tid]/ { @latency = hist(nsecs -
@start[tid]); delete(@start[tid]); }'
Attaching 2 probes...
@latency:
[4K, 8K) 6805 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
[8K, 16K) 3427 |@@@@@@@@@@@@@@@@@@@@@@@@@@ |
[16K, 32K) 8 | |
Summarize: The overhead of __free_hugepage is about ~2-3x slower than before.
Although the overhead has increased, the overhead is not significant. Like Mike
said, "However, remember that the majority of use cases create hugetlb pages at
or shortly after boot time and add them to the pool. So, additional overhead is
at pool creation time. There is no change to 'normal run time' operations of
getting a page from or returning a page to the pool (think page fault/unmap)".
Despite the overhead and in addition to the memory gains from this series. The
following data is obtained by Joao Martins. Very thanks to his effort.
There's an additional benefit which is page (un)pinners will see an improvement
and Joao presumes because there are fewer memmap pages and thus the tail/head
pages are staying in cache more often.
Out of the box Joao saw (when comparing linux-next against linux-next + this series)
with gup_test and pinning a 16G hugetlb file (with 1G pages):
get_user_pages(): ~32k -> ~9k
unpin_user_pages(): ~75k -> ~70k
Usually any tight loop fetching compound_head(), or reading tail pages data (e.g.
compound_head) benefit a lot. There's some unpinning inefficiencies Joao was
fixing[0], but with that in added it shows even more:
unpin_user_pages(): ~27k -> ~3.8k
[0] https://lore.kernel.org/linux-mm/20210204202500.26474-1-joao.m.martins@oracle.com/
Todo:
- Free all of the tail vmemmap pages
Now for the 2MB HugrTLB page, we only free 6 vmemmap pages. we really can
free 7 vmemmap pages. In this case, we can see 8 of the 512 struct page
structures has beed set PG_head flag. If we can adjust compound_head()
slightly and make compound_head() return the real head struct page when
the parameter is the tail struct page but with PG_head flag set.
In order to make the code evolution route clearer. This feature can can be
a separate patch after this patchset is solid.
- Support for other architectures (e.g. aarch64).
- Enable PMD/huge page mapping of vmemmap even if this feature was enabled.
Changelog in v15 -> v16:
- Use GFP_KERNEL to allocate vmemmap pages.
Thanks to Mike, David H and Michal's suggestions.
Changelog in v14 -> v15:
- Fix some issues suggested by Oscar. Thanks to Oscar.
- Add numbers which Joao Martins tested to cover letter. Thanks to his effort.
Changelog in v13 -> v14:
- Refuse to free the HugeTLB page when the system is under memory pressure.
- Use GFP_ATOMIC to allocate vmemmap pages instead of GFP_KERNEL.
- Rebase to linux-next 20210202.
- Fix and add some comments for vmemmap_remap_free().
Thanks to Oscar, Mike, David H and David R's suggestions and review.
Changelog in v12 -> v13:
- Remove VM_WARN_ON_PAGE macro.
- Add more comments in vmemmap_pte_range() and vmemmap_remap_free().
Thanks to Oscar and Mike's suggestions and review.
Changelog in v11 -> v12:
- Move VM_WARN_ON_PAGE to a separate patch.
- Call __free_hugepage() with hugetlb_lock (See patch #5.) to serialize
with dissolve_free_huge_page(). It is to prepare for patch #9.
- Introduce PageHugeInflight. See patch #9.
Changelog in v10 -> v11:
- Fix compiler error when !CONFIG_HUGETLB_PAGE_FREE_VMEMMAP.
- Rework some comments and commit changes.
- Rework vmemmap_remap_free() to 3 parameters.
Thanks to Oscar and Mike's suggestions and review.
Changelog in v9 -> v10:
- Fix a bug in patch #11. Thanks to Oscar for pointing that out.
- Rework some commit log or comments. Thanks Mike and Oscar for the suggestions.
- Drop VMEMMAP_TAIL_PAGE_REUSE in the patch #3.
Thank you very much Mike and Oscar for reviewing the code.
Changelog in v8 -> v9:
- Rework some code. Very thanks to Oscar.
- Put all the non-hugetlb vmemmap functions under sparsemem-vmemmap.c.
Changelog in v7 -> v8:
- Adjust the order of patches.
Very thanks to David and Oscar. Your suggestions are very valuable.
Changelog in v6 -> v7:
- Rebase to linux-next 20201130
- Do not use basepage mapping for vmemmap when this feature is disabled.
- Rework some patchs.
[PATCH v6 08/16] mm/hugetlb: Free the vmemmap pages associated with each hugetlb page
[PATCH v6 10/16] mm/hugetlb: Allocate the vmemmap pages associated with each hugetlb page
Thanks to Oscar and Barry.
Changelog in v5 -> v6:
- Disable PMD/huge page mapping of vmemmap if this feature was enabled.
- Simplify the first version code.
Changelog in v4 -> v5:
- Rework somme comments and code in the [PATCH v4 04/21] and [PATCH v4 05/21].
Thanks to Mike and Oscar's suggestions.
Changelog in v3 -> v4:
- Move all the vmemmap functions to hugetlb_vmemmap.c.
- Make the CONFIG_HUGETLB_PAGE_FREE_VMEMMAP default to y, if we want to
disable this feature, we should disable it by a boot/kernel command line.
- Remove vmemmap_pgtable_{init, deposit, withdraw}() helper functions.
- Initialize page table lock for vmemmap through core_initcall mechanism.
Thanks for Mike and Oscar's suggestions.
Changelog in v2 -> v3:
- Rename some helps function name. Thanks Mike.
- Rework some code. Thanks Mike and Oscar.
- Remap the tail vmemmap page with PAGE_KERNEL_RO instead of PAGE_KERNEL.
Thanks Matthew.
- Add some overhead analysis in the cover letter.
- Use vmemap pmd table lock instead of a hugetlb specific global lock.
Changelog in v1 -> v2:
- Fix do not call dissolve_compound_page in alloc_huge_page_vmemmap().
- Fix some typo and code style problems.
- Remove unused handle_vmemmap_fault().
- Merge some commits to one commit suggested by Mike.
Muchun Song (9):
mm: memory_hotplug: factor out bootmem core functions to
bootmem_info.c
mm: hugetlb: introduce a new config HUGETLB_PAGE_FREE_VMEMMAP
mm: hugetlb: free the vmemmap pages associated with each HugeTLB page
mm: hugetlb: alloc the vmemmap pages associated with each HugeTLB page
mm: hugetlb: set the PageHWPoison to the raw error page
mm: hugetlb: add a kernel parameter hugetlb_free_vmemmap
mm: hugetlb: introduce nr_free_vmemmap_pages in the struct hstate
mm: hugetlb: gather discrete indexes of tail page
mm: hugetlb: optimize the code with the help of the compiler
Documentation/admin-guide/kernel-parameters.txt | 14 ++
Documentation/admin-guide/mm/hugetlbpage.rst | 12 +
arch/x86/mm/init_64.c | 13 +-
fs/Kconfig | 6 +
include/linux/bootmem_info.h | 65 ++++++
include/linux/hugetlb.h | 47 +++-
include/linux/hugetlb_cgroup.h | 19 +-
include/linux/memory_hotplug.h | 27 ---
include/linux/mm.h | 5 +
mm/Makefile | 2 +
mm/bootmem_info.c | 124 ++++++++++
mm/hugetlb.c | 166 +++++++++++---
mm/hugetlb_vmemmap.c | 293 ++++++++++++++++++++++++
mm/hugetlb_vmemmap.h | 51 +++++
mm/memory_hotplug.c | 116 ----------
mm/sparse-vmemmap.c | 280 ++++++++++++++++++++++
mm/sparse.c | 1 +
17 files changed, 1054 insertions(+), 187 deletions(-)
create mode 100644 include/linux/bootmem_info.h
create mode 100644 mm/bootmem_info.c
create mode 100644 mm/hugetlb_vmemmap.c
create mode 100644 mm/hugetlb_vmemmap.h
--
2.11.0
From: Muchun Song <hidden> Date: 2021-02-19 10:53:22
Move bootmem info registration common API to individual bootmem_info.c.
And we will use {get,put}_page_bootmem() to initialize the page for the
vmemmap pages or free the vmemmap pages to buddy in the later patch.
So move them out of CONFIG_MEMORY_HOTPLUG_SPARSE. This is just code
movement without any functional change.
Signed-off-by: Muchun Song <redacted>
Acked-by: Mike Kravetz <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: David Hildenbrand <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/mm/init_64.c | 3 +-
include/linux/bootmem_info.h | 40 +++++++++++++
include/linux/memory_hotplug.h | 27 ---------
mm/Makefile | 1 +
mm/bootmem_info.c | 124 +++++++++++++++++++++++++++++++++++++++++
mm/memory_hotplug.c | 116 --------------------------------------
mm/sparse.c | 1 +
7 files changed, 168 insertions(+), 144 deletions(-)
create mode 100644 include/linux/bootmem_info.h
create mode 100644 mm/bootmem_info.c
@@ -1571,7 +1572,7 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,returnerr;}-#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HAVE_BOOTMEM_INFO_NODE)+#ifdef CONFIG_HAVE_BOOTMEM_INFO_NODEvoidregister_page_bootmem_memmap(unsignedlongsection_nr,structpage*start_page,unsignedlongnr_pages){
@@ -18,18 +18,6 @@ struct vmem_altmap;#ifdef CONFIG_MEMORY_HOTPLUGstructpage*pfn_to_online_page(unsignedlongpfn);-/*-*Typesforfreebootmemstoredinpage->lru.next.Thesehavetobein-*somerandomrangeinunsignedlongspacefordebuggingpurposes.-*/-enum{-MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE=12,-SECTION_INFO=MEMORY_HOTPLUG_MIN_BOOTMEM_TYPE,-MIX_SECTION_INFO,-NODE_INFO,-MEMORY_HOTPLUG_MAX_BOOTMEM_TYPE=NODE_INFO,-};-/* Types for control the zone type of onlined and offlined memory */enum{/* Offline the memory. */
From: Muchun Song <hidden> Date: 2021-02-19 10:53:59
Every HugeTLB has more than one struct page structure. We __know__ that
we only use the first 4(HUGETLB_CGROUP_MIN_ORDER) struct page structures
to store metadata associated with each HugeTLB.
There are a lot of struct page structures associated with each HugeTLB
page. For tail pages, the value of compound_head is the same. So we can
reuse first page of tail page structures. We map the virtual addresses
of the remaining pages of tail page structures to the first tail page
struct, and then free these page frames. Therefore, we need to reserve
two pages as vmemmap areas.
When we allocate a HugeTLB page from the buddy, we can free some vmemmap
pages associated with each HugeTLB page. It is more appropriate to do it
in the prep_new_huge_page().
The free_vmemmap_pages_per_hpage(), which indicates how many vmemmap
pages associated with a HugeTLB page can be freed, returns zero for
now, which means the feature is disabled. We will enable it once all
the infrastructure is there.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
---
include/linux/bootmem_info.h | 27 +++++-
include/linux/mm.h | 3 +
mm/Makefile | 1 +
mm/hugetlb.c | 3 +
mm/hugetlb_vmemmap.c | 219 +++++++++++++++++++++++++++++++++++++++++++
mm/hugetlb_vmemmap.h | 20 ++++
mm/sparse-vmemmap.c | 207 ++++++++++++++++++++++++++++++++++++++++
7 files changed, 479 insertions(+), 1 deletion(-)
create mode 100644 mm/hugetlb_vmemmap.c
create mode 100644 mm/hugetlb_vmemmap.h
@@ -27,8 +27,215 @@#include<linux/spinlock.h>#include<linux/vmalloc.h>#include<linux/sched.h>+#include<linux/pgtable.h>+#include<linux/bootmem_info.h>+#include<asm/dma.h>#include<asm/pgalloc.h>+#include<asm/tlbflush.h>++/**+*vmemmap_remap_walk-walkvmemmappagetable+*+*@remap_pte:calledforeachlowest-levelentry(PTE).+*@reuse_page:thepagewhichisreusedforthetailvmemmappages.+*@reuse_addr:thevirtualaddressofthe@reuse_pagepage.+*@vmemmap_pages:thelistheadofthevmemmappagesthatcanbefreed.+*/+structvmemmap_remap_walk{+void(*remap_pte)(pte_t*pte,unsignedlongaddr,+structvmemmap_remap_walk*walk);+structpage*reuse_page;+unsignedlongreuse_addr;+structlist_head*vmemmap_pages;+};++staticvoidvmemmap_pte_range(pmd_t*pmd,unsignedlongaddr,+unsignedlongend,+structvmemmap_remap_walk*walk)+{+pte_t*pte;++pte=pte_offset_kernel(pmd,addr);++/*+*Thereuse_pageisfound'first'intablewalkbeforewestart+*remapping(whichiscalling@walk->remap_pte).+*/+if(!walk->reuse_page){+BUG_ON(pte_none(*pte));+BUG_ON(walk->reuse_addr!=addr);++walk->reuse_page=pte_page(*pte++);+/*+*Becausethereuseaddressispartoftherangethatweare+*walking,skipthereuseaddressrange.+*/+addr+=PAGE_SIZE;+}++for(;addr!=end;addr+=PAGE_SIZE,pte++){+BUG_ON(pte_none(*pte));++walk->remap_pte(pte,addr,walk);+}+}++staticvoidvmemmap_pmd_range(pud_t*pud,unsignedlongaddr,+unsignedlongend,+structvmemmap_remap_walk*walk)+{+pmd_t*pmd;+unsignedlongnext;++pmd=pmd_offset(pud,addr);+do{+BUG_ON(pmd_none(*pmd)||pmd_leaf(*pmd));++next=pmd_addr_end(addr,end);+vmemmap_pte_range(pmd,addr,next,walk);+}while(pmd++,addr=next,addr!=end);+}++staticvoidvmemmap_pud_range(p4d_t*p4d,unsignedlongaddr,+unsignedlongend,+structvmemmap_remap_walk*walk)+{+pud_t*pud;+unsignedlongnext;++pud=pud_offset(p4d,addr);+do{+BUG_ON(pud_none(*pud));++next=pud_addr_end(addr,end);+vmemmap_pmd_range(pud,addr,next,walk);+}while(pud++,addr=next,addr!=end);+}++staticvoidvmemmap_p4d_range(pgd_t*pgd,unsignedlongaddr,+unsignedlongend,+structvmemmap_remap_walk*walk)+{+p4d_t*p4d;+unsignedlongnext;++p4d=p4d_offset(pgd,addr);+do{+BUG_ON(p4d_none(*p4d));++next=p4d_addr_end(addr,end);+vmemmap_pud_range(p4d,addr,next,walk);+}while(p4d++,addr=next,addr!=end);+}++staticvoidvmemmap_remap_range(unsignedlongstart,unsignedlongend,+structvmemmap_remap_walk*walk)+{+unsignedlongaddr=start;+unsignedlongnext;+pgd_t*pgd;++VM_BUG_ON(!IS_ALIGNED(start,PAGE_SIZE));+VM_BUG_ON(!IS_ALIGNED(end,PAGE_SIZE));++pgd=pgd_offset_k(addr);+do{+BUG_ON(pgd_none(*pgd));++next=pgd_addr_end(addr,end);+vmemmap_p4d_range(pgd,addr,next,walk);+}while(pgd++,addr=next,addr!=end);++/*+*Weonlychangethemappingofthevmemmapvirtualaddressrange+*[@start+PAGE_SIZE,end),soweonlyneedtoflushtheTLBwhich+*belongstotherange.+*/+flush_tlb_kernel_range(start+PAGE_SIZE,end);+}++/*+*Freeavmemmappage.Avmemmappagecanbeallocatedfromthememblock+*allocatororbuddyallocator.IfthePG_reservedflagisset,itmeans+*thatitallocatedfromthememblockallocator,justfreeitviathe+*free_bootmem_page().Otherwise,use__free_page().+*/+staticinlinevoidfree_vmemmap_page(structpage*page)+{+if(PageReserved(page))+free_bootmem_page(page);+else+__free_page(page);+}++/* Free a list of the vmemmap pages */+staticvoidfree_vmemmap_page_list(structlist_head*list)+{+structpage*page,*next;++list_for_each_entry_safe(page,next,list,lru){+list_del(&page->lru);+free_vmemmap_page(page);+}+}++staticvoidvmemmap_remap_pte(pte_t*pte,unsignedlongaddr,+structvmemmap_remap_walk*walk)+{+/*+*Remapthetailpagesasread-onlytocatchillegalwriteoperation+*tothetailpages.+*/+pgprot_tpgprot=PAGE_KERNEL_RO;+pte_tentry=mk_pte(walk->reuse_page,pgprot);+structpage*page=pte_page(*pte);++list_add(&page->lru,walk->vmemmap_pages);+set_pte_at(&init_mm,addr,pte,entry);+}++/**+*vmemmap_remap_free-remapthevmemmapvirtualaddressrange[@start,@end)+*tothepagewhich@reuseismappedto,thenfreevmemmap+*whichtherangearemappedto.+*@start:startaddressofthevmemmapvirtualaddressrangethatwewant+*toremap.+*@end:endaddressofthevmemmapvirtualaddressrangethatwewantto+*remap.+*@reuse:reuseaddress.+*+*Note:Thisfunctiondependsonvmemmapbeingbasepagemapped.Pleasemake+*surethatwedisablePMDmappingofvmemmappageswhencallingthisfunction.+*/+voidvmemmap_remap_free(unsignedlongstart,unsignedlongend,+unsignedlongreuse)+{+LIST_HEAD(vmemmap_pages);+structvmemmap_remap_walkwalk={+.remap_pte=vmemmap_remap_pte,+.reuse_addr=reuse,+.vmemmap_pages=&vmemmap_pages,+};++/*+*Inordertomakeremappingroutinemostefficientforthehugepages,+*theroutineofvmemmappagetablewalkinghasthefollowingrules+*(seemoredetailsfromthevmemmap_pte_range()):+*+*-Therange[@start,@end)andtherange[@reuse,@reuse+PAGE_SIZE)+*shouldbecontinuous.+*-The@reuseaddressispartoftherange[@reuse,@end)thatweare+*walkingwhichispassedtovmemmap_remap_range().+*-The@reuseaddressisthefirstinthecompleterange.+*+*Soweneedtomakesurethat@startand@reusemeettheaboverules.+*/+BUG_ON(start-reuse!=PAGE_SIZE);++vmemmap_remap_range(reuse,end,&walk);+free_vmemmap_page_list(&vmemmap_pages);+}/**Allocateablockofmemorytobeusedtobackthevirtualmemorymap
From: Muchun Song <hidden> Date: 2021-02-19 10:54:00
The option HUGETLB_PAGE_FREE_VMEMMAP allows for the freeing of
some vmemmap pages associated with pre-allocated HugeTLB pages.
For example, on X86_64 6 vmemmap pages of size 4KB each can be
saved for each 2MB HugeTLB page. 4094 vmemmap pages of size 4KB
each can be saved for each 1GB HugeTLB page.
When a HugeTLB page is allocated or freed, the vmemmap array
representing the range associated with the page will need to be
remapped. When a page is allocated, vmemmap pages are freed
after remapping. When a page is freed, previously discarded
vmemmap pages must be allocated before remapping.
The config option is introduced early so that supporting code
can be written to depend on the option. The initial version of
the code only provides support for x86-64.
Like other code which frees vmemmap, this config option depends on
HAVE_BOOTMEM_INFO_NODE. The routine register_page_bootmem_info() is
used to register bootmem info. Therefore, make sure
register_page_bootmem_info is enabled if HUGETLB_PAGE_FREE_VMEMMAP
is defined.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Acked-by: Mike Kravetz <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/mm/init_64.c | 2 +-
fs/Kconfig | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
From: Muchun Song <hidden> Date: 2021-02-19 10:54:37
When we free a HugeTLB page to the buddy allocator, we should allocate
the vmemmap pages associated with it. But we may cannot allocate vmemmap
pages when the system is under memory pressure, in this case, we just
refuse to free the HugeTLB page instead of looping forever trying to
allocate the pages. This changes some behavior (list below) on some
corner cases.
1) Failing to free a huge page triggered by the user (decrease nr_pages).
Need try again later by the user.
2) Failing to free a surplus huge page when freed by the application.
Try again later when freeing a huge page next time.
3) Failing to dissolve a free huge page on ZONE_MOVABLE via
offline_pages().
This is a bit unfortunate if we have plenty of ZONE_MOVABLE memory
but are low on kernel memory. For example, migration of huge pages
would still work, however, dissolving the free page does not work.
This is a corner cases. When the system is that much under memory
pressure, offlining/unplug can be expected to fail.
4) Failing to dissolve a huge page on CMA/ZONE_MOVABLE via
alloc_contig_range() - once we have that handling in place. Mainly
affects CMA and virtio-mem.
Similar to 3). virito-mem will handle migration errors gracefully.
CMA might be able to fallback on other free areas within the CMA
region.
We do not want to use GFP_ATOMIC to allocate vmemmap pages. Because it
grants access to memory reserves and we do not think it is reasonable
to use memory reserves. We use GFP_KERNEL in alloc_huge_page_vmemmap().
Signed-off-by: Muchun Song <redacted>
---
Documentation/admin-guide/mm/hugetlbpage.rst | 8 +++
include/linux/mm.h | 2 +
mm/hugetlb.c | 81 ++++++++++++++++++++--------
mm/hugetlb_vmemmap.c | 22 ++++++++
mm/hugetlb_vmemmap.h | 6 +++
mm/sparse-vmemmap.c | 75 +++++++++++++++++++++++++-
6 files changed, 171 insertions(+), 23 deletions(-)
@@ -60,6 +60,10 @@ HugePages_Surp the pool above the value in ``/proc/sys/vm/nr_hugepages``. The maximum number of surplus huge pages is controlled by``/proc/sys/vm/nr_overcommit_hugepages``.+ Note: When the feature of freeing unused vmemmap pages associated+ with each hugetlb page is enabled, the number of the surplus huge+ pages may be temporarily larger than the maximum number of surplus+ huge pages when the system is under memory pressure. Hugepagesize is the default hugepage size (in Kb). Hugetlb
@@ -80,6 +84,10 @@ returned to the huge page pool when freed by a task. A user with root privileges can dynamically allocate more or free some persistent huge pages by increasing or decreasing the value of ``nr_hugepages``.+Note: When the feature of freeing unused vmemmap pages associated with each+hugetlb page is enabled, we can failed to free the huge pages triggered by+the user when ths system is under memory pressure. Please try again later.+ Pages that are used as huge pages are reserved inside the kernel and cannot be used for other purposes. Huge pages cannot be swapped out under memory pressure.
@@ -1403,9 +1434,9 @@ static void __free_huge_page(struct page *page)}elseif(h->surplus_huge_pages_node[nid]){/* remove the page from active list */list_del(&page->lru);-update_and_free_page(h,page);h->surplus_huge_pages--;h->surplus_huge_pages_node[nid]--;+update_and_free_page(h,page);}else{arch_clear_hugepage_flags(page);enqueue_huge_page(h,page);
@@ -237,6 +238,78 @@ void vmemmap_remap_free(unsigned long start, unsigned long end,free_vmemmap_page_list(&vmemmap_pages);}+staticvoidvmemmap_restore_pte(pte_t*pte,unsignedlongaddr,+structvmemmap_remap_walk*walk)+{+pgprot_tpgprot=PAGE_KERNEL;+structpage*page;+void*to;++BUG_ON(pte_page(*pte)!=walk->reuse_page);++page=list_first_entry(walk->vmemmap_pages,structpage,lru);+list_del(&page->lru);+to=page_to_virt(page);+copy_page(to,(void*)walk->reuse_addr);++set_pte_at(&init_mm,addr,pte,mk_pte(page,pgprot));+}++staticintalloc_vmemmap_page_list(unsignedlongstart,unsignedlongend,+gfp_tgfp_mask,structlist_head*list)+{+unsignedlongnr_pages=(end-start)>>PAGE_SHIFT;+intnid=page_to_nid((structpage*)start);+structpage*page,*next;++while(nr_pages--){+page=alloc_pages_node(nid,gfp_mask,0);+if(!page)+gotoout;+list_add_tail(&page->lru,list);+}++return0;+out:+list_for_each_entry_safe(page,next,list,lru)+__free_pages(page,0);+return-ENOMEM;+}++/**+*vmemmap_remap_alloc-remapthevmemmapvirtualaddressrange[@start,end)+*tothepagewhichisfromthe@vmemmap_pages+*respectively.+*@start:startaddressofthevmemmapvirtualaddressrangethatwewant+*toremap.+*@end:endaddressofthevmemmapvirtualaddressrangethatwewantto+*remap.+*@reuse:reuseaddress.+*@gpf_mask:GFPflagforallocatingvmemmappages.+*/+intvmemmap_remap_alloc(unsignedlongstart,unsignedlongend,+unsignedlongreuse,gfp_tgfp_mask)+{+LIST_HEAD(vmemmap_pages);+structvmemmap_remap_walkwalk={+.remap_pte=vmemmap_restore_pte,+.reuse_addr=reuse,+.vmemmap_pages=&vmemmap_pages,+};++/* See the comment in the vmemmap_remap_free(). */+BUG_ON(start-reuse!=PAGE_SIZE);++might_sleep_if(gfpflags_allow_blocking(gfp_mask));++if(alloc_vmemmap_page_list(start,end,gfp_mask,&vmemmap_pages))+return-ENOMEM;++vmemmap_remap_range(reuse,end,&walk);++return0;+}+/**Allocateablockofmemorytobeusedtobackthevirtualmemorymap*ortobackthepagetablesthatareusedtocreatethemapping.
From: Muchun Song <hidden> Date: 2021-02-19 10:55:25
Because we reuse the first tail vmemmap page frame and remap it
with read-only, we cannot set the PageHWPosion on some tail pages.
So we can use the head[4].private (There are at least 128 struct
page structures associated with the optimized HugeTLB page, so
using head[4].private is safe) to record the real error page index
and set the raw error page PageHWPoison later.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Acked-by: David Rientjes <rientjes@google.com>
---
mm/hugetlb.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++------
mm/hugetlb_vmemmap.c | 12 --------
mm/hugetlb_vmemmap.h | 17 +++++++++++
3 files changed, 91 insertions(+), 21 deletions(-)
From: Muchun Song <hidden> Date: 2021-02-19 10:55:52
All the infrastructure is ready, so we introduce nr_free_vmemmap_pages
field in the hstate to indicate how many vmemmap pages associated with
a HugeTLB page that can be freed to buddy allocator. And initialize it
in the hugetlb_vmemmap_init(). This patch is actual enablement of the
feature.
Signed-off-by: Muchun Song <redacted>
Acked-by: Mike Kravetz <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
---
include/linux/hugetlb.h | 3 +++
mm/hugetlb.c | 1 +
mm/hugetlb_vmemmap.c | 25 +++++++++++++++++++++++++
mm/hugetlb_vmemmap.h | 10 ++++++----
4 files changed, 35 insertions(+), 4 deletions(-)
From: Muchun Song <hidden> Date: 2021-02-19 10:56:16
For HugeTLB page, there are more metadata to save in the struct page.
But the head struct page cannot meet our needs, so we have to abuse
other tail struct page to store the metadata. In order to avoid
conflicts caused by subsequent use of more tail struct pages, we can
gather these discrete indexes of tail struct page. In this case, it
will be easier to add a new tail page index later.
There are only (RESERVE_VMEMMAP_SIZE / sizeof(struct page)) struct
page structs that can be used when CONFIG_HUGETLB_PAGE_FREE_VMEMMAP,
so add a BUILD_BUG_ON to catch invalid usage of the tail struct page.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
---
include/linux/hugetlb.h | 24 ++++++++++++++++++++++--
include/linux/hugetlb_cgroup.h | 19 +++++++++++--------
mm/hugetlb.c | 6 +++---
mm/hugetlb_vmemmap.c | 8 ++++++++
4 files changed, 44 insertions(+), 13 deletions(-)
From: Muchun Song <hidden> Date: 2021-02-19 10:56:26
Add a kernel parameter hugetlb_free_vmemmap to enable the feature of
freeing unused vmemmap pages associated with each hugetlb page on boot.
We disables PMD mapping of vmemmap pages for x86-64 arch when this
feature is enabled. Because vmemmap_remap_free() depends on vmemmap
being base page mapped.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Reviewed-by: Barry Song <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 14 ++++++++++++++
Documentation/admin-guide/mm/hugetlbpage.rst | 14 +++++++++-----
arch/x86/mm/init_64.c | 8 ++++++--
include/linux/hugetlb.h | 19 +++++++++++++++++++
mm/hugetlb_vmemmap.c | 24 ++++++++++++++++++++++++
5 files changed, 72 insertions(+), 7 deletions(-)
@@ -1577,6 +1577,20 @@ Documentation/admin-guide/mm/hugetlbpage.rst. Format: size[KMG]+ hugetlb_free_vmemmap=+ [KNL] When CONFIG_HUGETLB_PAGE_FREE_VMEMMAP is set,+ this controls freeing unused vmemmap pages associated+ with each HugeTLB page. When this option is enabled,+ we disable PMD/huge page mapping of vmemmap pages which+ increase page table pages. So if a user/sysadmin only+ uses a small number of HugeTLB pages (as a percentage+ of system memory), they could end up using more memory+ with hugetlb_free_vmemmap on as opposed to off.+ Format: { on | off (default) }++ on: enable the feature+ off: disable the feature+ hung_task_panic= [KNL] Should the hung task detector generate panics. Format: 0 | 1
@@ -60,8 +60,8 @@ HugePages_Surp the pool above the value in ``/proc/sys/vm/nr_hugepages``. The maximum number of surplus huge pages is controlled by``/proc/sys/vm/nr_overcommit_hugepages``.- Note: When the feature of freeing unused vmemmap pages associated- with each hugetlb page is enabled, the number of the surplus huge+ Note: When CONFIG_HUGETLB_PAGE_FREE_VMEMMAP and the kernel parameter+ of ``hugetlb_free_vmemmap=on`` are set, the number of the surplus huge pages may be temporarily larger than the maximum number of surplus huge pages when the system is under memory pressure. Hugepagesize
@@ -84,9 +84,10 @@ returned to the huge page pool when freed by a task. A user with root privileges can dynamically allocate more or free some persistent huge pages by increasing or decreasing the value of ``nr_hugepages``.-Note: When the feature of freeing unused vmemmap pages associated with each-hugetlb page is enabled, we can failed to free the huge pages triggered by-the user when ths system is under memory pressure. Please try again later.+Note: When CONFIG_HUGETLB_PAGE_FREE_VMEMMAP and the kernel parameter of+``hugetlb_free_vmemmap=on`` are set, we can failed to free the huge pages+triggered by the user when ths system is under memory pressure. Please+try again later. Pages that are used as huge pages are reserved inside the kernel and cannot be used for other purposes. Huge pages cannot be swapped out under
@@ -153,6 +154,9 @@ default_hugepagesz will all result in 256 2M huge pages being allocated. Valid default huge page size is architecture dependent.+hugetlb_free_vmemmap+ When CONFIG_HUGETLB_PAGE_FREE_VMEMMAP is set, this enables freeing+ unused vmemmap pages associated with each HugeTLB page. When multiple huge page sizes are supported, ``/proc/sys/vm/nr_hugepages`` indicates the current number of pre-allocated huge pages of the default size.
@@ -1557,7 +1558,8 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,{interr;-if(end-start<PAGES_PER_SECTION*sizeof(structpage))+if((is_hugetlb_free_vmemmap_enabled()&&!altmap)||+end-start<PAGES_PER_SECTION*sizeof(structpage))err=vmemmap_populate_basepages(start,end,node,NULL);elseif(boot_cpu_has(X86_FEATURE_PSE))err=vmemmap_populate_hugepages(start,end,node,altmap);
@@ -1585,6 +1587,8 @@ void register_page_bootmem_memmap(unsigned long section_nr,pmd_t*pmd;unsignedintnr_pmd_pages;structpage*page;+boolbase_mapping=!boot_cpu_has(X86_FEATURE_PSE)||+is_hugetlb_free_vmemmap_enabled();for(;addr<end;addr=next){pte_t*pte=NULL;
@@ -1610,7 +1614,7 @@ void register_page_bootmem_memmap(unsigned long section_nr,}get_page_bootmem(section_nr,pud_page(*pud),MIX_SECTION_INFO);-if(!boot_cpu_has(X86_FEATURE_PSE)){+if(base_mapping){next=(addr+PAGE_SIZE)&PAGE_MASK;pmd=pmd_offset(pud,addr);if(pmd_none(*pmd))
From: Muchun Song <hidden> Date: 2021-02-19 10:57:24
When the "struct page size" crosses page boundaries we cannot
make use of this feature. Let free_vmemmap_pages_per_hpage()
return zero if that is the case, most of the functions can be
optimized away.
Signed-off-by: Muchun Song <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
---
include/linux/hugetlb.h | 3 ++-
mm/hugetlb_vmemmap.c | 7 +++++++
mm/hugetlb_vmemmap.h | 6 ++++++
3 files changed, 15 insertions(+), 1 deletion(-)
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-19 14:13:17
On Fri 19-02-21 18:49:49, Muchun Song wrote:
When we free a HugeTLB page to the buddy allocator, we should allocate
the vmemmap pages associated with it. But we may cannot allocate vmemmap
pages when the system is under memory pressure, in this case, we just
refuse to free the HugeTLB page instead of looping forever trying to
allocate the pages. This changes some behavior (list below) on some
corner cases.
1) Failing to free a huge page triggered by the user (decrease nr_pages).
Need try again later by the user.
2) Failing to free a surplus huge page when freed by the application.
Try again later when freeing a huge page next time.
This means that surplus pages can accumulate right? This should be
rather unlikely because one released huge page could then be reused for
normal allocations - including vmemmap. Unlucky timing might still end
up in the accumulation though. Not something critical though.
3) Failing to dissolve a free huge page on ZONE_MOVABLE via
offline_pages().
This is a bit unfortunate if we have plenty of ZONE_MOVABLE memory
but are low on kernel memory. For example, migration of huge pages
would still work, however, dissolving the free page does not work.
This is a corner cases. When the system is that much under memory
pressure, offlining/unplug can be expected to fail.
Please mention that this is unfortunate because it prevents from the
memory offlining which shouldn't happen for movable zones. People
depending on the memory hotplug and movable zone should carefuly
consider whether savings on unmovable memory are worth losing their
hotplug functionality in some situations.
4) Failing to dissolve a huge page on CMA/ZONE_MOVABLE via
alloc_contig_range() - once we have that handling in place. Mainly
affects CMA and virtio-mem.
What about hugetlb page poisoning on HW failure (resp. soft offlining)?
Similar to 3). virito-mem will handle migration errors gracefully.
CMA might be able to fallback on other free areas within the CMA
region.
We do not want to use GFP_ATOMIC to allocate vmemmap pages. Because it
grants access to memory reserves and we do not think it is reasonable
to use memory reserves. We use GFP_KERNEL in alloc_huge_page_vmemmap().
This likely needs more context around. Maybe something like
"
Vmemmap pages are allocated from the page freeing context. In order for
those allocations to be not disruptive (e.g. trigger oom killer)
__GFP_NORETRY is used. hugetlb_lock is dropped for the allocation
because a non sleeping allocation would be too fragile and it could fail
too easily under memory pressure. GFP_ATOMIC or other modes to access
memory reserves is not used because we want to prevent consuming
reserves under heavy hugetlb freeing.
"
I haven't gone through the patch in a great detail yet, from a high
level POV it looks good although the counter changes and reshuffling
seems little wild. That requires a more detailed look I do not have time
for right now. Mike would be much better for that anywya ;)
I do not see any check for an atomic context in free_huge_page path. I
have suggested to replace in_task by in_atomic check (with a gotcha that
the later doesn't work without preempt_count but there is a work to
address that).
--
Michal Hocko
SUSE Labs
From: Muchun Song <hidden> Date: 2021-02-20 04:22:12
On Fri, Feb 19, 2021 at 10:12 PM Michal Hocko [off-list ref] wrote:
On Fri 19-02-21 18:49:49, Muchun Song wrote:
quoted
When we free a HugeTLB page to the buddy allocator, we should allocate
the vmemmap pages associated with it. But we may cannot allocate vmemmap
pages when the system is under memory pressure, in this case, we just
refuse to free the HugeTLB page instead of looping forever trying to
allocate the pages. This changes some behavior (list below) on some
corner cases.
1) Failing to free a huge page triggered by the user (decrease nr_pages).
Need try again later by the user.
2) Failing to free a surplus huge page when freed by the application.
Try again later when freeing a huge page next time.
This means that surplus pages can accumulate right? This should be
rather unlikely because one released huge page could then be reused for
normal allocations - including vmemmap. Unlucky timing might still end
up in the accumulation though. Not something critical though.
Agree.
quoted
3) Failing to dissolve a free huge page on ZONE_MOVABLE via
offline_pages().
This is a bit unfortunate if we have plenty of ZONE_MOVABLE memory
but are low on kernel memory. For example, migration of huge pages
would still work, however, dissolving the free page does not work.
This is a corner cases. When the system is that much under memory
pressure, offlining/unplug can be expected to fail.
Please mention that this is unfortunate because it prevents from the
memory offlining which shouldn't happen for movable zones. People
depending on the memory hotplug and movable zone should carefuly
consider whether savings on unmovable memory are worth losing their
hotplug functionality in some situations.
Make sense. I will mention this in the change log. Thanks.
quoted
4) Failing to dissolve a huge page on CMA/ZONE_MOVABLE via
alloc_contig_range() - once we have that handling in place. Mainly
affects CMA and virtio-mem.
What about hugetlb page poisoning on HW failure (resp. soft offlining)?
If the HW poisoned hugetlb page failed to be dissolved, the page
will go back to the free list with PG_HWPoison set. But the page
will not be used, because we will check whether the page is HW
poisoned when it is dequeued from the free list. If so, we will skip
this page.
quoted
Similar to 3). virito-mem will handle migration errors gracefully.
CMA might be able to fallback on other free areas within the CMA
region.
We do not want to use GFP_ATOMIC to allocate vmemmap pages. Because it
grants access to memory reserves and we do not think it is reasonable
to use memory reserves. We use GFP_KERNEL in alloc_huge_page_vmemmap().
This likely needs more context around. Maybe something like
"
Vmemmap pages are allocated from the page freeing context. In order for
those allocations to be not disruptive (e.g. trigger oom killer)
__GFP_NORETRY is used. hugetlb_lock is dropped for the allocation
because a non sleeping allocation would be too fragile and it could fail
too easily under memory pressure. GFP_ATOMIC or other modes to access
memory reserves is not used because we want to prevent consuming
reserves under heavy hugetlb freeing.
"
Thanks. I will use this to the change log.
I haven't gone through the patch in a great detail yet, from a high
level POV it looks good although the counter changes and reshuffling
seems little wild. That requires a more detailed look I do not have time
for right now. Mike would be much better for that anywya ;)
Yeah. Hope Mike will review this (I believe he is good at this area).
I do not see any check for an atomic context in free_huge_page path. I
have suggested to replace in_task by in_atomic check (with a gotcha that
the later doesn't work without preempt_count but there is a work to
address that).
Sorry. I forgot it. I will replace in_task with in_atomic in the next version.
Thanks for your suggestions.
From: Michal Hocko <mhocko@suse.com> Date: 2021-02-22 09:27:10
On Sat 20-02-21 12:20:36, Muchun Song wrote:
On Fri, Feb 19, 2021 at 10:12 PM Michal Hocko [off-list ref] wrote:
[...]
quoted
What about hugetlb page poisoning on HW failure (resp. soft offlining)?
If the HW poisoned hugetlb page failed to be dissolved, the page
will go back to the free list with PG_HWPoison set. But the page
will not be used, because we will check whether the page is HW
poisoned when it is dequeued from the free list. If so, we will skip
this page.
Can this lead to an underprovisioned pool then? Or is there a new
hugetlb allocated to replace the poisoned one?
--
Michal Hocko
SUSE Labs
From: Muchun Song <hidden> Date: 2021-02-22 10:32:32
On Mon, Feb 22, 2021 at 5:25 PM Michal Hocko [off-list ref] wrote:
On Sat 20-02-21 12:20:36, Muchun Song wrote:
quoted
On Fri, Feb 19, 2021 at 10:12 PM Michal Hocko [off-list ref] wrote:
[...]
quoted
quoted
What about hugetlb page poisoning on HW failure (resp. soft offlining)?
If the HW poisoned hugetlb page failed to be dissolved, the page
will go back to the free list with PG_HWPoison set. But the page
will not be used, because we will check whether the page is HW
poisoned when it is dequeued from the free list. If so, we will skip
this page.
Can this lead to an under provisioned pool then? Or is there a new
hugetlb allocated to replace the poisoned one?
Actually, no page will be allocated. Your concern is right. But without
this patch, the result does not change. e.g. The HW poisoned page
can fail to be dissolved when h->free_huge_pages is equal to
h->resv_huge_pages. But no one seems to have reported this issue so
far. Maybe this behavior needs improvement in the feature.
From: Oscar Salvador <osalvador@suse.de> Date: 2021-02-22 10:52:02
On Mon, Feb 22, 2021 at 06:31:12PM +0800, Muchun Song wrote:
On Mon, Feb 22, 2021 at 5:25 PM Michal Hocko [off-list ref] wrote:
quoted
On Sat 20-02-21 12:20:36, Muchun Song wrote:
quoted
On Fri, Feb 19, 2021 at 10:12 PM Michal Hocko [off-list ref] wrote:
[...]
quoted
quoted
What about hugetlb page poisoning on HW failure (resp. soft offlining)?
If the HW poisoned hugetlb page failed to be dissolved, the page
will go back to the free list with PG_HWPoison set. But the page
will not be used, because we will check whether the page is HW
poisoned when it is dequeued from the free list. If so, we will skip
this page.
Not really. If the huge page is dissolved, we will take the page out of the
the freelist. See take_page_off_buddy in memory_failure_hugetlb.
In an ideal world, we should inspect that page in free_pages_prepare(),
remove the HPWpoisoned page and process the others, without letting that
page hit Buddy.
And not only for hugetlb, but for any higher order page.
See how memory_failure() happily disengage itself when it finds a higher
order page.
It does it because we have the premise that once that page hits Buddy,
it will stay there as the check_new_page guards us.
But this has been proofed to be quite a weak measure, as compaction does
not performs such a check, and so the page can sneak in.
I fixed that for soft-offline, and for memory-failure in some cases, but more
needs to be done and is it in my TODO list.
quoted
Can this lead to an under provisioned pool then? Or is there a new
hugetlb allocated to replace the poisoned one?
Actually, no page will be allocated. Your concern is right. But without
this patch, the result does not change. e.g. The HW poisoned page
can fail to be dissolved when h->free_huge_pages is equal to
h->resv_huge_pages. But no one seems to have reported this issue so
far. Maybe this behavior needs improvement in the feature.
Yes, something to improve.
I shall have a look.
--
Oscar Salvador
SUSE L3
From: Mike Kravetz <hidden> Date: 2021-02-23 00:02:51
On 2/19/21 2:49 AM, Muchun Song wrote:
When we free a HugeTLB page to the buddy allocator, we should allocate
the vmemmap pages associated with it. But we may cannot allocate vmemmap
pages when the system is under memory pressure, in this case, we just
refuse to free the HugeTLB page instead of looping forever trying to
allocate the pages. This changes some behavior (list below) on some
corner cases.
Thank you for listing changes in behavior and possible side effects of
not being able to allocate vmemmmap and free huge page to buddy!
I will not repeat Michal's comment about the check for an atomic context
in free_huge_page path.
quoted hunk
1) Failing to free a huge page triggered by the user (decrease nr_pages).
Need try again later by the user.
2) Failing to free a surplus huge page when freed by the application.
Try again later when freeing a huge page next time.
3) Failing to dissolve a free huge page on ZONE_MOVABLE via
offline_pages().
This is a bit unfortunate if we have plenty of ZONE_MOVABLE memory
but are low on kernel memory. For example, migration of huge pages
would still work, however, dissolving the free page does not work.
This is a corner cases. When the system is that much under memory
pressure, offlining/unplug can be expected to fail.
4) Failing to dissolve a huge page on CMA/ZONE_MOVABLE via
alloc_contig_range() - once we have that handling in place. Mainly
affects CMA and virtio-mem.
Similar to 3). virito-mem will handle migration errors gracefully.
CMA might be able to fallback on other free areas within the CMA
region.
We do not want to use GFP_ATOMIC to allocate vmemmap pages. Because it
grants access to memory reserves and we do not think it is reasonable
to use memory reserves. We use GFP_KERNEL in alloc_huge_page_vmemmap().
Signed-off-by: Muchun Song <redacted>
---
Documentation/admin-guide/mm/hugetlbpage.rst | 8 +++
include/linux/mm.h | 2 +
mm/hugetlb.c | 81 ++++++++++++++++++++--------
mm/hugetlb_vmemmap.c | 22 ++++++++
mm/hugetlb_vmemmap.h | 6 +++
mm/sparse-vmemmap.c | 75 +++++++++++++++++++++++++-
6 files changed, 171 insertions(+), 23 deletions(-)
@@ -60,6 +60,10 @@ HugePages_Surp the pool above the value in ``/proc/sys/vm/nr_hugepages``. The maximum number of surplus huge pages is controlled by``/proc/sys/vm/nr_overcommit_hugepages``.+ Note: When the feature of freeing unused vmemmap pages associated+ with each hugetlb page is enabled, the number of the surplus huge
Small wording change:
with each hugetlb page is enabled, the number of surplus huge
quoted hunk
+ pages may be temporarily larger than the maximum number of surplus
+ huge pages when the system is under memory pressure.
Hugepagesize
is the default hugepage size (in Kb).
Hugetlb
@@ -80,6 +84,10 @@ returned to the huge page pool when freed by a task. A user with root privileges can dynamically allocate more or free some persistent huge pages by increasing or decreasing the value of ``nr_hugepages``.+Note: When the feature of freeing unused vmemmap pages associated with each+hugetlb page is enabled, we can failed to free the huge pages triggered by
Small wording change:
hugetlb page is enabled, we can fail to free the huge pages triggered by
quoted hunk
+the user when ths system is under memory pressure. Please try again later.
+
Pages that are used as huge pages are reserved inside the kernel and cannot
be used for other purposes. Huge pages cannot be swapped out under
memory pressure.
I think you added the set_page_refcounted() because the huge page will
appear as just a compound page without a reference after dropping the
hugetlb lock? It might be better to set the reference before modifying
the destructor. Otherwise, page scanning code could find the non-hugetlb
compound page with no reference. I could not find any code where this
would be a problem, but I think it would be safer to set the reference
first.
+ spin_unlock(&hugetlb_lock);
I really like the way this code is structured. It is much simpler than
previous versions with retries or workqueue. There is nothing wrong with
always dropping the lock here. However, I wonder if we should think about
optimizing for the case where this feature is not enabled and we are not
freeing a 1G huge page. I suspect this will be the most common case for
some time, and there is no need to drop the lock in this case.
Please do not change the code based on my comment. I just wanted to bring
this up for thought.
Is it as simple as checking?
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_unlock(&hugetlb_lock);
/* before return */
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_lock(&hugetlb_lock);
quoted hunk
+
+ if (alloc_huge_page_vmemmap(h, page)) {
+ int zeroed;
+
+ spin_lock(&hugetlb_lock);
+ INIT_LIST_HEAD(&page->lru);
+ set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
+ h->nr_huge_pages++;
+ h->nr_huge_pages_node[nid]++;
+
+ /*
+ * If we cannot allocate vmemmap pages, just refuse to free the
+ * page and put the page back on the hugetlb free list and treat
+ * as a surplus page.
+ */
+ h->surplus_huge_pages++;
+ h->surplus_huge_pages_node[nid]++;
+
+ /*
+ * This page is now managed by the hugetlb allocator and has
+ * no users -- drop the last reference.
+ */
+ zeroed = put_page_testzero(page);
+ VM_BUG_ON_PAGE(!zeroed, page);
+ arch_clear_hugepage_flags(page);
+ enqueue_huge_page(h, page);
+
+ return -ENOMEM;
+ }
+
for (i = 0; i < pages_per_huge_page(h); i++) {
page[i].flags &= ~(1 << PG_locked | 1 << PG_error |
1 << PG_referenced | 1 << PG_dirty |
1 << PG_active | 1 << PG_private |
1 << PG_writeback);
}
- VM_BUG_ON_PAGE(hugetlb_cgroup_from_page(page), page);
- VM_BUG_ON_PAGE(hugetlb_cgroup_from_page_rsvd(page), page);
- set_compound_page_dtor(page, NULL_COMPOUND_DTOR);
- set_page_refcounted(page);
if (hstate_is_gigantic(h)) {
- /*
- * Temporarily drop the hugetlb_lock, because
- * we might block in free_gigantic_page().
- */
- spin_unlock(&hugetlb_lock);
destroy_compound_gigantic_page(page, huge_page_order(h));
free_gigantic_page(page, huge_page_order(h));
- spin_lock(&hugetlb_lock);
} else {
__free_pages(page, huge_page_order(h));
}
+
+ spin_lock(&hugetlb_lock);
+
+ return 0;
}
struct hstate *size_to_hstate(unsigned long size)
@@ -1403,9 +1434,9 @@ static void __free_huge_page(struct page *page) } else if (h->surplus_huge_pages_node[nid]) { /* remove the page from active list */ list_del(&page->lru);- update_and_free_page(h, page); h->surplus_huge_pages--; h->surplus_huge_pages_node[nid]--;+ update_and_free_page(h, page); } else { arch_clear_hugepage_flags(page); enqueue_huge_page(h, page);
@@ -1700,8 +1732,7 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, h->surplus_huge_pages--; h->surplus_huge_pages_node[node]--; }- update_and_free_page(h, page);- ret = 1;+ ret = !update_and_free_page(h, page); break; } }
@@ -1714,10 +1745,14 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, * nothing for in-use hugepages and non-hugepages. * This function returns values like below: *- * -EBUSY: failed to dissolved free hugepages or the hugepage is in-use- * (allocated or reserved.)- * 0: successfully dissolved free hugepages or the page is not a- * hugepage (considered as already dissolved)+ * -ENOMEM: failed to allocate vmemmap pages to free the freed hugepages+ * when the system is under memory pressure and the feature of+ * freeing unused vmemmap pages associated with each hugetlb page+ * is enabled.+ * -EBUSY: failed to dissolved free hugepages or the hugepage is in-use+ * (allocated or reserved.)+ * 0: successfully dissolved free hugepages or the page is not a+ * hugepage (considered as already dissolved) */ int dissolve_free_huge_page(struct page *page) {
Since update_and_free_page failed, the number of surplus pages was
incremented. Surplus pages are the number of pages greater than
max_huge_pages. Since we are incrementing max_huge_pages, we should
decrement (undo) the addition to surplus_huge_pages and
surplus_huge_pages_node[nid]. So, I think we want
h->surplus_huge_pages--;
h->surplus_huge_pages_node[nid]--;
here as well.
}
out:
spin_unlock(&hugetlb_lock);
In previous version of this patch series, we discussed and refined the
vmemmap manipulation routines below. They still look good to me.
In general, I like the approach taken in this patch. Hopefully, others
will comment and we can move the series forward.
--
Mike Kravetz
@@ -237,6 +238,78 @@ void vmemmap_remap_free(unsigned long start, unsigned long end,free_vmemmap_page_list(&vmemmap_pages);}+staticvoidvmemmap_restore_pte(pte_t*pte,unsignedlongaddr,+structvmemmap_remap_walk*walk)+{+pgprot_tpgprot=PAGE_KERNEL;+structpage*page;+void*to;++BUG_ON(pte_page(*pte)!=walk->reuse_page);++page=list_first_entry(walk->vmemmap_pages,structpage,lru);+list_del(&page->lru);+to=page_to_virt(page);+copy_page(to,(void*)walk->reuse_addr);++set_pte_at(&init_mm,addr,pte,mk_pte(page,pgprot));+}++staticintalloc_vmemmap_page_list(unsignedlongstart,unsignedlongend,+gfp_tgfp_mask,structlist_head*list)+{+unsignedlongnr_pages=(end-start)>>PAGE_SHIFT;+intnid=page_to_nid((structpage*)start);+structpage*page,*next;++while(nr_pages--){+page=alloc_pages_node(nid,gfp_mask,0);+if(!page)+gotoout;+list_add_tail(&page->lru,list);+}++return0;+out:+list_for_each_entry_safe(page,next,list,lru)+__free_pages(page,0);+return-ENOMEM;+}++/**+*vmemmap_remap_alloc-remapthevmemmapvirtualaddressrange[@start,end)+*tothepagewhichisfromthe@vmemmap_pages+*respectively.+*@start:startaddressofthevmemmapvirtualaddressrangethatwewant+*toremap.+*@end:endaddressofthevmemmapvirtualaddressrangethatwewantto+*remap.+*@reuse:reuseaddress.+*@gpf_mask:GFPflagforallocatingvmemmappages.+*/+intvmemmap_remap_alloc(unsignedlongstart,unsignedlongend,+unsignedlongreuse,gfp_tgfp_mask)+{+LIST_HEAD(vmemmap_pages);+structvmemmap_remap_walkwalk={+.remap_pte=vmemmap_restore_pte,+.reuse_addr=reuse,+.vmemmap_pages=&vmemmap_pages,+};++/* See the comment in the vmemmap_remap_free(). */+BUG_ON(start-reuse!=PAGE_SIZE);++might_sleep_if(gfpflags_allow_blocking(gfp_mask));++if(alloc_vmemmap_page_list(start,end,gfp_mask,&vmemmap_pages))+return-ENOMEM;++vmemmap_remap_range(reuse,end,&walk);++return0;+}+/**Allocateablockofmemorytobeusedtobackthevirtualmemorymap*ortobackthepagetablesthatareusedtocreatethemapping.
From: Muchun Song <hidden> Date: 2021-02-23 05:37:16
On Tue, Feb 23, 2021 at 8:01 AM Mike Kravetz [off-list ref] wrote:
On 2/19/21 2:49 AM, Muchun Song wrote:
quoted
When we free a HugeTLB page to the buddy allocator, we should allocate
the vmemmap pages associated with it. But we may cannot allocate vmemmap
pages when the system is under memory pressure, in this case, we just
refuse to free the HugeTLB page instead of looping forever trying to
allocate the pages. This changes some behavior (list below) on some
corner cases.
Thank you for listing changes in behavior and possible side effects of
not being able to allocate vmemmmap and free huge page to buddy!
I will not repeat Michal's comment about the check for an atomic context
in free_huge_page path.
quoted
1) Failing to free a huge page triggered by the user (decrease nr_pages).
Need try again later by the user.
2) Failing to free a surplus huge page when freed by the application.
Try again later when freeing a huge page next time.
3) Failing to dissolve a free huge page on ZONE_MOVABLE via
offline_pages().
This is a bit unfortunate if we have plenty of ZONE_MOVABLE memory
but are low on kernel memory. For example, migration of huge pages
would still work, however, dissolving the free page does not work.
This is a corner cases. When the system is that much under memory
pressure, offlining/unplug can be expected to fail.
4) Failing to dissolve a huge page on CMA/ZONE_MOVABLE via
alloc_contig_range() - once we have that handling in place. Mainly
affects CMA and virtio-mem.
Similar to 3). virito-mem will handle migration errors gracefully.
CMA might be able to fallback on other free areas within the CMA
region.
We do not want to use GFP_ATOMIC to allocate vmemmap pages. Because it
grants access to memory reserves and we do not think it is reasonable
to use memory reserves. We use GFP_KERNEL in alloc_huge_page_vmemmap().
Signed-off-by: Muchun Song <redacted>
---
Documentation/admin-guide/mm/hugetlbpage.rst | 8 +++
include/linux/mm.h | 2 +
mm/hugetlb.c | 81 ++++++++++++++++++++--------
mm/hugetlb_vmemmap.c | 22 ++++++++
mm/hugetlb_vmemmap.h | 6 +++
mm/sparse-vmemmap.c | 75 +++++++++++++++++++++++++-
6 files changed, 171 insertions(+), 23 deletions(-)
@@ -60,6 +60,10 @@ HugePages_Surp the pool above the value in ``/proc/sys/vm/nr_hugepages``. The maximum number of surplus huge pages is controlled by``/proc/sys/vm/nr_overcommit_hugepages``.+ Note: When the feature of freeing unused vmemmap pages associated+ with each hugetlb page is enabled, the number of the surplus huge
Small wording change:
with each hugetlb page is enabled, the number of surplus huge
Thanks. I will update this.
quoted
+ pages may be temporarily larger than the maximum number of surplus
+ huge pages when the system is under memory pressure.
Hugepagesize
is the default hugepage size (in Kb).
Hugetlb
@@ -80,6 +84,10 @@ returned to the huge page pool when freed by a task. A user with root privileges can dynamically allocate more or free some persistent huge pages by increasing or decreasing the value of ``nr_hugepages``.+Note: When the feature of freeing unused vmemmap pages associated with each+hugetlb page is enabled, we can failed to free the huge pages triggered by
Small wording change:
hugetlb page is enabled, we can fail to free the huge pages triggered by
Thanks. I will update this.
quoted
+the user when ths system is under memory pressure. Please try again later.
+
Pages that are used as huge pages are reserved inside the kernel and cannot
be used for other purposes. Huge pages cannot be swapped out under
memory pressure.
I think you added the set_page_refcounted() because the huge page will
appear as just a compound page without a reference after dropping the
hugetlb lock?
Right.
It might be better to set the reference before modifying
the destructor. Otherwise, page scanning code could find the non-hugetlb
compound page with no reference. I could not find any code where this
would be a problem, but I think it would be safer to set the reference
first.
Make sense to me. It is better to set the refcount first.
quoted
+ spin_unlock(&hugetlb_lock);
I really like the way this code is structured. It is much simpler than
previous versions with retries or workqueue. There is nothing wrong with
always dropping the lock here. However, I wonder if we should think about
optimizing for the case where this feature is not enabled and we are not
freeing a 1G huge page. I suspect this will be the most common case for
some time, and there is no need to drop the lock in this case.
Please do not change the code based on my comment. I just wanted to bring
this up for thought.
At least make sense to me. It may take a long time to free a 1G
huge page. Dropping the lock may be a good choice. But I also
want to listen to Oscar and Michal's opinion on this.
Is it as simple as checking?
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_unlock(&hugetlb_lock);
/* before return */
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_lock(&hugetlb_lock);
quoted
+
+ if (alloc_huge_page_vmemmap(h, page)) {
+ int zeroed;
+
+ spin_lock(&hugetlb_lock);
+ INIT_LIST_HEAD(&page->lru);
+ set_compound_page_dtor(page, HUGETLB_PAGE_DTOR);
+ h->nr_huge_pages++;
+ h->nr_huge_pages_node[nid]++;
+
+ /*
+ * If we cannot allocate vmemmap pages, just refuse to free the
+ * page and put the page back on the hugetlb free list and treat
+ * as a surplus page.
+ */
+ h->surplus_huge_pages++;
+ h->surplus_huge_pages_node[nid]++;
+
+ /*
+ * This page is now managed by the hugetlb allocator and has
+ * no users -- drop the last reference.
+ */
+ zeroed = put_page_testzero(page);
+ VM_BUG_ON_PAGE(!zeroed, page);
+ arch_clear_hugepage_flags(page);
+ enqueue_huge_page(h, page);
+
+ return -ENOMEM;
+ }
+
for (i = 0; i < pages_per_huge_page(h); i++) {
page[i].flags &= ~(1 << PG_locked | 1 << PG_error |
1 << PG_referenced | 1 << PG_dirty |
1 << PG_active | 1 << PG_private |
1 << PG_writeback);
}
- VM_BUG_ON_PAGE(hugetlb_cgroup_from_page(page), page);
- VM_BUG_ON_PAGE(hugetlb_cgroup_from_page_rsvd(page), page);
- set_compound_page_dtor(page, NULL_COMPOUND_DTOR);
- set_page_refcounted(page);
if (hstate_is_gigantic(h)) {
- /*
- * Temporarily drop the hugetlb_lock, because
- * we might block in free_gigantic_page().
- */
- spin_unlock(&hugetlb_lock);
destroy_compound_gigantic_page(page, huge_page_order(h));
free_gigantic_page(page, huge_page_order(h));
- spin_lock(&hugetlb_lock);
} else {
__free_pages(page, huge_page_order(h));
}
+
+ spin_lock(&hugetlb_lock);
+
+ return 0;
}
struct hstate *size_to_hstate(unsigned long size)
@@ -1403,9 +1434,9 @@ static void __free_huge_page(struct page *page) } else if (h->surplus_huge_pages_node[nid]) { /* remove the page from active list */ list_del(&page->lru);- update_and_free_page(h, page); h->surplus_huge_pages--; h->surplus_huge_pages_node[nid]--;+ update_and_free_page(h, page); } else { arch_clear_hugepage_flags(page); enqueue_huge_page(h, page);
@@ -1700,8 +1732,7 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, h->surplus_huge_pages--; h->surplus_huge_pages_node[node]--; }- update_and_free_page(h, page);- ret = 1;+ ret = !update_and_free_page(h, page); break; } }
@@ -1714,10 +1745,14 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed, * nothing for in-use hugepages and non-hugepages. * This function returns values like below: *- * -EBUSY: failed to dissolved free hugepages or the hugepage is in-use- * (allocated or reserved.)- * 0: successfully dissolved free hugepages or the page is not a- * hugepage (considered as already dissolved)+ * -ENOMEM: failed to allocate vmemmap pages to free the freed hugepages+ * when the system is under memory pressure and the feature of+ * freeing unused vmemmap pages associated with each hugetlb page+ * is enabled.+ * -EBUSY: failed to dissolved free hugepages or the hugepage is in-use+ * (allocated or reserved.)+ * 0: successfully dissolved free hugepages or the page is not a+ * hugepage (considered as already dissolved) */ int dissolve_free_huge_page(struct page *page) {
Since update_and_free_page failed, the number of surplus pages was
incremented. Surplus pages are the number of pages greater than
max_huge_pages. Since we are incrementing max_huge_pages, we should
decrement (undo) the addition to surplus_huge_pages and
surplus_huge_pages_node[nid]. So, I think we want
h->surplus_huge_pages--;
h->surplus_huge_pages_node[nid]--;
here as well.
You are right. Thanks for reminding me of this.
quoted
}
out:
spin_unlock(&hugetlb_lock);
In previous version of this patch series, we discussed and refined the
vmemmap manipulation routines below. They still look good to me.
In general, I like the approach taken in this patch. Hopefully, others
will comment and we can move the series forward.
--
Mike Kravetz
@@ -237,6 +238,78 @@ void vmemmap_remap_free(unsigned long start, unsigned long end,free_vmemmap_page_list(&vmemmap_pages);}+staticvoidvmemmap_restore_pte(pte_t*pte,unsignedlongaddr,+structvmemmap_remap_walk*walk)+{+pgprot_tpgprot=PAGE_KERNEL;+structpage*page;+void*to;++BUG_ON(pte_page(*pte)!=walk->reuse_page);++page=list_first_entry(walk->vmemmap_pages,structpage,lru);+list_del(&page->lru);+to=page_to_virt(page);+copy_page(to,(void*)walk->reuse_addr);++set_pte_at(&init_mm,addr,pte,mk_pte(page,pgprot));+}++staticintalloc_vmemmap_page_list(unsignedlongstart,unsignedlongend,+gfp_tgfp_mask,structlist_head*list)+{+unsignedlongnr_pages=(end-start)>>PAGE_SHIFT;+intnid=page_to_nid((structpage*)start);+structpage*page,*next;++while(nr_pages--){+page=alloc_pages_node(nid,gfp_mask,0);+if(!page)+gotoout;+list_add_tail(&page->lru,list);+}++return0;+out:+list_for_each_entry_safe(page,next,list,lru)+__free_pages(page,0);+return-ENOMEM;+}++/**+*vmemmap_remap_alloc-remapthevmemmapvirtualaddressrange[@start,end)+*tothepagewhichisfromthe@vmemmap_pages+*respectively.+*@start:startaddressofthevmemmapvirtualaddressrangethatwewant+*toremap.+*@end:endaddressofthevmemmapvirtualaddressrangethatwewantto+*remap.+*@reuse:reuseaddress.+*@gpf_mask:GFPflagforallocatingvmemmappages.+*/+intvmemmap_remap_alloc(unsignedlongstart,unsignedlongend,+unsignedlongreuse,gfp_tgfp_mask)+{+LIST_HEAD(vmemmap_pages);+structvmemmap_remap_walkwalk={+.remap_pte=vmemmap_restore_pte,+.reuse_addr=reuse,+.vmemmap_pages=&vmemmap_pages,+};++/* See the comment in the vmemmap_remap_free(). */+BUG_ON(start-reuse!=PAGE_SIZE);++might_sleep_if(gfpflags_allow_blocking(gfp_mask));++if(alloc_vmemmap_page_list(start,end,gfp_mask,&vmemmap_pages))+return-ENOMEM;++vmemmap_remap_range(reuse,end,&walk);++return0;+}+/**Allocateablockofmemorytobeusedtobackthevirtualmemorymap*ortobackthepagetablesthatareusedtocreatethemapping.
I think you added the set_page_refcounted() because the huge page will
appear as just a compound page without a reference after dropping the
hugetlb lock? It might be better to set the reference before modifying
the destructor. Otherwise, page scanning code could find the non-hugetlb
compound page with no reference. I could not find any code where this
would be a problem, but I think it would be safer to set the reference
first.
But we already had set_page_refcounted() before this patchset there.
Are the worries only because we drop the lock? AFAICS, the "page-scanning"
problem could have happened before as well?
Although, what does page scanning mean in this context?
I am not opposed to move it above, but I would like to understand the concern
here.
quoted
+ spin_unlock(&hugetlb_lock);
I really like the way this code is structured. It is much simpler than
previous versions with retries or workqueue. There is nothing wrong with
always dropping the lock here. However, I wonder if we should think about
optimizing for the case where this feature is not enabled and we are not
freeing a 1G huge page. I suspect this will be the most common case for
some time, and there is no need to drop the lock in this case.
Please do not change the code based on my comment. I just wanted to bring
this up for thought.
Is it as simple as checking?
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_unlock(&hugetlb_lock);
/* before return */
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_lock(&hugetlb_lock);
AFAIK, we at least need the hstate_is_gigantic? Comment below says that
free_gigantic_page might block, so we need to drop the lock.
And I am fine with the change overall.
Unless I am missing something, we should not need to drop the lock unless
we need to allocate vmemmap pages (apart from gigantic pages).
+
+ /*
+ * If we cannot allocate vmemmap pages, just refuse to free the
+ * page and put the page back on the hugetlb free list and treat
+ * as a surplus page.
+ */
+ h->surplus_huge_pages++;
+ h->surplus_huge_pages_node[nid]++;
+
+ /*
+ * This page is now managed by the hugetlb allocator and has
+ * no users -- drop the last reference.
+ */
+ zeroed = put_page_testzero(page);
+ VM_BUG_ON_PAGE(!zeroed, page);
Can this actually happen? AFAIK, page landed in update_and_free_page should be
zero refcounted, then we increase the reference, and I cannot see how the
reference might have changed in the meantime.
I am all for catching corner cases, but not sure how realistic this is.
Moreover, if we __ever__ get there, things can get nasty.
We basically will have an in-use page in the free hugetlb pool, so corruption
will happen. At that point, a plain BUG_ON might be better.
But as I said, I do not think we need that.
I yet need to look further, but what I have seen so far looks good.
--
Oscar Salvador
SUSE L3
I think you added the set_page_refcounted() because the huge page will
appear as just a compound page without a reference after dropping the
hugetlb lock? It might be better to set the reference before modifying
the destructor. Otherwise, page scanning code could find the non-hugetlb
compound page with no reference. I could not find any code where this
would be a problem, but I think it would be safer to set the reference
first.
But we already had set_page_refcounted() before this patchset there.
Are the worries only because we drop the lock? AFAICS, the "page-scanning"
problem could have happened before as well?
Although, what does page scanning mean in this context?
I am not opposed to move it above, but I would like to understand the concern
here.
quoted
quoted
+ spin_unlock(&hugetlb_lock);
I really like the way this code is structured. It is much simpler than
previous versions with retries or workqueue. There is nothing wrong with
always dropping the lock here. However, I wonder if we should think about
optimizing for the case where this feature is not enabled and we are not
freeing a 1G huge page. I suspect this will be the most common case for
some time, and there is no need to drop the lock in this case.
Please do not change the code based on my comment. I just wanted to bring
this up for thought.
Is it as simple as checking?
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_unlock(&hugetlb_lock);
/* before return */
if (free_vmemmap_pages_per_hpage(h) || hstate_is_gigantic(h))
spin_lock(&hugetlb_lock);
AFAIK, we at least need the hstate_is_gigantic? Comment below says that
free_gigantic_page might block, so we need to drop the lock.
And I am fine with the change overall.
Unless I am missing something, we should not need to drop the lock unless
we need to allocate vmemmap pages (apart from gigantic pages).
Actually, there are some differences. e.g. prep_new_huge_page()
will reset hugetlb cgroup and ClearHPageFreed, but we do not need
them here. And prep_new_huge_page will acquire and release
the hugetlb_lock. But here we also need hold the lock to update
the surplus counter and enqueue the page to the free list.
So I do not think reuse prep_new_huge_page is a good idea.
quoted
quoted
+
+ /*
+ * If we cannot allocate vmemmap pages, just refuse to free the
+ * page and put the page back on the hugetlb free list and treat
+ * as a surplus page.
+ */
+ h->surplus_huge_pages++;
+ h->surplus_huge_pages_node[nid]++;
+
+ /*
+ * This page is now managed by the hugetlb allocator and has
+ * no users -- drop the last reference.
+ */
+ zeroed = put_page_testzero(page);
+ VM_BUG_ON_PAGE(!zeroed, page);
Can this actually happen? AFAIK, page landed in update_and_free_page should be
zero refcounted, then we increase the reference, and I cannot see how the
reference might have changed in the meantime.
I am not sure whether other modules get the page and then put the
page. I see gather_surplus_pages does the same thing. So I copied
from there. I try to look at the memory_failure routine.
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
Maybe this can happen. But it is a very corner case. If we want to
deal with this. We can put_page_testzero() first and then
set_compound_page_dtor(HUGETLB_PAGE_DTOR).
I am all for catching corner cases, but not sure how realistic this is.
Moreover, if we __ever__ get there, things can get nasty.
We basically will have an in-use page in the free hugetlb pool, so corruption
will happen. At that point, a plain BUG_ON might be better.
But as I said, I do not think we need that.
I yet need to look further, but what I have seen so far looks good.
--
Oscar Salvador
SUSE L3
Actually, there are some differences. e.g. prep_new_huge_page()
will reset hugetlb cgroup and ClearHPageFreed, but we do not need
them here. And prep_new_huge_page will acquire and release
the hugetlb_lock. But here we also need hold the lock to update
the surplus counter and enqueue the page to the free list.
So I do not think reuse prep_new_huge_page is a good idea.
I see, I missed that.
quoted
Can this actually happen? AFAIK, page landed in update_and_free_page should be
zero refcounted, then we increase the reference, and I cannot see how the
reference might have changed in the meantime.
I am not sure whether other modules get the page and then put the
page. I see gather_surplus_pages does the same thing. So I copied
from there. I try to look at the memory_failure routine.
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
Maybe this can happen. But it is a very corner case. If we want to
deal with this. We can put_page_testzero() first and then
set_compound_page_dtor(HUGETLB_PAGE_DTOR).
I have to check further, but it looks like this could actually happen.
Handling this with VM_BUG_ON is wrong, because memory_failure/soft_offline are
entitled to increase the refcount of the page.
AFAICS,
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
identify_page_state
me_huge_page
I think we can reach me_huge_page with either refcount = 1 or refcount =2,
depending whether put_page_testzero has been issued.
For now, I would not re-enqueue the page if put_page_testzero == false.
I have to see how this can be handled gracefully.
--
Oscar Salvador
SUSE L3
From: Oscar Salvador <osalvador@suse.de> Date: 2021-02-23 15:42:16
On Tue, Feb 23, 2021 at 11:50:05AM +0100, Oscar Salvador wrote:
quoted
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
Maybe this can happen. But it is a very corner case. If we want to
deal with this. We can put_page_testzero() first and then
set_compound_page_dtor(HUGETLB_PAGE_DTOR).
I have to check further, but it looks like this could actually happen.
Handling this with VM_BUG_ON is wrong, because memory_failure/soft_offline are
entitled to increase the refcount of the page.
AFAICS,
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
identify_page_state
me_huge_page
I think we can reach me_huge_page with either refcount = 1 or refcount =2,
depending whether put_page_testzero has been issued.
For now, I would not re-enqueue the page if put_page_testzero == false.
I have to see how this can be handled gracefully.
I took a brief look.
It is not really your patch fault. Hugetlb <-> memory-failure synchronization is
a bit odd, it definitely needs improvment.
The thing is, we can have different scenarios here.
E.g: by the time we return from put_page_testzero, we might have refcount ==
0 and PageHWPoison, or refcount == 1 PageHWPoison.
The former will let a user get a page from the pool and get a sigbus
when it faults in the page, and the latter will be even more odd as we
will have a self-refcounted page in the free pool (and hwpoisoned).
As I said, it is not this patchset fault. I just made me realize this
problem.
I have to think some more about this.
--
Oscar Salvador
SUSE L3
From: Oscar Salvador <osalvador@suse.de> Date: 2021-02-23 22:35:59
On Tue, Feb 23, 2021 at 04:41:28PM +0100, Oscar Salvador wrote:
On Tue, Feb 23, 2021 at 11:50:05AM +0100, Oscar Salvador wrote:
quoted
quoted
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
Maybe this can happen. But it is a very corner case. If we want to
deal with this. We can put_page_testzero() first and then
set_compound_page_dtor(HUGETLB_PAGE_DTOR).
I have to check further, but it looks like this could actually happen.
Handling this with VM_BUG_ON is wrong, because memory_failure/soft_offline are
entitled to increase the refcount of the page.
AFAICS,
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
identify_page_state
me_huge_page
I think we can reach me_huge_page with either refcount = 1 or refcount =2,
depending whether put_page_testzero has been issued.
For now, I would not re-enqueue the page if put_page_testzero == false.
I have to see how this can be handled gracefully.
I took a brief look.
It is not really your patch fault. Hugetlb <-> memory-failure synchronization is
a bit odd, it definitely needs improvment.
The thing is, we can have different scenarios here.
E.g: by the time we return from put_page_testzero, we might have refcount ==
0 and PageHWPoison, or refcount == 1 PageHWPoison.
The former will let a user get a page from the pool and get a sigbus
when it faults in the page, and the latter will be even more odd as we
will have a self-refcounted page in the free pool (and hwpoisoned).
As I said, it is not this patchset fault. I just made me realize this
problem.
I have to think some more about this.
I have been thinking more about this.
memory failure events can occur at any time, and we might not be in a
position where we can handle gracefully the error, meaning that the page
might end up in non desirable state.
E.g: we could flag the page right before enqueing it.
I still think that VM_BUG_ON should go, as the refcount can be perfectly
increased by memory-failure/soft_offline handlers, so BUGing there does
not make much sense.
One think we could do is to check the state of the page we want to
retrieve from the free hugepage pool.
We should discard any HWpoisoned ones, and dissolve them.
The thing is, memory-failure/soft_offline should allocate a new hugepage
for the free pool, so keep the pool stable.
Something like [1].
Anyway, this is orthogonal to this patch, and something I will work on
soon.
[1] https://lore.kernel.org/linux-mm/20210222135137.25717-2-osalvador@suse.de/T/#u
--
Oscar Salvador
SUSE L3
From: Muchun Song <hidden> Date: 2021-02-24 03:49:31
On Wed, Feb 24, 2021 at 6:32 AM Oscar Salvador [off-list ref] wrote:
On Tue, Feb 23, 2021 at 04:41:28PM +0100, Oscar Salvador wrote:
quoted
On Tue, Feb 23, 2021 at 11:50:05AM +0100, Oscar Salvador wrote:
quoted
quoted
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
Maybe this can happen. But it is a very corner case. If we want to
deal with this. We can put_page_testzero() first and then
set_compound_page_dtor(HUGETLB_PAGE_DTOR).
I have to check further, but it looks like this could actually happen.
Handling this with VM_BUG_ON is wrong, because memory_failure/soft_offline are
entitled to increase the refcount of the page.
AFAICS,
CPU0: CPU1:
set_compound_page_dtor(HUGETLB_PAGE_DTOR);
memory_failure_hugetlb
get_hwpoison_page
__get_hwpoison_page
get_page_unless_zero
put_page_testzero()
identify_page_state
me_huge_page
I think we can reach me_huge_page with either refcount = 1 or refcount =2,
depending whether put_page_testzero has been issued.
For now, I would not re-enqueue the page if put_page_testzero == false.
I have to see how this can be handled gracefully.
I took a brief look.
It is not really your patch fault. Hugetlb <-> memory-failure synchronization is
a bit odd, it definitely needs improvment.
The thing is, we can have different scenarios here.
E.g: by the time we return from put_page_testzero, we might have refcount ==
0 and PageHWPoison, or refcount == 1 PageHWPoison.
The former will let a user get a page from the pool and get a sigbus
when it faults in the page, and the latter will be even more odd as we
will have a self-refcounted page in the free pool (and hwpoisoned).
I have been looking at the dequeue_huge_page_node_exact().
If a PageHWPoison huge page is in the free pool list, the page will
not be allocated to the user. The PageHWPoison huge page
will be skip in the dequeue_huge_page_node_exact().
quoted
As I said, it is not this patchset fault. I just made me realize this
problem.
I have to think some more about this.
I have been thinking more about this.
memory failure events can occur at any time, and we might not be in a
position where we can handle gracefully the error, meaning that the page
might end up in non desirable state.
E.g: we could flag the page right before enqueing it.
I still think that VM_BUG_ON should go, as the refcount can be perfectly
increased by memory-failure/soft_offline handlers, so BUGing there does
not make much sense.
Make sense. I will remove the VM_BUG_ON.
One think we could do is to check the state of the page we want to
retrieve from the free hugepage pool.
We should discard any HWpoisoned ones, and dissolve them.
The thing is, memory-failure/soft_offline should allocate a new hugepage
for the free pool, so keep the pool stable.
Something like [1].
Anyway, this is orthogonal to this patch, and something I will work on
soon.
[1] https://lore.kernel.org/linux-mm/20210222135137.25717-2-osalvador@suse.de/T/#u
From: Oscar Salvador <osalvador@suse.de> Date: 2021-02-24 08:33:13
On Wed, Feb 24, 2021 at 11:47:49AM +0800, Muchun Song wrote:
I have been looking at the dequeue_huge_page_node_exact().
If a PageHWPoison huge page is in the free pool list, the page will
not be allocated to the user. The PageHWPoison huge page
will be skip in the dequeue_huge_page_node_exact().
Yes, now I see where the problem lies.
hugetlb_no_page()->..->dequeue_huge_page_node_exact() will fail if the only
page in the pool is hwpoisoned, as expected.
Then alloc_buddy_huge_page_with_mpol() will be tried, but since surplus_huge_pages
counter is stale, we will fail there.
That relates to the problem Mike pointed out, that we should decrease again the
surplus_huge_pages.
I think hwpoisoned pages should not be in the free pool though.
Probably we want to take them off when we notice we have one:
e.g: dequeue_huge_page_node_exact could place the page in another list
and place it back in case it was unpoisoned.
But anyway, that has nothing to do with this (apart from the surplus problem).
--
Oscar Salvador
SUSE L3