| Message ID | 20170522133604.11392-5-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 22, 2017 at 3:36 PM, Punit Agrawal <punit.agrawal@arm.com> wrote: > diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h > index 99b490b4d05a..3138e126f43b 100644 > --- a/include/asm-generic/hugetlb.h > +++ b/include/asm-generic/hugetlb.h > @@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) > return pte_modify(pte, newprot); > } > > -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, > - pte_t *ptep) > -{ > - pte_clear(mm, addr, ptep); > -} > +void huge_pte_clear(struct mm_struct *mm, unsigned long addr, > + pte_t *ptep, unsigned long sz); > > #endif /* _ASM_GENERIC_HUGETLB_H */ > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 0e4d1fb3122f..2b0f6f96f2c1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > return ret; > } > > +void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr, > + pte_t *ptep, unsigned long sz) > +{ > + pte_clear(mm, addr, ptep); > +} > + > void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, > unsigned long start, unsigned long end, > struct page *ref_page) I don't really like how this moves the inline version from asm-generic into a __weak function here. I think it would be better to either stop using asm-generic/hugetlb.h on s390, or enclose the generic definition in #ifndef huge_pte_clear and then override by defining a macro in s390 as we do in other files in asm-generic. Arnd
Arnd Bergmann <arnd@arndb.de> writes: > On Mon, May 22, 2017 at 3:36 PM, Punit Agrawal <punit.agrawal@arm.com> wrote: >> diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h >> index 99b490b4d05a..3138e126f43b 100644 >> --- a/include/asm-generic/hugetlb.h >> +++ b/include/asm-generic/hugetlb.h >> @@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) >> return pte_modify(pte, newprot); >> } >> >> -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, >> - pte_t *ptep) >> -{ >> - pte_clear(mm, addr, ptep); >> -} >> +void huge_pte_clear(struct mm_struct *mm, unsigned long addr, >> + pte_t *ptep, unsigned long sz); >> >> #endif /* _ASM_GENERIC_HUGETLB_H */ >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 0e4d1fb3122f..2b0f6f96f2c1 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, >> return ret; >> } >> >> +void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr, >> + pte_t *ptep, unsigned long sz) >> +{ >> + pte_clear(mm, addr, ptep); >> +} >> + >> void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, >> unsigned long start, unsigned long end, >> struct page *ref_page) > > I don't really like how this moves the inline version from asm-generic into > a __weak function here. I think it would be better to either stop > using asm-generic/hugetlb.h > on s390, or enclose the generic definition in > > #ifndef huge_pte_clear > > and then override by defining a macro in s390 as we do in other files > in asm-generic. Nice! I wasn't aware asm-generic follows this as a standard pattern. s390 doesn't use asm-generic, but I needed to update the prototype with an additional parameter (size) and needlessly moved the function. I'll update the patch. The changes is needed to enable contiguous pte hugepage support on arm64 [0]. Thanks for taking a look. Punit [0] https://www.spinics.net/lists/arm-kernel/msg582758.html
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h index cd546a245c68..aa8489c07f24 100644 --- a/arch/s390/include/asm/hugetlb.h +++ b/arch/s390/include/asm/hugetlb.h @@ -38,14 +38,8 @@ static inline int prepare_hugepage_range(struct file *file, #define arch_clear_hugepage_flags(page) do { } while (0) -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, - pte_t *ptep) -{ - if ((pte_val(*ptep) & _REGION_ENTRY_TYPE_MASK) == _REGION_ENTRY_TYPE_R3) - pte_val(*ptep) = _REGION3_ENTRY_EMPTY; - else - pte_val(*ptep) = _SEGMENT_ENTRY_EMPTY; -} +void huge_pte_clear(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, unsigned long sz); static inline void huge_ptep_clear_flush(struct vm_area_struct *vma, unsigned long address, pte_t *ptep) diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c index ae23afc18493..48e19b324017 100644 --- a/arch/s390/mm/hugetlbpage.c +++ b/arch/s390/mm/hugetlbpage.c @@ -144,6 +144,15 @@ pte_t huge_ptep_get(pte_t *ptep) return __rste_to_pte(pte_val(*ptep)); } +void huge_pte_clear(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, unsigned long sz) +{ + if ((pte_val(*ptep) & _REGION_ENTRY_TYPE_MASK) == _REGION_ENTRY_TYPE_R3) + pte_val(*ptep) = _REGION3_ENTRY_EMPTY; + else + pte_val(*ptep) = _SEGMENT_ENTRY_EMPTY; +} + pte_t huge_ptep_get_and_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep) { diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h index 99b490b4d05a..3138e126f43b 100644 --- a/include/asm-generic/hugetlb.h +++ b/include/asm-generic/hugetlb.h @@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) return pte_modify(pte, newprot); } -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, - pte_t *ptep) -{ - pte_clear(mm, addr, ptep); -} +void huge_pte_clear(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, unsigned long sz); #endif /* _ASM_GENERIC_HUGETLB_H */ diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 0e4d1fb3122f..2b0f6f96f2c1 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, return ret; } +void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr, + pte_t *ptep, unsigned long sz) +{ + pte_clear(mm, addr, ptep); +} + void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, unsigned long start, unsigned long end, struct page *ref_page) @@ -3338,7 +3344,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma, * unmapped and its refcount is dropped, so just clear pte here. */ if (unlikely(!pte_present(pte))) { - huge_pte_clear(mm, address, ptep); + huge_pte_clear(mm, address, ptep, sz); spin_unlock(ptl); continue; }
When unmapping a hugepage range, huge_pte_clear() is used to clear the page table entries that are marked as not present. huge_pte_clear() internally just ends up calling pte_clear() which does not correctly deal with hugepages consisting of contiguous page table entries. Add a size argument to address this issue and allow architectures to override huge_pte_clear() in subsequent patches by making it a weak function. Update the s390 to use the new mechanism to override huge_pte_clear(). Note that the change only affects huge_pte_clear() - the other generic hugetlb functions don't need any change. Signed-off-by: Punit Agrawal <punit.agrawal@arm.com> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> Cc: Mike Kravetz <mike.kravetz@oracle.com> --- arch/s390/include/asm/hugetlb.h | 10 ++-------- arch/s390/mm/hugetlbpage.c | 9 +++++++++ include/asm-generic/hugetlb.h | 7 ++----- mm/hugetlb.c | 8 +++++++- 4 files changed, 20 insertions(+), 14 deletions(-)