Re: [RFC PATCH 3/3] mm/hugetlb: Remove pmd_huge_split_prepare

2017-07-28 Thread Michal Hocko 
On Thu 27-07-17 21:27:37, Aneesh Kumar K.V wrote:
> 
> 
> On 07/27/2017 06:27 PM, Michal Hocko wrote:
> >On Thu 27-07-17 14:07:56, Aneesh Kumar K.V wrote:
> >>Instead of marking the pmd ready for split, invalidate the pmd. This should
> >>take care of powerpc requirement.
> >
> >which is?
> 
> I can add the commit which explain details here. Or add more details from
> the older commit here.
> 
> c777e2a8b65420b31dac28a453e35be984f5808b
> 
> powerpc/mm: Fix Multi hit ERAT cause by recent THP update

Each patch should be self descriptive. You can reference older commits
but always make sure that the full context is clear. This will make the
life easier to whoever is going to look at it later.
 
> >>Only side effect is that we mark the pmd
> >>invalid early. This can result in us blocking access to the page a bit 
> >>longer
> >>if we race against a thp split.
> >
> >Again, this doesn't tell me what is the problem and why do we care.
> 
> Primary motivation is code reduction.

Then be explicit about it. This wasn't clear from the above description.
At least not to me.
 
>   7 files changed, 35 insertions(+), 87 deletions(-)
> 
> 
> -aneesh

-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 3/3] mm/hugetlb: Remove pmd_huge_split_prepare

2017-07-27 Thread Aneesh Kumar K.V 



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

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

Instead of marking the pmd ready for split, invalidate the pmd. This should
take care of powerpc requirement.


which is?


I can add the commit which explain details here. Or add more details 
from the older commit here.


c777e2a8b65420b31dac28a453e35be984f5808b

powerpc/mm: Fix Multi hit ERAT cause by recent THP update





Only side effect is that we mark the pmd
invalid early. This can result in us blocking access to the page a bit longer
if we race against a thp split.


Again, this doesn't tell me what is the problem and why do we care.


Primary motivation is code reduction.

  7 files changed, 35 insertions(+), 87 deletions(-)


-aneesh

Re: [RFC PATCH 3/3] mm/hugetlb: Remove pmd_huge_split_prepare

2017-07-27 Thread Michal Hocko 
On Thu 27-07-17 14:07:56, Aneesh Kumar K.V wrote:
> Instead of marking the pmd ready for split, invalidate the pmd. This should
> take care of powerpc requirement.

which is?

> Only side effect is that we mark the pmd
> invalid early. This can result in us blocking access to the page a bit longer
> if we race against a thp split.

Again, this doesn't tell me what is the problem and why do we care.

