Re: [PATCH 1/2] mm/autonuma: Let architecture override how the write bit should be stashed in a protnone pte.

[Michael Neuling]

On Thu, 2017-02-09 at 08:30 +0530, Aneesh Kumar K.V wrote:
> Autonuma preserves the write permission across numa fault to avoid taking
> a writefault after a numa fault (Commit: b191f9b106ea " mm: numa: preserve PTE
> write permissions across a NUMA hinting fault"). Architecture can implement
> protnone in different ways and some may choose to implement that by clearing
> Read/
> Write/Exec bit of pte. Setting the write bit on such pte can result in wrong
> behaviour. Fix this up by allowing arch to override how to save the write bit
> on a protnone pte.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>

FWIW this is pretty simple and helps with us in powerpc...

Acked-By: Michael Neuling <mikey@xxxxxxxxxxx>

> ---
> Âinclude/asm-generic/pgtable.h | 16 ++++++++++++++++
> Âmm/huge_memory.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ4 ++--
> Âmm/memory.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ2 +-
> Âmm/mprotect.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ4 ++--
> Â4 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index 18af2bcefe6a..b6f3a8a4b738 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -192,6 +192,22 @@ static inline void ptep_set_wrprotect(struct mm_struct
> *mm, unsigned long addres
> Â}
> Â#endif
> Â
> +#ifndef pte_savedwrite
> +#define pte_savedwrite pte_write
> +#endif
> +
> +#ifndef pte_mk_savedwrite
> +#define pte_mk_savedwrite pte_mkwrite
> +#endif
> +
> +#ifndef pmd_savedwrite
> +#define pmd_savedwrite pmd_write
> +#endif
> +
> +#ifndef pmd_mk_savedwrite
> +#define pmd_mk_savedwrite pmd_mkwrite
> +#endif
> +
> Â#ifndef __HAVE_ARCH_PMDP_SET_WRPROTECT
> Â#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> Âstatic inline void pmdp_set_wrprotect(struct mm_struct *mm,
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 9a6bd6c8d55a..2f0f855ec911 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1300,7 +1300,7 @@ int do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t
> pmd)
> Â	goto out;
> Âclear_pmdnuma:
> Â	BUG_ON(!PageLocked(page));
> -	was_writable = pmd_write(pmd);
> +	was_writable = pmd_savedwrite(pmd);
> Â	pmd = pmd_modify(pmd, vma->vm_page_prot);
> Â	pmd = pmd_mkyoung(pmd);
> Â	if (was_writable)
> @@ -1555,7 +1555,7 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t
> *pmd,
> Â			entry = pmdp_huge_get_and_clear_notify(mm, addr,
> pmd);
> Â			entry = pmd_modify(entry, newprot);
> Â			if (preserve_write)
> -				entry = pmd_mkwrite(entry);
> +				entry = pmd_mk_savedwrite(entry);
> Â			ret = HPAGE_PMD_NR;
> Â			set_pmd_at(mm, addr, pmd, entry);
> Â			BUG_ON(vma_is_anonymous(vma) && !preserve_write &&
> diff --git a/mm/memory.c b/mm/memory.c
> index e78bf72f30dd..88c24f89d6d3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3388,7 +3388,7 @@ static int do_numa_page(struct vm_fault *vmf)
> Â	int target_nid;
> Â	bool migrated = false;
> Â	pte_t pte;
> -	bool was_writable = pte_write(vmf->orig_pte);
> +	bool was_writable = pte_savedwrite(vmf->orig_pte);
> Â	int flags = 0;
> Â
> Â	/*
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index f9c07f54dd62..15f5c174a7c1 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -113,13 +113,13 @@ static unsigned long change_pte_range(struct
> vm_area_struct *vma, pmd_t *pmd,
> Â			ptent = ptep_modify_prot_start(mm, addr, pte);
> Â			ptent = pte_modify(ptent, newprot);
> Â			if (preserve_write)
> -				ptent = pte_mkwrite(ptent);
> +				ptent = pte_mk_savedwrite(ptent);
> Â
> Â			/* Avoid taking write faults for known dirty pages */
> Â			if (dirty_accountable && pte_dirty(ptent) &&
> Â					(pte_soft_dirty(ptent) ||
> Â					Â!(vma->vm_flags & VM_SOFTDIRTY))) {
> -				ptent = pte_mkwrite(ptent);
> +				ptent = pte_mk_savedwrite(ptent);
> Â			}
> Â			ptep_modify_prot_commit(mm, addr, pte, ptent);
> Â			pages++;