| Message ID | 20210211000322.159437-2-mike.kravetz@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add hugetlb soft dirty support | expand |
On Wed, Feb 10, 2021 at 04:03:18PM -0800, Mike Kravetz wrote: > Add interfaces to set and clear soft dirty in hugetlb ptes. Make > hugetlb interfaces needed for /proc clear_refs available outside > hugetlb.c. > > arch/s390 has it's own version of most routines in asm-generic/hugetlb.h, > so add new routines there as well. > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > --- > arch/s390/include/asm/hugetlb.h | 30 ++++++++++++++++++++++++++++++ > include/asm-generic/hugetlb.h | 30 ++++++++++++++++++++++++++++++ > include/linux/hugetlb.h | 1 + > mm/hugetlb.c | 10 +--------- > 4 files changed, 62 insertions(+), 9 deletions(-) > > diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h > index 60f9241e5e4a..b7d26248fb1c 100644 > --- a/arch/s390/include/asm/hugetlb.h > +++ b/arch/s390/include/asm/hugetlb.h > @@ -105,6 +105,11 @@ static inline pte_t huge_pte_mkdirty(pte_t pte) > return pte_mkdirty(pte); > } > > +static inline pte_t huge_pte_mkyoung(pte_t pte) > +{ > + return pte_mkyoung(pte); > +} > + > static inline pte_t huge_pte_wrprotect(pte_t pte) > { > return pte_wrprotect(pte); > @@ -115,9 +120,34 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) > return pte_modify(pte, newprot); > } > > +static inline bool huge_pte_soft_dirty(pte_t pte) > +{ > + return pte_soft_dirty(pte); > +} > + > +static inline pte_t huge_pte_clear_soft_dirty(pte_t pte) > +{ > + return pte_clear_soft_dirty(pte); > +} > + > +static inline pte_t huge_pte_swp_clear_soft_dirty(pte_t pte) > +{ > + return pte_swp_clear_soft_dirty(pte); > +} > + Indeed asm/hugetlb.h of s390 didn't include asm-generic/hugetlb.h as what was normally done by asm/hugetlb.h of other archs. Do you know why it's special? E.g. huge_pte_wrprotect() of s390 version is actually the same of the default version. When I looked at the huge_pte_wrprotect() I also see that there seems to have no real user of __HAVE_ARCH_HUGE_PTE_WRPROTECT. Not sure whether it can be dropped. My gut feeling is that s390 should also include asm-generic/hugetlb.h but only redefine the helper only if necessary, since I see no point defining the same helper multiple times. > static inline bool gigantic_page_runtime_supported(void) > { > return true; > } > > +#if !defined(__HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE) && !defined(MODULE) > +#include <asm/tlbflush.h> > + > +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, > + unsigned long start, unsigned long end) > +{ > + flush_tlb_range(vma, start, end); > +} > +#endif Similar question here, only ppc defined __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE, so IIUC it means s390 should simply use the default version, and it'll be great if we don't need to redefine it here. Thanks,
On 2/17/21 8:24 AM, Peter Xu wrote: > On Wed, Feb 10, 2021 at 04:03:18PM -0800, Mike Kravetz wrote: >> Add interfaces to set and clear soft dirty in hugetlb ptes. Make >> hugetlb interfaces needed for /proc clear_refs available outside >> hugetlb.c. >> >> arch/s390 has it's own version of most routines in asm-generic/hugetlb.h, >> so add new routines there as well. >> >> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> >> --- >> arch/s390/include/asm/hugetlb.h | 30 ++++++++++++++++++++++++++++++ >> include/asm-generic/hugetlb.h | 30 ++++++++++++++++++++++++++++++ >> include/linux/hugetlb.h | 1 + >> mm/hugetlb.c | 10 +--------- >> 4 files changed, 62 insertions(+), 9 deletions(-) >> >> diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h >> index 60f9241e5e4a..b7d26248fb1c 100644 >> --- a/arch/s390/include/asm/hugetlb.h >> +++ b/arch/s390/include/asm/hugetlb.h >> @@ -105,6 +105,11 @@ static inline pte_t huge_pte_mkdirty(pte_t pte) >> return pte_mkdirty(pte); >> } >> >> +static inline pte_t huge_pte_mkyoung(pte_t pte) >> +{ >> + return pte_mkyoung(pte); >> +} >> + >> static inline pte_t huge_pte_wrprotect(pte_t pte) >> { >> return pte_wrprotect(pte); >> @@ -115,9 +120,34 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) >> return pte_modify(pte, newprot); >> } >> >> +static inline bool huge_pte_soft_dirty(pte_t pte) >> +{ >> + return pte_soft_dirty(pte); >> +} >> + >> +static inline pte_t huge_pte_clear_soft_dirty(pte_t pte) >> +{ >> + return pte_clear_soft_dirty(pte); >> +} >> + >> +static inline pte_t huge_pte_swp_clear_soft_dirty(pte_t pte) >> +{ >> + return pte_swp_clear_soft_dirty(pte); >> +} >> + > > Indeed asm/hugetlb.h of s390 didn't include asm-generic/hugetlb.h as what was > normally done by asm/hugetlb.h of other archs. Do you know why it's special? > E.g. huge_pte_wrprotect() of s390 version is actually the same of the default > version. > > When I looked at the huge_pte_wrprotect() I also see that there seems to have > no real user of __HAVE_ARCH_HUGE_PTE_WRPROTECT. Not sure whether it can be > dropped. My gut feeling is that s390 should also include asm-generic/hugetlb.h > but only redefine the helper only if necessary, since I see no point defining > the same helper multiple times. I do not know why s390 is special in this way. However, I did cc some s390 people and the list. Perhaps they know? > >> static inline bool gigantic_page_runtime_supported(void) >> { >> return true; >> } >> >> +#if !defined(__HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE) && !defined(MODULE) >> +#include <asm/tlbflush.h> >> + >> +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, >> + unsigned long start, unsigned long end) >> +{ >> + flush_tlb_range(vma, start, end); >> +} >> +#endif > > Similar question here, only ppc defined __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE, so > IIUC it means s390 should simply use the default version, and it'll be great if > we don't need to redefine it here. Actually, your patch "mm/hugetlb: Move flush_hugetlb_tlb_range() into hugetlb.h" makes this change unnecessary. But, the question about ppc remains.
On Wed, 17 Feb 2021 11:24:15 -0500 Peter Xu <peterx@redhat.com> wrote: > On Wed, Feb 10, 2021 at 04:03:18PM -0800, Mike Kravetz wrote: > > Add interfaces to set and clear soft dirty in hugetlb ptes. Make > > hugetlb interfaces needed for /proc clear_refs available outside > > hugetlb.c. > > > > arch/s390 has it's own version of most routines in asm-generic/hugetlb.h, > > so add new routines there as well. > > > > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> > > --- > > arch/s390/include/asm/hugetlb.h | 30 ++++++++++++++++++++++++++++++ > > include/asm-generic/hugetlb.h | 30 ++++++++++++++++++++++++++++++ > > include/linux/hugetlb.h | 1 + > > mm/hugetlb.c | 10 +--------- > > 4 files changed, 62 insertions(+), 9 deletions(-) > > > > diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h > > index 60f9241e5e4a..b7d26248fb1c 100644 > > --- a/arch/s390/include/asm/hugetlb.h > > +++ b/arch/s390/include/asm/hugetlb.h > > @@ -105,6 +105,11 @@ static inline pte_t huge_pte_mkdirty(pte_t pte) > > return pte_mkdirty(pte); > > } > > > > +static inline pte_t huge_pte_mkyoung(pte_t pte) > > +{ > > + return pte_mkyoung(pte); > > +} > > + > > static inline pte_t huge_pte_wrprotect(pte_t pte) > > { > > return pte_wrprotect(pte); > > @@ -115,9 +120,34 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) > > return pte_modify(pte, newprot); > > } > > > > +static inline bool huge_pte_soft_dirty(pte_t pte) > > +{ > > + return pte_soft_dirty(pte); > > +} > > + > > +static inline pte_t huge_pte_clear_soft_dirty(pte_t pte) > > +{ > > + return pte_clear_soft_dirty(pte); > > +} > > + > > +static inline pte_t huge_pte_swp_clear_soft_dirty(pte_t pte) > > +{ > > + return pte_swp_clear_soft_dirty(pte); > > +} > > + > > Indeed asm/hugetlb.h of s390 didn't include asm-generic/hugetlb.h as what was > normally done by asm/hugetlb.h of other archs. Do you know why it's special? > E.g. huge_pte_wrprotect() of s390 version is actually the same of the default > version. That is for "historical reasons", and yes, it doesn't look like it makes a lot of sense any more. The history part: When s390 hugetlb support was introduced in 2008, there was no asm-generic/hugetlb.h, and also no huge_pte_xxx primitives at all. They were actually introduced because of s390, since the hugetlb common code did not make any difference between pte and pmd types, see commit 7f2e9525ba55 ("hugetlbfs: common code update for s390"). Back then, only few architectures with hugetlb support existed, and instead of creating an asm-generic/hugetlb.h, I just added the primitives to the individual arch include files. 5 years later, more huge_pte_xxx primitives were needed, and it appeared to make sense to introduce asm-generic/hugetlb.h, see commit 106c992a5ebe ("mm/hugetlb: add more arch-defined huge_pte functions"). However, for s390, all those primitives still needed special care, so we were / are the only architecture not including that. Then we fundamentally changed the way how we deal with that "hugetlb code is treating pmds as ptes" issue. Instead of caring about that in all huge_pte_xxx primitives, huge_ptep_get() will now return a nicely faked pte for s390, i.e. something that looks like a pte would look like, and not the real pmd/pud value. With that, hugetlb code can do all its pte handling on that fake pte, and the conversion back to a proper pmd/pud is done in set_huge_pte(). This is also why it will go very wrong on s390 if you directly look at or manipulate a huge pte (i.e. pmd or pud) via its pointer, and not use huge_ptep_get() and set_huge_pte(). Since that change, most of the huge_pte_xxx primitives are now the default pte_xxx primitives also for s390, but apparently nobody thought about moving to asm-generic/hugetlb.h. > When I looked at the huge_pte_wrprotect() I also see that there seems to have > no real user of __HAVE_ARCH_HUGE_PTE_WRPROTECT. Not sure whether it can be > dropped. My gut feeling is that s390 should also include asm-generic/hugetlb.h > but only redefine the helper only if necessary, since I see no point defining > the same helper multiple times. Your gut feeling seems right, I will look into cleaning that up. But don't let that keep you from adding things there for now. The __HAVE_ARCH_HUGE_PTE_WRPROTECT is not related to any s390-specifics, I think it was already unused when it was introduced with commit c4916a008665a ("hugetlb: introduce generic version of huge_pte_wrprotect"). Maybe it was just added for completeness or future support, because the corresponding __HAVE_ARCH_HUGE_PTEP_SET_WRPROTECT did / does have some users.
On Wed, 24 Feb 2021 17:46:08 +0100 Gerald Schaefer <gerald.schaefer@linux.ibm.com> wrote: [...] > Then we fundamentally changed the way how we deal with that "hugetlb code > is treating pmds as ptes" issue. Instead of caring about that in all > huge_pte_xxx primitives, huge_ptep_get() will now return a nicely faked pte > for s390, i.e. something that looks like a pte would look like, and not the > real pmd/pud value. With that, hugetlb code can do all its pte handling on > that fake pte, and the conversion back to a proper pmd/pud is done in > set_huge_pte(). BTW, in case anybody is wondering, this conversion from and to pmd for s390 will also care about the soft dirty bit, even though it was not really used before for hugetlb. So Mikes approach to add the default primitives for s390 should work fine.
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h index 60f9241e5e4a..b7d26248fb1c 100644 --- a/arch/s390/include/asm/hugetlb.h +++ b/arch/s390/include/asm/hugetlb.h @@ -105,6 +105,11 @@ static inline pte_t huge_pte_mkdirty(pte_t pte) return pte_mkdirty(pte); } +static inline pte_t huge_pte_mkyoung(pte_t pte) +{ + return pte_mkyoung(pte); +} + static inline pte_t huge_pte_wrprotect(pte_t pte) { return pte_wrprotect(pte); @@ -115,9 +120,34 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) return pte_modify(pte, newprot); } +static inline bool huge_pte_soft_dirty(pte_t pte) +{ + return pte_soft_dirty(pte); +} + +static inline pte_t huge_pte_clear_soft_dirty(pte_t pte) +{ + return pte_clear_soft_dirty(pte); +} + +static inline pte_t huge_pte_swp_clear_soft_dirty(pte_t pte) +{ + return pte_swp_clear_soft_dirty(pte); +} + static inline bool gigantic_page_runtime_supported(void) { return true; } +#if !defined(__HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE) && !defined(MODULE) +#include <asm/tlbflush.h> + +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, + unsigned long start, unsigned long end) +{ + flush_tlb_range(vma, start, end); +} +#endif + #endif /* _ASM_S390_HUGETLB_H */ diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h index 8e1e6244a89d..d8a78dab63bf 100644 --- a/include/asm-generic/hugetlb.h +++ b/include/asm-generic/hugetlb.h @@ -27,11 +27,31 @@ static inline pte_t huge_pte_mkdirty(pte_t pte) return pte_mkdirty(pte); } +static inline pte_t huge_pte_mkyoung(pte_t pte) +{ + return pte_mkyoung(pte); +} + static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot) { return pte_modify(pte, newprot); } +static inline bool huge_pte_soft_dirty(pte_t pte) +{ + return pte_soft_dirty(pte); +} + +static inline pte_t huge_pte_clear_soft_dirty(pte_t pte) +{ + return pte_clear_soft_dirty(pte); +} + +static inline pte_t huge_pte_swp_clear_soft_dirty(pte_t pte) +{ + return pte_swp_clear_soft_dirty(pte); +} + #ifndef __HAVE_ARCH_HUGE_PTE_CLEAR static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep, unsigned long sz) @@ -133,4 +153,14 @@ static inline bool gigantic_page_runtime_supported(void) } #endif /* __HAVE_ARCH_GIGANTIC_PAGE_RUNTIME_SUPPORTED */ +#if !defined(__HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE) && !defined(MODULE) +#include <asm/tlbflush.h> + +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, + unsigned long start, unsigned long end) +{ + flush_tlb_range(vma, start, end); +} +#endif + #endif /* _ASM_GENERIC_HUGETLB_H */ diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index b5807f23caf8..7b6c35c5df99 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -187,6 +187,7 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma, unsigned long address, unsigned long end, pgprot_t newprot); bool is_hugetlb_entry_migration(pte_t pte); +bool is_hugetlb_entry_hwpoisoned(pte_t pte); #else /* !CONFIG_HUGETLB_PAGE */ diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 4bdb58ab14cb..47f3123afd1a 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3759,7 +3759,7 @@ bool is_hugetlb_entry_migration(pte_t pte) return false; } -static bool is_hugetlb_entry_hwpoisoned(pte_t pte) +bool is_hugetlb_entry_hwpoisoned(pte_t pte) { swp_entry_t swp; @@ -4965,14 +4965,6 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, return i ? i : err; } -#ifndef __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE -/* - * ARCHes with special requirements for evicting HUGETLB backing TLB entries can - * implement this. - */ -#define flush_hugetlb_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end) -#endif - unsigned long hugetlb_change_protection(struct vm_area_struct *vma, unsigned long address, unsigned long end, pgprot_t newprot) {
Add interfaces to set and clear soft dirty in hugetlb ptes. Make hugetlb interfaces needed for /proc clear_refs available outside hugetlb.c. arch/s390 has it's own version of most routines in asm-generic/hugetlb.h, so add new routines there as well. Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com> --- arch/s390/include/asm/hugetlb.h | 30 ++++++++++++++++++++++++++++++ include/asm-generic/hugetlb.h | 30 ++++++++++++++++++++++++++++++ include/linux/hugetlb.h | 1 + mm/hugetlb.c | 10 +--------- 4 files changed, 62 insertions(+), 9 deletions(-)