> Signed-off-by: Aneesh Kumar K.V 
> ---
>  arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 -
>  arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 -
>  arch/powerpc/include/asm/book3s/64/pgtable.h  |  9 
>  arch/powerpc/include/asm/book3s/64/radix.h|  6 ---
>  arch/powerpc/mm/pgtable-hash64.c  | 22 
>  include/asm-generic/pgtable.h |  8 ---
>  mm/huge_memory.c  | 73 
> +--
>  7 files changed, 35 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h 
> b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> index 0c4e470571ca..7d914f4fc534 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> @@ -100,8 +100,6 @@ extern pmd_t hash__pmdp_collapse_flush(struct 
> vm_area_struct *vma,
>  extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t 
> *pmdp,
>pgtable_t pgtable);
>  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, 
> pmd_t *pmdp);
> -extern void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,
> -   unsigned long address, pmd_t *pmdp);
>  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
>  unsigned long addr, pmd_t *pmdp);
>  extern int hash__has_transparent_hugepage(void);
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h 
> b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> index 8c8fb6fbdabe..b856e130c678 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> @@ -164,8 +164,6 @@ extern pmd_t hash__pmdp_collapse_flush(struct 
> vm_area_struct *vma,
>  extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t 
> *pmdp,
>pgtable_t pgtable);
>  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, 
> pmd_t *pmdp);
> -extern void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,
> -   unsigned long address, pmd_t *pmdp);
>  extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
>  unsigned long addr, pmd_t *pmdp);
>  extern int hash__has_transparent_hugepage(void);
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
> b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index ece6912fae8e..557915792214 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1122,15 +1122,6 @@ static inline pgtable_t 
> pgtable_trans_huge_withdraw(struct mm_struct *mm,
>  extern pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long 
> address,
>pmd_t *pmdp);
>  
> -#define __HAVE_ARCH_PMDP_HUGE_SPLIT_PREPARE
> -static inline void pmdp_huge_split_prepare(struct vm_area_struct *vma,
> -unsigned long address, pmd_t *pmdp)
> -{
> - if (radix_enabled())
> - return radix__pmdp_huge_split_prepare(vma, address, pmdp);
> - return hash__pmdp_huge_split_prepare(vma, address, pmdp);
> -}
> -
>  #define pmd_move_must_withdraw pmd_move_must_withdraw
>  struct spinlock;
>  static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
> diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
> b/arch/powerpc/include/asm/book3s/64/radix.h
> index 558fea3b2d22..a779a43b643b 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix.h
> @@ -270,12 +270,6 @@ static inline pmd_t radix__pmd_mkhuge(pmd_t pmd)
>   return __pmd(pmd_val(pmd) | _PAGE_PTE | R_PAGE_LARGE);
>   return __pmd(pmd_val(pmd) | _PAGE_PTE);
>  }
> -static inline void radix__pmdp_huge_split_prepare(struct vm_area_struct *vma,
> - unsigned long address, pmd_t *pmdp)
> -{
> - /* Nothing to do for radix. */
> - return;
> -}
>  
>  extern unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, 
> unsigned long addr,
> pmd_t *pmdp, unsigned long clr,
> diff --git a/arch/powerpc/mm/pgtable-hash64.c 
> b/arch/powerpc/mm/pgtable-hash64.c
> index c0a7372bdaa6..00aee1485714 100644
> --- a/arch/powerpc/mm/pgtable-hash64.c
> +++ b/arch/powerpc/mm/pgtable-hash64.c
> @@ -296,28 +296,6 @@ pgtable_t hash__pgtable_trans_huge_withdraw(struct 
>

Re: [RFC PATCH 3/3] mm/hugetlb: Remove pmd_huge_split_prepare

2017-07-27 Thread Aneesh Kumar K.V 



On 07/27/2017 02:07 PM, Aneesh Kumar K.V wrote:


diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h 
b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index 8c8fb6fbdabe..b856e130c678 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -164,8 +164,6 @@ extern pmd_t hash__pmdp_collapse_flush(struct 
vm_area_struct *vma,
  extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t 
*pmdp,
 pgtable_t pgtable);
  extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, 
pmd_t *pmdp);
-extern void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,

@@ -1956,14 +1956,39 @@ static void __split_huge_pmd_locked(struct 
vm_area_struct *vma, pmd_t *pmd,
return __split_huge_zero_page_pmd(vma, haddr, pmd);
}







+*/
+   old_pmd = pmdp_invalidate(vma, haddr, pmd);
+
+   page = pmd_page(old_pmd);
VM_BUG_ON_PAGE(!page_count(page), page);
page_ref_add(page, HPAGE_PMD_NR - 1);
-   write = pmd_write(*pmd);
-   young = pmd_young(*pmd);
-   soft_dirty = pmd_soft_dirty(*pmd);
-
-   pmdp_huge_split_prepare(vma, haddr, pmd);
+   write = pmd_write(old_pmd);
+   young = pmd_young(old_pmd);
+   dirty = pmd_dirty(*pmd);


This should be

dirty = pmd_dirty(old_pmd);



+   soft_dirty = pmd_soft_dirty(old_pmd);
+   /*
+* withdraw the table only after we mark the pmd entry invalid
+*/
pgtable = pgtable_trans_huge_withdraw(mm, pmd);


