[PATCH V5 1/5] mm: Update ptep_modify_prot_start/commit to take vm_area_struct as arg

Michael Ellerman mpe at ellerman.id.au
Wed Jan 30 21:33:57 AEDT 2019


"Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:

> Some architecture may want to call flush_tlb_range from these helpers.

That's what we want to do, but wouldn't a better description be that
some architectures may need access to the vma for some reason, one of
which might be flushing the TLB.

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
> ---
>  arch/s390/include/asm/pgtable.h       |  4 ++--
>  arch/s390/mm/pgtable.c                |  6 ++++--
>  arch/x86/include/asm/paravirt.h       | 11 ++++++-----
>  arch/x86/include/asm/paravirt_types.h |  5 +++--
>  arch/x86/xen/mmu.h                    |  4 ++--
>  arch/x86/xen/mmu_pv.c                 |  8 ++++----
>  fs/proc/task_mmu.c                    |  4 ++--
>  include/asm-generic/pgtable.h         | 16 ++++++++--------
>  mm/memory.c                           |  4 ++--
>  mm/mprotect.c                         |  4 ++--
>  10 files changed, 35 insertions(+), 31 deletions(-)
>
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 063732414dfb..5d730199e37b 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -1069,8 +1069,8 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
>  }
>  
>  #define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> -pte_t ptep_modify_prot_start(struct mm_struct *, unsigned long, pte_t *);
> -void ptep_modify_prot_commit(struct mm_struct *, unsigned long, pte_t *, pte_t);
> +pte_t ptep_modify_prot_start(struct vm_area_struct *, unsigned long, pte_t *);
> +void ptep_modify_prot_commit(struct vm_area_struct *, unsigned long, pte_t *, pte_t);
>  
>  #define __HAVE_ARCH_PTEP_CLEAR_FLUSH
>  static inline pte_t ptep_clear_flush(struct vm_area_struct *vma,
> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index f2cc7da473e4..29c0a21cd34a 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -301,12 +301,13 @@ pte_t ptep_xchg_lazy(struct mm_struct *mm, unsigned long addr,
>  }
>  EXPORT_SYMBOL(ptep_xchg_lazy);
>  
> -pte_t ptep_modify_prot_start(struct mm_struct *mm, unsigned long addr,
> +pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
>  			     pte_t *ptep)
>  {
>  	pgste_t pgste;
>  	pte_t old;
>  	int nodat;
> +	struct mm_struct *mm = vma->vm_mm;

If this was my code I'd want the mm as the first variable, to preserve
the Reverse Christmas tree format.

>  	preempt_disable();
>  	pgste = ptep_xchg_start(mm, addr, ptep);
> @@ -320,10 +321,11 @@ pte_t ptep_modify_prot_start(struct mm_struct *mm, unsigned long addr,
>  }
>  EXPORT_SYMBOL(ptep_modify_prot_start);
>  
> -void ptep_modify_prot_commit(struct mm_struct *mm, unsigned long addr,
> +void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
>  			     pte_t *ptep, pte_t pte)
>  {
>  	pgste_t pgste;
> +	struct mm_struct *mm = vma->vm_mm;
  
Ditto.

>  	if (!MACHINE_HAS_NX)
>  		pte_val(pte) &= ~_PAGE_NOEXEC;
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index a97f28d914d5..c5a7f18cce7e 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -422,25 +422,26 @@ static inline pgdval_t pgd_val(pgd_t pgd)
>  }
>  
>  #define  __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
> -static inline pte_t ptep_modify_prot_start(struct mm_struct *mm, unsigned long addr,
> +static inline pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
>  					   pte_t *ptep)
>  {
>  	pteval_t ret;
>  
> -	ret = PVOP_CALL3(pteval_t, mmu.ptep_modify_prot_start, mm, addr, ptep);
> +	ret = PVOP_CALL3(pteval_t, mmu.ptep_modify_prot_start, vma, addr, ptep);
>  
>  	return (pte_t) { .pte = ret };
>  }
>  
> -static inline void ptep_modify_prot_commit(struct mm_struct *mm, unsigned long addr,
> +static inline void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
>  					   pte_t *ptep, pte_t pte)
>  {
> +

Unnecessary blank line here.

>  	if (sizeof(pteval_t) > sizeof(long))
>  		/* 5 arg words */
> -		pv_ops.mmu.ptep_modify_prot_commit(mm, addr, ptep, pte);
> +		pv_ops.mmu.ptep_modify_prot_commit(vma, addr, ptep, pte);
>  	else
>  		PVOP_VCALL4(mmu.ptep_modify_prot_commit,
> -			    mm, addr, ptep, pte.pte);
> +			    vma, addr, ptep, pte.pte);
>  }
>  
>  static inline void set_pte(pte_t *ptep, pte_t pte)

The rest looks good.

cheers


More information about the Linuxppc-dev mailing list