| Message ID | 20230214015939.1853438-3-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support page table check | expand |
Le 14/02/2023 à 02:59, Rohan McLure a écrit : > pmdp_collapse_flush has references in generic code with just three > parameters, due to the choice of mm context being implied by the vm_area > context parameter. > > Define __pmdp_collapse_flush to accept an additional mm_struct * > parameter, with pmdp_collapse_flush a macro that unpacks the vma and > calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a > future patch providing Page Table Check support, which is defined in > terms of mm context objects. > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > v6: New patch > --- > arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h > index cb4c67bf45d7..9d8b4e25f5ed 100644 > --- a/arch/powerpc/include/asm/book3s/64/pgtable.h > +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h > @@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm, > return hash__pmdp_huge_get_and_clear(mm, addr, pmdp); > } > > -static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma, > - unsigned long address, pmd_t *pmdp) > +static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct mm_struct *mm, > + unsigned long address, pmd_t *pmdp) > { > if (radix_enabled()) > return radix__pmdp_collapse_flush(vma, address, pmdp); > return hash__pmdp_collapse_flush(vma, address, pmdp); > } > -#define pmdp_collapse_flush pmdp_collapse_flush > +#define pmdp_collapse_flush(vma, addr, pmdp) \ > +({ \ > + struct vm_area_struct *_vma = (vma); \ > + pmd_t _r; \ > + \ > + _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \ > + \ > + _r; \ > +}) Can you make it a static inline function instead of a ugly macro ? > > #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL > pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
> On 14 Feb 2023, at 5:02 pm, Christophe Leroy <christophe.leroy@csgroup.eu> wrote: > > > > Le 14/02/2023 à 02:59, Rohan McLure a écrit : >> pmdp_collapse_flush has references in generic code with just three >> parameters, due to the choice of mm context being implied by the vm_area >> context parameter. >> >> Define __pmdp_collapse_flush to accept an additional mm_struct * >> parameter, with pmdp_collapse_flush a macro that unpacks the vma and >> calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a >> future patch providing Page Table Check support, which is defined in >> terms of mm context objects. >> >> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> >> --- >> v6: New patch >> --- >> arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h >> index cb4c67bf45d7..9d8b4e25f5ed 100644 >> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >> @@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm, >> return hash__pmdp_huge_get_and_clear(mm, addr, pmdp); >> } >> >> -static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma, >> - unsigned long address, pmd_t *pmdp) >> +static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct mm_struct *mm, >> + unsigned long address, pmd_t *pmdp) >> { >> if (radix_enabled()) >> return radix__pmdp_collapse_flush(vma, address, pmdp); >> return hash__pmdp_collapse_flush(vma, address, pmdp); >> } >> -#define pmdp_collapse_flush pmdp_collapse_flush >> +#define pmdp_collapse_flush(vma, addr, pmdp) \ >> +({ \ >> + struct vm_area_struct *_vma = (vma); \ >> + pmd_t _r; \ >> + \ >> + _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \ >> + \ >> + _r; \ >> +}) > > Can you make it a static inline function instead of a ugly macro ? Due to some header hell, it’s looking like this location only has access to a prototype for struct vm_area_struct. Might have to remain a macro then. Probably don’t need to expliclty declare a variable for the macro ‘return’ though. > >> >> #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL >> pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
> On 15 Feb 2023, at 11:17 am, Rohan McLure <rmclure@linux.ibm.com> wrote: > >> On 14 Feb 2023, at 5:02 pm, Christophe Leroy <christophe.leroy@csgroup.eu> wrote: >> >> >> >> Le 14/02/2023 à 02:59, Rohan McLure a écrit : >>> pmdp_collapse_flush has references in generic code with just three >>> parameters, due to the choice of mm context being implied by the vm_area >>> context parameter. >>> >>> Define __pmdp_collapse_flush to accept an additional mm_struct * >>> parameter, with pmdp_collapse_flush a macro that unpacks the vma and >>> calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a >>> future patch providing Page Table Check support, which is defined in >>> terms of mm context objects. >>> >>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> >>> --- >>> v6: New patch >>> --- >>> arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++++++++++--- >>> 1 file changed, 11 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h >>> index cb4c67bf45d7..9d8b4e25f5ed 100644 >>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h >>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h >>> @@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm, >>> return hash__pmdp_huge_get_and_clear(mm, addr, pmdp); >>> } >>> >>> -static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma, >>> - unsigned long address, pmd_t *pmdp) >>> +static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct mm_struct *mm, >>> + unsigned long address, pmd_t *pmdp) >>> { >>> if (radix_enabled()) >>> return radix__pmdp_collapse_flush(vma, address, pmdp); >>> return hash__pmdp_collapse_flush(vma, address, pmdp); >>> } >>> -#define pmdp_collapse_flush pmdp_collapse_flush >>> +#define pmdp_collapse_flush(vma, addr, pmdp) \ >>> +({ \ >>> + struct vm_area_struct *_vma = (vma); \ >>> + pmd_t _r; \ >>> + \ >>> + _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \ >>> + \ >>> + _r; \ >>> +}) >> >> Can you make it a static inline function instead of a ugly macro ? > > Due to some header hell, it’s looking like this location only has access to > a prototype for struct vm_area_struct. Might have to remain a macro then. > > Probably don’t need to expliclty declare a variable for the macro ‘return’ > though. It’s the same solution opted for by ptep_test_and_clear_young. #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG #define ptep_test_and_clear_young(__vma, __addr, __ptep) \ ({ \ __ptep_test_and_clear_young((__vma)->vm_mm, __addr, __ptep); \ }) > >> >>> >>> #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL >>> pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h index cb4c67bf45d7..9d8b4e25f5ed 100644 --- a/arch/powerpc/include/asm/book3s/64/pgtable.h +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h @@ -1244,14 +1244,22 @@ static inline pmd_t pmdp_huge_get_and_clear(struct mm_struct *mm, return hash__pmdp_huge_get_and_clear(mm, addr, pmdp); } -static inline pmd_t pmdp_collapse_flush(struct vm_area_struct *vma, - unsigned long address, pmd_t *pmdp) +static inline pmd_t __pmdp_collapse_flush(struct vm_area_struct *vma, struct mm_struct *mm, + unsigned long address, pmd_t *pmdp) { if (radix_enabled()) return radix__pmdp_collapse_flush(vma, address, pmdp); return hash__pmdp_collapse_flush(vma, address, pmdp); } -#define pmdp_collapse_flush pmdp_collapse_flush +#define pmdp_collapse_flush(vma, addr, pmdp) \ +({ \ + struct vm_area_struct *_vma = (vma); \ + pmd_t _r; \ + \ + _r = __pmdp_collapse_flush(_vma, _vma->vm_mm, (addr), (pmdp)); \ + \ + _r; \ +}) #define __HAVE_ARCH_PMDP_HUGE_GET_AND_CLEAR_FULL pmd_t pmdp_huge_get_and_clear_full(struct vm_area_struct *vma,
pmdp_collapse_flush has references in generic code with just three parameters, due to the choice of mm context being implied by the vm_area context parameter. Define __pmdp_collapse_flush to accept an additional mm_struct * parameter, with pmdp_collapse_flush a macro that unpacks the vma and calls __pmdp_collapse_flush. The mm_struct * parameter is needed in a future patch providing Page Table Check support, which is defined in terms of mm context objects. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- v6: New patch --- arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)