Fix CONT-PTE/PMD size hugetlb issue when unmapping or migrating
Subsystems (11)
v3 cover Fix CONT-PTE/PMD size hugetlb issue when unmapping or migrating
Hi,
Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll use ptep_clear_flush() and set_pte_at() to nuke the page table entry and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb page, which will cause potential data consistent issue. This patch set will change to use hugetlb related APIs to fix this issue, please find details in each patch. Thanks.
Note: Mike pointed out the huge_ptep_get() will only return the one specific value, and it would not take into account the dirty or young bits of CONT-PTE/PMDs like the huge_ptep_get_and_clear() [1]. This inconsistent issue is not introduced by this patch set, and will address this issue in another thread [2]. Meanwhile the uffd for hugetlb case [3] pointed by Gerald also need another patch to address.
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/linux-mm/20220503120343.6264e126@thinkpad/
Changes from v2:
- Collect reviewed tags from Muchun and Mike.
- Drop the unnecessary casting in hugetlb.c.
- Fix building errors with adding dummy functions for !CONFIG_HUGETLB_PAGE.
Changes from v1:
- Add acked tag from Mike.
- Update some commit message.
- Add VM_BUG_ON in try_to_unmap() for hugetlb case.
- Add an explict void casting for huge_ptep_clear_flush() in hugetlb.c.
Baolin Wang (3):
mm: change huge_ptep_clear_flush() to return the original pte
mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when unmapping
arch/arm64/include/asm/hugetlb.h | 4 +-- arch/arm64/mm/hugetlbpage.c | 12 +++----- arch/ia64/include/asm/hugetlb.h | 4 +-- arch/mips/include/asm/hugetlb.h | 9 ++++-- arch/parisc/include/asm/hugetlb.h | 4 +-- arch/powerpc/include/asm/hugetlb.h | 9 ++++-- arch/s390/include/asm/hugetlb.h | 6 ++-- arch/sh/include/asm/hugetlb.h | 4 +-- arch/sparc/include/asm/hugetlb.h | 4 +-- include/asm-generic/hugetlb.h | 4 +-- include/linux/hugetlb.h | 11 +++++++ mm/rmap.c | 63 ++++++++++++++++++++++++-------------- 12 files changed, 83 insertions(+), 51 deletions(-)
Signature · 1 line
1.8.3.1
PATCH v3 1/3 mm: change huge_ptep_clear_flush() to return the original pte
It is incorrect to use ptep_clear_flush() to nuke a hugetlb page table when unmapping or migrating a hugetlb page, and will change to use huge_ptep_clear_flush() instead in the following patches.
So this is a preparation patch, which changes the huge_ptep_clear_flush() to return the original pte to help to nuke a hugetlb page table.
--- arch/arm64/include/asm/hugetlb.h | 4 ++-- arch/arm64/mm/hugetlbpage.c | 12 +++++------- arch/ia64/include/asm/hugetlb.h | 4 ++-- arch/mips/include/asm/hugetlb.h | 9 ++++++--- arch/parisc/include/asm/hugetlb.h | 4 ++-- arch/powerpc/include/asm/hugetlb.h | 9 ++++++--- arch/s390/include/asm/hugetlb.h | 6 +++--- arch/sh/include/asm/hugetlb.h | 4 ++-- arch/sparc/include/asm/hugetlb.h | 4 ++-- include/asm-generic/hugetlb.h | 4 ++-- 10 files changed, 32 insertions(+), 28 deletions(-)
arch/arm64/include/asm/hugetlb.h +2 -2
@@ -39,8 +39,8 @@ extern pte_t huge_ptep_get_and_clear(struct mm_struct *mm,extern void huge_ptep_set_wrprotect (struct mm_struct *mm,unsigned long addr,pte_t *ptep);#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -extern void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep); +extern pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep);#define __HAVE_ARCH_HUGE_PTE_CLEARextern void huge_pte_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep,unsigned long sz);
arch/arm64/mm/hugetlbpage.c +5 -7
@@ -486,19 +486,17 @@ void huge_ptep_set_wrprotect(struct mm_struct *mm,set_pte_at (mm, addr, ptep,pfn_pte (pfn, hugeprot)); } -void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) {size_t pgsize;int ncontig; -if (!pte_cont (READ_ONCE (*ptep))) { -ptep_clear_flush (vma, addr, ptep); -return ; - } +if (!pte_cont (READ_ONCE (*ptep))) +return ptep_clear_flush (vma, addr, ptep); ncontig =find_num_contig (vma->vm_mm, addr, ptep, &pgsize); -clear_flush (vma->vm_mm, addr, ptep, pgsize, ncontig); +return get_clear_flush (vma->vm_mm, addr, ptep, pgsize, ncontig); }static int __init hugetlbpage_init (void )
arch/ia64/include/asm/hugetlb.h +2 -2
@@ -23,8 +23,8 @@ static inline int is_hugepage_only_range(struct mm_struct *mm,#define is_hugepage_only_range is_hugepage_only_range#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { }
arch/mips/include/asm/hugetlb.h +6 -3
@@ -43,16 +43,19 @@ static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm, }#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { +pte_t pte; +/* * clear the huge pte entry firstly, so that the other smp threads will * not get old pte entry after finishing flush_tlb_page and before * setting new huge pte entry */ -huge_ptep_get_and_clear (vma->vm_mm, addr, ptep); + pte =huge_ptep_get_and_clear (vma->vm_mm, addr, ptep);flush_tlb_page (vma, addr); +return pte; }#define __HAVE_ARCH_HUGE_PTE_NONE
arch/parisc/include/asm/hugetlb.h +2 -2
@@ -28,8 +28,8 @@ static inline int prepare_hugepage_range(struct file *file, }#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { }
arch/powerpc/include/asm/hugetlb.h +6 -3
@@ -43,11 +43,14 @@ static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm, }#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { -huge_ptep_get_and_clear (vma->vm_mm, addr, ptep); +pte_t pte; + + pte =huge_ptep_get_and_clear (vma->vm_mm, addr, ptep);flush_hugetlb_page (vma, addr); +return pte; }#define __HAVE_ARCH_HUGE_PTEP_SET_ACCESS_FLAGS
arch/s390/include/asm/hugetlb.h +3 -3
@@ -50,10 +50,10 @@ static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,set_pte (ptep,__pte (_SEGMENT_ENTRY_EMPTY)); } -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long address,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long address,pte_t *ptep) { -huge_ptep_get_and_clear (vma->vm_mm, address, ptep); +return huge_ptep_get_and_clear (vma->vm_mm, address, ptep); } static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
arch/sh/include/asm/hugetlb.h +2 -2
@@ -21,8 +21,8 @@ static inline int prepare_hugepage_range(struct file *file, }#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { }
arch/sparc/include/asm/hugetlb.h +2 -2
@@ -21,8 +21,8 @@ pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep);#define __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) { }
include/asm-generic/hugetlb.h +2 -2
@@ -84,10 +84,10 @@ static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,#endif #ifndef __HAVE_ARCH_HUGE_PTEP_CLEAR_FLUSH -static inline void huge_ptep_clear_flush (struct vm_area_struct *vma, +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) { -ptep_clear_flush (vma, addr, ptep); +return ptep_clear_flush (vma, addr, ptep); }#endif
Signature · 1 line
1.8.3.1
PATCH v3 2/3 mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
In reply to Baolin WangOn some architectures (like ARM64), it can support CONT-PTE/PMD size hugetlb, which means it can support not only PMD/PUD size hugetlb:
2M and 1G, but also CONT-PTE/PMD size: 64K and 32M if a 4K page size specified.
When migrating a hugetlb page, we will get the relevant page table entry by huge_pte_offset() only once to nuke it and remap it with a migration pte entry. This is correct for PMD or PUD size hugetlb, since they always contain only one pmd entry or pud entry in the page table.
However this is incorrect for CONT-PTE and CONT-PMD size hugetlb, since they can contain several continuous pte or pmd entry with same page table attributes. So we will nuke or remap only one pte or pmd entry for this CONT-PTE/PMD size hugetlb page, which is not expected for hugetlb migration. The problem is we can still continue to modify the subpages' data of a hugetlb page during migrating a hugetlb page, which can cause a serious data consistent issue, since we did not nuke the page table entry and set a migration pte for the subpages of a hugetlb page.
To fix this issue, we should change to use huge_ptep_clear_flush() to nuke a hugetlb page table, and remap it with set_huge_pte_at() and set_huge_swap_pte_at() when migrating a hugetlb page, which already considered the CONT-PTE or CONT-PMD size hugetlb.
--- include/linux/hugetlb.h | 11 +++++++++++ mm/rmap.c | 24 ++++++++++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-)
include/linux/hugetlb.h +11 -0
@@ -1093,6 +1093,17 @@ static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr pte_t *ptep, pte_t pte, unsigned long sz) { } + +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) +{ +return ptep_get (ptep); +} + +static inline void set_huge_pte_at (struct mm_struct *mm,unsigned long addr, +pte_t *ptep,pte_t pte) +{ +}#endif /* CONFIG_HUGETLB_PAGE */ static inline spinlock_t *huge_pte_lock(struct hstate *h,
mm/rmap.c +18 -6
@@ -1926,13 +1926,15 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,break ; } } + +/* Nuke the hugetlb page table entry */ + pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); }else {flush_cache_page (vma, address,pte_pfn (*pvmw.pte)); +/* Nuke the page table entry. */ + pteval =ptep_clear_flush (vma, address, pvmw.pte); } -/* Nuke the page table entry. */ - pteval =ptep_clear_flush (vma, address, pvmw.pte); -/* Set the dirty flag on the folio now the pte is gone. */ if (pte_dirty (pteval))folio_mark_dirty (folio);
@@ -2017,7 +2019,10 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,pte_t swp_pte;if (arch_unmap_one (mm, vma, address, pteval) <0 ) { -set_pte_at (mm, address, pvmw.pte, pteval); +if (folio_test_hugetlb (folio)) +set_huge_pte_at (mm, address, pvmw.pte, pteval); +else +set_pte_at (mm, address, pvmw.pte, pteval); ret = false;page_vma_mapped_walk_done (&pvmw);break ;
@@ -2026,7 +2031,10 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma, !anon_exclusive, subpage);if (anon_exclusive &&page_try_share_anon_rmap (subpage)) { -set_pte_at (mm, address, pvmw.pte, pteval); +if (folio_test_hugetlb (folio)) +set_huge_pte_at (mm, address, pvmw.pte, pteval); +else +set_pte_at (mm, address, pvmw.pte, pteval); ret = false;page_vma_mapped_walk_done (&pvmw);break ;
@@ -2052,7 +2060,11 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma, swp_pte =pte_swp_mksoft_dirty (swp_pte);if (pte_uffd_wp (pteval)) swp_pte =pte_swp_mkuffd_wp (swp_pte); -set_pte_at (mm, address, pvmw.pte, swp_pte); +if (folio_test_hugetlb (folio)) +set_huge_swap_pte_at (mm, address, pvmw.pte, + swp_pte,vma_mmu_pagesize (vma)); +else +set_pte_at (mm, address, pvmw.pte, swp_pte);trace_set_migration_pte (address,pte_val (swp_pte),compound_order (&folio->page));/*
Signature · 1 line
1.8.3.1
Andrew Morton
maintainerRe: [PATCH v3 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
Quoted 37 lines · On Tue, 10 May 2022 11:45:59 +0800 Baolin Wang <[email protected]>…
On some architectures (like ARM64), it can support CONT-PTE/PMD size hugetlb, which means it can support not only PMD/PUD size hugetlb:
2M and 1G, but also CONT-PTE/PMD size: 64K and 32M if a 4K page size specified.When migrating a hugetlb page, we will get the relevant page table entry by
huge_pte_offset()only once to nuke it and remap it with a migration pte entry. This is correct for PMD or PUD size hugetlb, since they always contain only one pmd entry or pud entry in the page table.However this is incorrect for CONT-PTE and CONT-PMD size hugetlb, since they can contain several continuous pte or pmd entry with same page table attributes. So we will nuke or remap only one pte or pmd entry for this CONT-PTE/PMD size hugetlb page, which is not expected for hugetlb migration. The problem is we can still continue to modify the subpages' data of a hugetlb page during migrating a hugetlb page, which can cause a serious data consistent issue, since we did not nuke the page table entry and set a migration pte for the subpages of a hugetlb page.
To fix this issue, we should change to use
huge_ptep_clear_flush()to nuke a hugetlb page table, and remap it withset_huge_pte_at()andset_huge_swap_pte_at()when migrating a hugetlb page, which already considered the CONT-PTE or CONT-PMD size hugetlb....
include/linux/hugetlb.h +3
-0@@ -1093,6 +1093,17 @@ static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr pte_t *ptep, pte_t pte, unsigned long sz) { } + +static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep)
include/linux/hugetlb.h static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr · line 1108
Quoted 10 lines
include/linux/hugetlb.h +8
-0+{ +return ptep_get (ptep); +} + +static inline void set_huge_pte_at (struct mm_struct *mm,unsigned long addr, +pte_t *ptep,pte_t pte) +{ +}#endif /* CONFIG_HUGETLB_PAGE */
This blows up nommu (arm allnoconfig):
In file included from fs/io_uring.c:71:./include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':./include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
1100 | return ptep_get(ptep);
| ^~~~~~~~huge_ptep_clear_flush() is only used in CONFIG_NOMMU=n files, so I simply zapped this change.
include/linux/hugetlb.h renamed from include/linux/hugetlb.h~mm-rmap-fix-cont-pte-pmd-size-hugetlb-issue-when-migration-fix +0 -11
@@ -1093,17 +1093,6 @@ static inline void set_huge_swap_pte_at( pte_t *ptep, pte_t pte, unsigned long sz) { } - -static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma, -unsigned long addr,pte_t *ptep) -{ -return ptep_get (ptep); -} - -static inline void set_huge_pte_at (struct mm_struct *mm,unsigned long addr, -pte_t *ptep,pte_t pte) -{ -}#endif /* CONFIG_HUGETLB_PAGE */ static inline spinlock_t *huge_pte_lock(struct hstate *h,
_
Andrew Morton
maintainerRe: [PATCH v3 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
Quoted 19 lines · On Tue, 10 May 2022 16:17:39 -0700 Andrew Morton <[email protected]>…
Review
Quoted 13 lines
(quoted diff) +11
-0+ +static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, + unsigned long addr, pte_t *ptep) +{ + return ptep_get(ptep); +} + +static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, pte_t pte) +{ +} #endif /* CONFIG_HUGETLB_PAGE */This blows up nommu (arm allnoconfig):
In file included from
fs/io_uring.c:71:./include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':./include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
Quoted 7 lines
1100 | return ptep_get(ptep); | ^~~~~~~~
huge_ptep_clear_flush()is only used inCONFIG_NOMMU=nfiles, so I simply zapped this change.
Well that wasn't a great success. Doing this instead. It's pretty nasty - something nicer would be nicer please.
include/linux/hugetlb.h renamed from include/linux/hugetlb.h~mm-rmap-fix-cont-pte-pmd-size-hugetlb-issue-when-migration-fix +2 -0
@@ -1094,6 +1094,7 @@ static inline void set_huge_swap_pte_at( { } +#ifdef CONFIG_MMU static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {
@@ -1104,6 +1105,7 @@ static inline void set_huge_pte_at(struc pte_t *ptep, pte_t pte) { } +#endif #endif /* CONFIG_HUGETLB_PAGE */ static inline spinlock_t *huge_pte_lock(struct hstate *h,
_
Re: [PATCH v3 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
Quoted 25 lines · Andrew Morton
Quoted 23 lines · On Tue, 10 May 2022 16:17:39 -0700 Andrew Morton <[email protected]>…
Review
Quoted 13 lines
(quoted diff) +11
-0+ +static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma, + unsigned long addr, pte_t *ptep) +{ + return ptep_get(ptep); +} + +static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, pte_t pte) +{ +} #endif /* CONFIG_HUGETLB_PAGE */This blows up nommu (arm allnoconfig):
In file included from
fs/io_uring.c:71:./include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':./include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]1100 | return ptep_get(ptep); | ^~~~~~~~
Quoted 6 lines
Quoted 3 lines
huge_ptep_clear_flush()is only used inCONFIG_NOMMU=nfiles, so I simply zapped this change.Well that wasn't a great success. Doing this instead. It's pretty nasty - something nicer would be nicer please.
Thanks for fixing the building issue. I'll look at this to simplify the dummy function. Myabe just remove the ptep_get().
include/linux/hugetlb.h +1 -1
@@ -1097,7 +1097,7 @@ static inline void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addrstatic inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) { -return ptep_get (ptep); +return *ptep; }
Quoted 20 lines
include/linux/hugetlb.h renamed from include/linux/hugetlb.h~mm-rmap-fix-cont-pte-pmd-size-hugetlb-issue-when-migration-fix +2
-0@@ -1094,6 +1094,7 @@ static inline void set_huge_swap_pte_at( { } +#ifdef CONFIG_MMU static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep) {@@ -1104,6 +1105,7 @@ static inline void set_huge_pte_at(struc pte_t *ptep, pte_t pte) { } +#endif #endif /* CONFIG_HUGETLB_PAGE */ static inline spinlock_t *huge_pte_lock(struct hstate *h,_
kernel test robot
build botRe: [PATCH v3 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
In reply to Baolin WangI love your patch! Yet something to improve:
Hi Baolin,
I love your patch! Yet something to improve:
[auto build test ERROR on akpm-mm/mm-everything]
[cannot apply to hnaz-mm/master arm64/for-next/core linus/master v5.18-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Baolin-Wang/Fix-CONT-PTE-PMD-size-hugetlb-issue-when-unmapping-or-migrating/20220510-114753 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything config: h8300-buildonly-randconfig-r001-20220509 (https://download.01.org/0day-ci/archive/20220511/[email protected]/config) compiler: h8300-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/b666792b4c5f9774c350977ff88837bafc36365a git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Baolin-Wang/Fix-CONT-PTE-PMD-size-hugetlb-issue-when-unmapping-or-migrating/20220510-114753 git checkout b666792b4c5f9774c350977ff88837bafc36365a # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=h8300 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>):
In file included from kernel/fork.c:51:
include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':
>> include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
1100 | return ptep_get(ptep);
| ^~~~~~~~
>> include/linux/hugetlb.h:1100:16: error: incompatible types when returning type 'int' but 'pte_t' was expected
1100 | return ptep_get(ptep);
| ^~~~~~~~~~~~~~
kernel/fork.c: At top level:
kernel/fork.c:162:13: warning: no previous prototype for 'arch_release_task_struct' [-Wmissing-prototypes]
162 | void __weak arch_release_task_struct(struct task_struct *tsk)
| ^~~~~~~~~~~~~~~~~~~~~~~~
kernel/fork.c:847:20: warning: no previous prototype for 'arch_task_cache_init' [-Wmissing-prototypes]
847 | void __init __weak arch_task_cache_init(void) { }
| ^~~~~~~~~~~~~~~~~~~~ kernel/fork.c:942:12: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes]
942 | int __weak arch_dup_task_struct(struct task_struct *dst,
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from kernel/sysctl.c:46:
include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':
>> include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
1100 | return ptep_get(ptep);
| ^~~~~~~~
>> include/linux/hugetlb.h:1100:16: error: incompatible types when returning type 'int' but 'pte_t' was expected
1100 | return ptep_get(ptep);
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/linux/migrate.h:8,
from mm/page_alloc.c:62:
include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':
>> include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
1100 | return ptep_get(ptep);
| ^~~~~~~~
>> include/linux/hugetlb.h:1100:16: error: incompatible types when returning type 'int' but 'pte_t' was expected
1100 | return ptep_get(ptep);
| ^~~~~~~~~~~~~~
In file included from include/asm-generic/bug.h:5,
from arch/h8300/include/asm/bug.h:8,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:6,
from mm/page_alloc.c:19:
mm/page_alloc.c: In function 'free_pages':
include/asm-generic/page.h:89:51: warning: ordered comparison of pointer with null pointer [-Wextra]
89 | #define virt_addr_valid(kaddr) (((void *)(kaddr) >= (void *)PAGE_OFFSET) && \
| ^~
include/linux/compiler.h:78:45: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/mmdebug.h:17:25: note: in expansion of macro 'BUG_ON'
17 | #define VM_BUG_ON(cond) BUG_ON(cond)
| ^~~~~~
mm/page_alloc.c:5489:17: note: in expansion of macro 'VM_BUG_ON'
5489 | VM_BUG_ON(!virt_addr_valid((void *)addr));
| ^~~~~~~~~
mm/page_alloc.c:5489:28: note: in expansion of macro 'virt_addr_valid'
5489 | VM_BUG_ON(!virt_addr_valid((void *)addr));
| ^~~~~~~~~~~~~~~ cc1: some warnings being treated as errors
--
In file included from fs/io_uring.c:71:
include/linux/hugetlb.h: In function 'huge_ptep_clear_flush':
>> include/linux/hugetlb.h:1100:16: error: implicit declaration of function 'ptep_get' [-Werror=implicit-function-declaration]
1100 | return ptep_get(ptep);
| ^~~~~~~~
>> include/linux/hugetlb.h:1100:16: error: incompatible types when returning type 'int' but 'pte_t' was expected
1100 | return ptep_get(ptep);
| ^~~~~~~~~~~~~~
fs/io_uring.c: In function '__io_submit_flush_completions':
fs/io_uring.c:2660:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
2660 | struct io_wq_work_node *node, *prev;
| ^~~~cc1: some warnings being treated as errors
vim +/ptep_get +1100 include/linux/hugetlb.h
1096
1097 static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
1098 unsigned long addr, pte_t *ptep)
1099 {
> 1100 return ptep_get(ptep);
1101 }
1102
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
Andrew Morton
maintainerRe: [PATCH v3 2/3] mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when migration
Quoted 25 lines · On Wed, 11 May 2022 09:19:17 +0800 kernel test robot <[email protected]> wrote:
Hi Baolin,
I love your patch! Yet something to improve:
[auto build test ERROR on akpm-mm/mm-everything]
[cannot apply to hnaz-mm/master arm64/for-next/core linus/master v5.18-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]url: https://github.com/intel-lab-lkp/linux/commits/Baolin-Wang/Fix-CONT-PTE-PMD-size-hugetlb-issue-when-unmapping-or-migrating/20220510-114753 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything config: h8300-buildonly-randconfig-r001-20220509 (https://download.01.org/0day-ci/archive/20220511/[email protected]/config) compiler: h8300-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/b666792b4c5f9774c350977ff88837bafc36365a git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Baolin-Wang/Fix-CONT-PTE-PMD-size-hugetlb-issue-when-unmapping-or-migrating/20220510-114753 git checkout b666792b4c5f9774c350977ff88837bafc36365a # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=h8300 SHELL=/bin/bash
Quoted 1 line
...
Thanks.
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-rmap-fix-cont-pte-pmd-size-hugetlb-issue-when-migration-fix.patch
(which is in current mm-everything) fixes this up.
PATCH v3 3/3 mm: rmap: Fix CONT-PTE/PMD size hugetlb issue when unmapping
In reply to Baolin WangOn some architectures (like ARM64), it can support CONT-PTE/PMD size hugetlb, which means it can support not only PMD/PUD size hugetlb:
2M and 1G, but also CONT-PTE/PMD size: 64K and 32M if a 4K page size specified.
When unmapping a hugetlb page, we will get the relevant page table entry by huge_pte_offset() only once to nuke it. This is correct for PMD or PUD size hugetlb, since they always contain only one pmd entry or pud entry in the page table.
However this is incorrect for CONT-PTE and CONT-PMD size hugetlb, since they can contain several continuous pte or pmd entry with same page table attributes, so we will nuke only one pte or pmd entry for this CONT-PTE/PMD size hugetlb page.
And now try_to_unmap() is only passed a hugetlb page in the case where the hugetlb page is poisoned. Which means now we will unmap only one pte entry for a CONT-PTE or CONT-PMD size poisoned hugetlb page, and we can still access other subpages of a CONT-PTE or CONT-PMD size poisoned hugetlb page, which will cause serious issues possibly.
So we should change to use huge_ptep_clear_flush() to nuke the hugetlb page table to fix this issue, which already considered CONT-PTE and CONT-PMD size hugetlb.
We've already used set_huge_swap_pte_at() to set a poisoned swap entry for a poisoned hugetlb page. Meanwhile adding a VM_BUG_ON() to make sure the passed hugetlb page is poisoned in try_to_unmap().
--- mm/rmap.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-)
mm/rmap.c +22 -17
@@ -1528,6 +1528,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,if (folio_test_hugetlb (folio)) {/* +* The try_to_unmap() is only passed a hugetlb page +* in the case where the hugetlb page is poisoned. +*/ +VM_BUG_ON_PAGE (!PageHWPoison (subpage), subpage); +/* * huge_pmd_unshare may unmap an entire PMD page. * There is no way of knowing exactly which PMDs may * be cached for this mm, so we must flush them all.
@@ -1562,28 +1567,28 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,break ; } } + pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); }else {flush_cache_page (vma, address,pte_pfn (*pvmw.pte)); - } - -/* -* Nuke the page table entry. When having to clear -* PageAnonExclusive(), we always have to flush. -*/ -if (should_defer_flush (mm, flags) && !anon_exclusive) {/* -* We clear the PTE but do not flush so potentially -* a remote CPU could still be writing to the folio. -* If the entry was previously clean then the -* architecture must guarantee that a clear->dirty -* transition on a cached TLB entry is written through -* and traps if the PTE is unmapped. +* Nuke the page table entry. When having to clear +* PageAnonExclusive(), we always have to flush. */ - pteval =ptep_get_and_clear (mm, address, pvmw.pte); +if (should_defer_flush (mm, flags) && !anon_exclusive) { +/* +* We clear the PTE but do not flush so potentially +* a remote CPU could still be writing to the folio. +* If the entry was previously clean then the +* architecture must guarantee that a clear->dirty +* transition on a cached TLB entry is written through +* and traps if the PTE is unmapped. +*/ + pteval =ptep_get_and_clear (mm, address, pvmw.pte); -set_tlb_ubc_flush_pending (mm,pte_dirty (pteval)); - }else { - pteval =ptep_clear_flush (vma, address, pvmw.pte); +set_tlb_ubc_flush_pending (mm,pte_dirty (pteval)); + }else { + pteval =ptep_clear_flush (vma, address, pvmw.pte); + } }/*
Signature · 1 line
1.8.3.1
Andrew Morton
maintainerQuoted 6 lines · On Tue, 10 May 2022 11:45:57 +0800 Baolin Wang <[email protected]>…
Hi,
Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll use
ptep_clear_flush()andset_pte_at()to nuke the page table entry and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb page,
It would be helpful to describe why it's wrong. Something like "should use huge_ptep_clear_flush() and huge_ptep_clear_flush() for this purpose"?
Quoted 3 lines
which will cause potential data consistent issue. This patch set will change to use hugetlb related APIs to fix this issue, please find details in each patch. Thanks.
Is a cc:stable needed here? And are we able to identify a target for a
Fixes: tag?
Quoted 4 lines · Andrew Morton
Quoted 2 lines · On Tue, 10 May 2022 11:45:57 +0800 Baolin Wang <[email protected]>…
Hi,
Quoted 8 lines
Quoted 4 lines
Now migrating a hugetlb page or unmapping a poisoned hugetlb page, we'll use
ptep_clear_flush()andset_pte_at()to nuke the page table entry and remap it, and this is incorrect for CONT-PTE or CONT-PMD size hugetlb page,It would be helpful to describe why it's wrong. Something like "should use
huge_ptep_clear_flush()andhuge_ptep_clear_flush()for this purpose"?
Sorry for the confusing description. I described the problem explicitly in each patch's commit message.
https://lore.kernel.org/all/ea5abf529f0997b5430961012bfda6166c1efc8c.1652147571.git.baolin.wang@linux.alibaba.com/
https://lore.kernel.org/all/730ea4b6d292f32fb10b7a4e87dad49b0eb30474.1652147571.git.baolin.wang@linux.alibaba.com/
Quoted 7 lines
Quoted 4 lines
which will cause potential data consistent issue. This patch set will change to use hugetlb related APIs to fix this issue, please find details in each patch. Thanks.
Is a cc:stable needed here? And are we able to identify a target for a
Fixes: tag?
I think need a cc:stable tag, however I am not sure the target fixes tag, since we should trace back to the introduction of CONT-PTE/PMD hugetlb? 66b3923a1a0f ("arm64: hugetlb: add support for PTE contiguous bit")