Re: [PATCH v2 08/12] mm: move some code around

Re: [PATCH v2 08/12] mm: move some code around

[Kirill A. Shutemov]

On Tue, Sep 22, 2015 at 04:04:52PM +0530, Vineet Gupta wrote:
> This reduces/simplifies the diff for the next patch which moves THP
> specific code.
> 
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>

Okay, so you group pte-related helpers together, right?
It would be nice to mention it in commit message.

Acked-by: Kirill A. Shutemov kirill.shutemov@linux.intel.com

-- 
 Kirill A. Shutemov

Re: [PATCH v2 09/12] mm,thp: reduce ifdef'ery for THP in generic code

[Kirill A. Shutemov]

On Tue, Sep 22, 2015 at 04:04:53PM +0530, Vineet Gupta wrote:
> - pgtable-generic.c: Fold individual #ifdef for each helper into a top
>   level #ifdef. Makes code more readable

Makes sense.

> - Per Andrew's suggestion removed the dummy implementations for !THP
>   in asm-generic/page-table.h to have build time failures vs. runtime.

I'm not sure it's a good idea. This can lead to unnecessary #ifdefs where
otherwise call to helper would be eliminated by compiler as dead code.

What about dummy helpers with BUILD_BUG()?

> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
>  include/asm-generic/pgtable.h | 49 ++++++++++++++++---------------------------
>  mm/pgtable-generic.c          | 24 +++------------------
>  2 files changed, 21 insertions(+), 52 deletions(-)
> 
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index 29c57b2cb344..2112f4147816 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -30,9 +30,12 @@ extern int ptep_set_access_flags(struct vm_area_struct *vma,
>  #endif
>  
>  #ifndef __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>  extern int pmdp_set_access_flags(struct vm_area_struct *vma,
>  				 unsigned long address, pmd_t *pmdp,
>  				 pmd_t entry, int dirty);
> +
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>  #endif
>  
>  #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
> @@ -64,14 +67,6 @@ static inline int pmdp_test_and_clear_young(struct vm_area_struct *vma,
>  		set_pmd_at(vma->vm_mm, address, pmdp, pmd_mkold(pmd));
>  	return r;
>  }
> -#else /* CONFIG_TRANSPARENT_HUGEPAGE */
> -static inline int pmdp_test_and_clear_young(struct vm_area_struct *vma,
> -					    unsigned long address,
> -					    pmd_t *pmdp)
> -{
> -	BUG();
> -	return 0;
> -}
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>  #endif
>  
> @@ -81,8 +76,21 @@ int ptep_clear_flush_young(struct vm_area_struct *vma,
>  #endif
>  
>  #ifndef __HAVE_ARCH_PMDP_CLEAR_YOUNG_FLUSH
> -int pmdp_clear_flush_young(struct vm_area_struct *vma,
> -			   unsigned long address, pmd_t *pmdp);
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +extern int pmdp_clear_flush_young(struct vm_area_struct *vma,
> +				  unsigned long address, pmd_t *pmdp);
> +#else
> +/*
> + * Despite relevant to THP only, this API is called from generic rmap code
> + * under PageTransHuge(), hence needs a dummy implementation for !THP
> + */

Looks like a case I described above. BUILD_BUG_ON() should work fine here.

> +static inline int pmdp_clear_flush_young(struct vm_area_struct *vma,
> +					 unsigned long address, pmd_t *pmdp)
> +{
> +	BUG();
> +	return 0;
> +}
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>  #endif

-- 
 Kirill A. Shutemov

Re: [PATCH v2 10/12] mm,thp: introduce flush_pmd_tlb_range

[Kirill A. Shutemov]

On Tue, Sep 22, 2015 at 04:04:54PM +0530, Vineet Gupta wrote:

Commit message: -ENOENT.

Otherwise, looks good:

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [PATCH v2 00/12] THP support for ARC

[Kirill A. Shutemov]

On Fri, Oct 09, 2015 at 03:03:26PM +0530, Vineet Gupta wrote:
> On Thursday 01 October 2015 11:32 AM, Vineet Gupta wrote:
> > On Tuesday 22 September 2015 04:04 PM, Vineet Gupta wrote:
> >> > Hi,
> >> > 
> >> > This series brings THP support to ARC. It also introduces an optional new
> >> > thp hook for arches to possibly optimize the TLB flush in thp regime.
> >> > 
> >> > Please review !
> >> > 
> >> > Changes Since v1 [*]
> >> >    - Rebased against v4.3-rc2
> >> >    - Switched ARC pgtable_t to pte_t * 		(Kiril)
> >> >    - Removed stub implementations for		(Andrew)
> >> > 	pmdp_set_access_flags, pmdp_test_and_clear_young, pmdp_set_wrprotect,
> >> > 	pmdp_collapse_flush, pmd_same
> >> > 
> >> > [*] http://lkml.kernel.org/r/1440666194-21478-1-git-send-email-vgupta@synopsys.com
> >> > 
> >> > Vineet Gupta (12):
> >> >   ARC: mm: switch pgtable_to to pte_t *
> >> >   ARC: mm: pte flags comsetic cleanups, comments
> >> >   ARC: mm: Introduce PTE_SPECIAL
> >> >   Documentation/features/vm: pte_special now supported by ARC
> >> >   ARCv2: mm: THP support
> >> >   ARCv2: mm: THP: boot validation/reporting
> >> >   Documentation/features/vm: THP now supported by ARC
> >> >   mm: move some code around
> >> >   mm,thp: reduce ifdef'ery for THP in generic code
> >> >   mm,thp: introduce flush_pmd_tlb_range
> >> >   ARCv2: mm: THP: Implement flush_pmd_tlb_range() optimization
> >> >   ARCv2: Add a DT which enables THP
> >> > 
> >> >  Documentation/features/vm/THP/arch-support.txt     |  2 +-
> >> >  .../features/vm/pte_special/arch-support.txt       |  2 +-
> >> >  arch/arc/Kconfig                                   |  4 +
> >> >  arch/arc/boot/dts/hs_thp.dts                       | 59 +++++++++++++
> >> >  arch/arc/include/asm/hugepage.h                    | 82 ++++++++++++++++++
> >> >  arch/arc/include/asm/page.h                        |  5 +-
> >> >  arch/arc/include/asm/pgalloc.h                     |  6 +-
> >> >  arch/arc/include/asm/pgtable.h                     | 60 +++++++------
> >> >  arch/arc/mm/tlb.c                                  | 76 ++++++++++++++++-
> >> >  arch/arc/mm/tlbex.S                                | 21 +++--
> >> >  include/asm-generic/pgtable.h                      | 49 ++++-------
> >> >  mm/huge_memory.c                                   |  2 +-
> >> >  mm/pgtable-generic.c                               | 99 ++++++++++------------
> >> >  13 files changed, 345 insertions(+), 122 deletions(-)
> >> >  create mode 100644 arch/arc/boot/dts/hs_thp.dts
> >> >  create mode 100644 arch/arc/include/asm/hugepage.h
> > Andrew, Kirill, could you please review/ack the generic mm bits atleast so I can
> > proceed with moving the stuff into linux-next !
> 
> Ping 2 !

Sorry.

> Can I please get some acks on the generic mm bits. Some of the changes
> will likely collide Kirill's THP rework !

Could you check if it acctually collides?

> Given people rebase off of mmtomm would it be better if generic patches went thru
> Andrew and probably included sooner for 4.4 target ?
> 
> -Vineet
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

Re: [PATCH v2 00/12] THP support for ARC

[Kirill A. Shutemov]

On Fri, Oct 09, 2015 at 04:59:43PM +0530, Vineet Gupta wrote:
> On Friday 09 October 2015 03:40 PM, Kirill A. Shutemov wrote:
> > On Fri, Oct 09, 2015 at 03:03:26PM +0530, Vineet Gupta wrote:
> >> On Thursday 01 October 2015 11:32 AM, Vineet Gupta wrote:
> >>> On Tuesday 22 September 2015 04:04 PM, Vineet Gupta wrote:
> >>>>> Hi,
> >>>>>
> >>>>> This series brings THP support to ARC. It also introduces an optional new
> >>>>> thp hook for arches to possibly optimize the TLB flush in thp regime.
> >>>>>
> >>>>> Please review !
> >>>>>
> >>>>> Changes Since v1 [*]
> >>>>>    - Rebased against v4.3-rc2
> >>>>>    - Switched ARC pgtable_t to pte_t * 		(Kiril)
> >>>>>    - Removed stub implementations for		(Andrew)
> >>>>> 	pmdp_set_access_flags, pmdp_test_and_clear_young, pmdp_set_wrprotect,
> >>>>> 	pmdp_collapse_flush, pmd_same
> >>>>>
> >>>>> [*] http://lkml.kernel.org/r/1440666194-21478-1-git-send-email-vgupta@synopsys.com
> >>>>>
> >>>>> Vineet Gupta (12):
> >>>>>   ARC: mm: switch pgtable_to to pte_t *
> >>>>>   ARC: mm: pte flags comsetic cleanups, comments
> >>>>>   ARC: mm: Introduce PTE_SPECIAL
> >>>>>   Documentation/features/vm: pte_special now supported by ARC
> >>>>>   ARCv2: mm: THP support
> >>>>>   ARCv2: mm: THP: boot validation/reporting
> >>>>>   Documentation/features/vm: THP now supported by ARC
> >>>>>   mm: move some code around
> >>>>>   mm,thp: reduce ifdef'ery for THP in generic code
> >>>>>   mm,thp: introduce flush_pmd_tlb_range
> >>>>>   ARCv2: mm: THP: Implement flush_pmd_tlb_range() optimization
> >>>>>   ARCv2: Add a DT which enables THP
> >>>>>
> >>>>>  Documentation/features/vm/THP/arch-support.txt     |  2 +-
> >>>>>  .../features/vm/pte_special/arch-support.txt       |  2 +-
> >>>>>  arch/arc/Kconfig                                   |  4 +
> >>>>>  arch/arc/boot/dts/hs_thp.dts                       | 59 +++++++++++++
> >>>>>  arch/arc/include/asm/hugepage.h                    | 82 ++++++++++++++++++
> >>>>>  arch/arc/include/asm/page.h                        |  5 +-
> >>>>>  arch/arc/include/asm/pgalloc.h                     |  6 +-
> >>>>>  arch/arc/include/asm/pgtable.h                     | 60 +++++++------
> >>>>>  arch/arc/mm/tlb.c                                  | 76 ++++++++++++++++-
> >>>>>  arch/arc/mm/tlbex.S                                | 21 +++--
> >>>>>  include/asm-generic/pgtable.h                      | 49 ++++-------
> >>>>>  mm/huge_memory.c                                   |  2 +-
> >>>>>  mm/pgtable-generic.c                               | 99 ++++++++++------------
> >>>>>  13 files changed, 345 insertions(+), 122 deletions(-)
> >>>>>  create mode 100644 arch/arc/boot/dts/hs_thp.dts
> >>>>>  create mode 100644 arch/arc/include/asm/hugepage.h
> >>> Andrew, Kirill, could you please review/ack the generic mm bits atleast so I can
> >>> proceed with moving the stuff into linux-next !
> >>
> >> Ping 2 !
> > 
> > Sorry.
> > 
> >> Can I please get some acks on the generic mm bits. Some of the changes
> >> will likely collide Kirill's THP rework !
> > 
> > Could you check if it acctually collides?
> 
> I rebased my changes on top of your v12 branch. As expected I get two merge
> conflicts as my patches update code which u have removed. But looks straight
> forward to resolve. Meaning we can carry respective patches and give Linus a heads
> up abt the merge conflict.
> 
> There's also a fixup needed for ARC to remove the pmdp splitting assuming ARC THP
> makes it into mainline first.

Okay. Let's see what hits upstream first.

Side question: I was surprised by your patchset. I didn't think about ARC
as an architecture which would benefit from THP. How big ARC systems
nowadays? How big benefit did you measured there from THP? 

-- 
 Kirill A. Shutemov