Re: [RFC PATCH 2/3] powerpc/mm: Implement pmdp_establish for ppc64

2017-07-27 Thread Aneesh Kumar K.V 



On 07/27/2017 06:26 PM, Michal Hocko wrote:

On Thu 27-07-17 14:07:55, Aneesh Kumar K.V wrote:

We can now use this to set pmd page table entries to absolute values. THP
need to ensure that we always update pmd PTE entries such that we never mark
the pmd none. pmdp_establish helps in implementing that.

This doesn't flush the tlb. Based on the old_pmd value returned caller can
decide to call flush_pmd_tlb_range()


_Why_ do we need this. It doesn't really help that the newly added
function is not used so we could check that...



We were looking at having pmdp_establish used by the core code. But i 
guess Kirill ended up using pmdp_invalidate. If we don't have 
pmdp_establish usage in core code, we can drop this. This is to help 
Kiril make progress with series at



https://lkml.kernel.org/r/20170615145224.66200-1-kirill.shute...@linux.intel.com


Also thinking about the interface further, I guess pmdp_establish 
interface is some what confusing. So we may want to rethink this 
further. I know that i asked for pmdp_establish in earlier review of 
Kirill's patchset. But now looking back i am not sure we can clearly 
explain only semantic requirement of pmdp_establish. One thing we may 
want to clarify is whether we should retain the Reference and change bit 
from the old entry when we are doing a pmdp_establish ?


Kirill,

Considering core code is still only using pmdp_invalidate(), we may want 
to drop this interface completely ?


-aneesh

Re: [RFC PATCH 2/3] powerpc/mm: Implement pmdp_establish for ppc64

2017-07-27 Thread Michal Hocko 
On Thu 27-07-17 14:07:55, Aneesh Kumar K.V wrote:
> We can now use this to set pmd page table entries to absolute values. THP
> need to ensure that we always update pmd PTE entries such that we never mark
> the pmd none. pmdp_establish helps in implementing that.
> 
> This doesn't flush the tlb. Based on the old_pmd value returned caller can
> decide to call flush_pmd_tlb_range()

_Why_ do we need this. It doesn't really help that the newly added
function is not used so we could check that...

> 
> Signed-off-by: Aneesh Kumar K.V 
> ---
>  arch/powerpc/include/asm/book3s/64/radix.h |  9 ++---
>  arch/powerpc/mm/pgtable-book3s64.c | 10 ++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
> b/arch/powerpc/include/asm/book3s/64/radix.h
> index cd481ab601b6..558fea3b2d22 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix.h
> @@ -131,7 +131,8 @@ static inline unsigned long __radix_pte_update(pte_t 
> *ptep, unsigned long clr,
>   do {
>   pte = READ_ONCE(*ptep);
>   old_pte = pte_val(pte);
> - new_pte = (old_pte | set) & ~clr;
> + new_pte = old_pte & ~clr;
> + new_pte |= set;
>  
>   } while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
>  
> @@ -153,9 +154,11 @@ static inline unsigned long radix__pte_update(struct 
> mm_struct *mm,
>  
>   old_pte = __radix_pte_update(ptep, ~0ul, 0);
>   /*
> -  * new value of pte
> +  * new value of pte. We clear all the bits in clr mask
> +  * first and set the bits in set mask.
>*/
> - new_pte = (old_pte | set) & ~clr;
> + new_pte = old_pte & ~clr;
> + new_pte |= set;
>   radix__flush_tlb_pte_p9_dd1(old_pte, mm, addr);
>   if (new_pte)
>   __radix_pte_update(ptep, 0, new_pte);
> diff --git a/arch/powerpc/mm/pgtable-book3s64.c 
> b/arch/powerpc/mm/pgtable-book3s64.c
> index 0bb7f824ecdd..7100b0150a2a 100644
> --- a/arch/powerpc/mm/pgtable-book3s64.c
> +++ b/arch/powerpc/mm/pgtable-book3s64.c
> @@ -45,6 +45,16 @@ int pmdp_set_access_flags(struct vm_area_struct *vma, 
> unsigned long address,
>   return changed;
>  }
>  
> +pmd_t pmdp_establish(struct vm_area_struct *vma, unsigned long addr,
> +  pmd_t *pmdp, pmd_t entry)
> +{
> + long pmdval;
> +
> + pmdval = pmd_hugepage_update(vma->vm_mm, addr, pmdp, ~0UL, 
> pmd_val(entry));
> + return __pmd(pmdval);
> +}
> +
> +
>  int pmdp_test_and_clear_young(struct vm_area_struct *vma,
> unsigned long address, pmd_t *pmdp)
>  {
> -- 
> 2.13.3
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
Michal Hocko
SUSE Labs

[RFC PATCH 2/3] powerpc/mm: Implement pmdp_establish for ppc64

2017-07-27 Thread Aneesh Kumar K.V 
We can now use this to set pmd page table entries to absolute values. THP
need to ensure that we always update pmd PTE entries such that we never mark
the pmd none. pmdp_establish helps in implementing that.

This doesn't flush the tlb. Based on the old_pmd value returned caller can
decide to call flush_pmd_tlb_range()

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/book3s/64/radix.h |  9 ++---
 arch/powerpc/mm/pgtable-book3s64.c | 10 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
b/arch/powerpc/include/asm/book3s/64/radix.h
index cd481ab601b6..558fea3b2d22 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -131,7 +131,8 @@ static inline unsigned long __radix_pte_update(pte_t *ptep, 
unsigned long clr,
do {
pte = READ_ONCE(*ptep);
old_pte = pte_val(pte);
-   new_pte = (old_pte | set) & ~clr;
+   new_pte = old_pte & ~clr;
+   new_pte |= set;
 
} while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
 
@@ -153,9 +154,11 @@ static inline unsigned long radix__pte_update(struct 
mm_struct *mm,
 
old_pte = __radix_pte_update(ptep, ~0ul, 0);
/*
-* new value of pte
+* new value of pte. We clear all the bits in clr mask
+* first and set the bits in set mask.
 */
-   new_pte = (old_pte | set) & ~clr;
+   new_pte = old_pte & ~clr;
+   new_pte |= set;
radix__flush_tlb_pte_p9_dd1(old_pte, mm, addr);
if (new_pte)
__radix_pte_update(ptep, 0, new_pte);
diff --git a/arch/powerpc/mm/pgtable-book3s64.c 
b/arch/powerpc/mm/pgtable-book3s64.c
index 0bb7f824ecdd..7100b0150a2a 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -45,6 +45,16 @@ int pmdp_set_access_flags(struct vm_area_struct *vma, 
unsigned long address,
return changed;
 }
 
+pmd_t pmdp_establish(struct vm_area_struct *vma, unsigned long addr,
+pmd_t *pmdp, pmd_t entry)
+{
+   long pmdval;
+
+   pmdval = pmd_hugepage_update(vma->vm_mm, addr, pmdp, ~0UL, 
pmd_val(entry));
+   return __pmd(pmdval);
+}
+
+
 int pmdp_test_and_clear_young(struct vm_area_struct *vma,
  unsigned long address, pmd_t *pmdp)
 {
-- 
2.13.3