-aneesh

[RFC PATCH 3/3] mm/hugetlb: Remove pmd_huge_split_prepare

2017-07-27 Thread Aneesh Kumar K.V 
Instead of marking the pmd ready for split, invalidate the pmd. This should
take care of powerpc requirement. Only side effect is that we mark the pmd
invalid early. This can result in us blocking access to the page a bit longer
if we race against a thp split.

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 -
 arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 -
 arch/powerpc/include/asm/book3s/64/pgtable.h  |  9 
 arch/powerpc/include/asm/book3s/64/radix.h|  6 ---
 arch/powerpc/mm/pgtable-hash64.c  | 22 
 include/asm-generic/pgtable.h |  8 ---
 mm/huge_memory.c  | 73 +--
 7 files changed, 35 insertions(+), 87 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h 
b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index 0c4e470571ca..7d914f4fc534 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -100,8 +100,6 @@ extern pmd_t hash__pmdp_collapse_flush(struct 
vm_area_struct *vma,
 extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 pgtable_t pgtable);
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t 
*pmdp);
-extern void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,
- unsigned long address, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
   unsigned long addr, pmd_t *pmdp);
 extern int hash__has_transparent_hugepage(void);
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h 
b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index 8c8fb6fbdabe..b856e130c678 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -164,8 +164,6 @@ extern pmd_t hash__pmdp_collapse_flush(struct 
vm_area_struct *vma,
 extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
 pgtable_t pgtable);
 extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t 
*pmdp);
-extern void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,
- unsigned long address, pmd_t *pmdp);
 extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
   unsigned long addr, pmd_t *pmdp);
 extern int hash__has_transparent_hugepage(void);
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index ece6912fae8e..557915792214 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1122,15 +1122,6 @@ static inline pgtable_t 
pgtable_trans_huge_withdraw(struct mm_struct *mm,
 extern pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
 pmd_t *pmdp);
 
-#define __HAVE_ARCH_PMDP_HUGE_SPLIT_PREPARE
-static inline void pmdp_huge_split_prepare(struct vm_area_struct *vma,
-  unsigned long address, pmd_t *pmdp)
-{
-   if (radix_enabled())
-   return radix__pmdp_huge_split_prepare(vma, address, pmdp);
-   return hash__pmdp_huge_split_prepare(vma, address, pmdp);
-}
-
 #define pmd_move_must_withdraw pmd_move_must_withdraw
 struct spinlock;
 static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
b/arch/powerpc/include/asm/book3s/64/radix.h
index 558fea3b2d22..a779a43b643b 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -270,12 +270,6 @@ static inline pmd_t radix__pmd_mkhuge(pmd_t pmd)
return __pmd(pmd_val(pmd) | _PAGE_PTE | R_PAGE_LARGE);
return __pmd(pmd_val(pmd) | _PAGE_PTE);
 }
-static inline void radix__pmdp_huge_split_prepare(struct vm_area_struct *vma,
-   unsigned long address, pmd_t *pmdp)
-{
-   /* Nothing to do for radix. */
-   return;
-}
 
 extern unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, unsigned 
long addr,
  pmd_t *pmdp, unsigned long clr,
diff --git a/arch/powerpc/mm/pgtable-hash64.c b/arch/powerpc/mm/pgtable-hash64.c
index c0a7372bdaa6..00aee1485714 100644
--- a/arch/powerpc/mm/pgtable-hash64.c
+++ b/arch/powerpc/mm/pgtable-hash64.c
@@ -296,28 +296,6 @@ pgtable_t hash__pgtable_trans_huge_withdraw(struct 
mm_struct *mm, pmd_t *pmdp)
return pgtable;
 }
 
-void hash__pmdp_huge_split_prepare(struct vm_area_struct *vma,
-  unsigned long address, pmd_t *pmdp)
-{
-   VM_BUG_ON(address & ~HPAGE_PMD_MASK);
-   VM_BUG_ON(REGION_ID(address) != USER_REGION_ID);
-