[PATCH] remove double indirection on tlb parameter to free_pgd_range() & Co

[PATCH] remove double indirection on tlb parameter to free_pgd_range() & Co

[Jan Beulich]

The double indirection here is not needed anywhere and hence (at least)
confusing.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/ia64/mm/hugetlbpage.c    |    2 +-
 arch/powerpc/mm/hugetlbpage.c |    8 ++++----
 fs/exec.c                     |    4 ++--
 include/asm-ia64/hugetlb.h    |    2 +-
 include/asm-powerpc/hugetlb.h |    2 +-
 include/asm-sh/hugetlb.h      |    2 +-
 include/asm-sparc64/hugetlb.h |    2 +-
 include/asm-x86/hugetlb.h     |    2 +-
 include/linux/mm.h            |    4 +---
 mm/internal.h                 |    3 +++
 mm/memory.c                   |   10 ++++++----
 mm/mmap.c                     |    6 ++++--
 12 files changed, 26 insertions(+), 21 deletions(-)

--- linux-2.6.26-rc2/arch/ia64/mm/hugetlbpage.c	2008-01-24 23:58:37.000000000 +0100
+++ 2.6.26-rc2-free_pgd_range-tlb-param/arch/ia64/mm/hugetlbpage.c	2008-05-15 11:58:09.000000000 +0200
@@ -112,7 +112,7 @@ follow_huge_pmd(struct mm_struct *mm, un
 	return NULL;
 }
 
-void hugetlb_free_pgd_range(struct mmu_gather **tlb,
+void hugetlb_free_pgd_range(struct mmu_gather *tlb,
 			unsigned long addr, unsigned long end,
 			unsigned long floor, unsigned long ceiling)
 {
--- linux-2.6.26-rc2/arch/powerpc/mm/hugetlbpage.c	2008-04-17 04:49:44.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/arch/powerpc/mm/hugetlbpage.c	2008-05-15 11:57:50.000000000 +0200
@@ -255,7 +255,7 @@ static void hugetlb_free_pud_range(struc
  *
  * Must be called with pagetable lock held.
  */
-void hugetlb_free_pgd_range(struct mmu_gather **tlb,
+void hugetlb_free_pgd_range(struct mmu_gather *tlb,
 			    unsigned long addr, unsigned long end,
 			    unsigned long floor, unsigned long ceiling)
 {
@@ -315,13 +315,13 @@ void hugetlb_free_pgd_range(struct mmu_g
 		return;
 
 	start = addr;
-	pgd = pgd_offset((*tlb)->mm, addr);
+	pgd = pgd_offset(tlb->mm, addr);
 	do {
-		BUG_ON(get_slice_psize((*tlb)->mm, addr) != mmu_huge_psize);
+		BUG_ON(get_slice_psize(tlb->mm, addr) != mmu_huge_psize);
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd))
 			continue;
-		hugetlb_free_pud_range(*tlb, pgd, addr, next, floor, ceiling);
+		hugetlb_free_pud_range(tlb, pgd, addr, next, floor, ceiling);
 	} while (pgd++, addr = next, addr != end);
 }
 
--- linux-2.6.26-rc2/fs/exec.c	2008-05-15 14:35:04.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/fs/exec.c	2008-05-15 11:53:59.000000000 +0200
@@ -537,7 +537,7 @@ static int shift_arg_pages(struct vm_are
 		/*
 		 * when the old and new regions overlap clear from new_end.
 		 */
-		free_pgd_range(&tlb, new_end, old_end, new_end,
+		free_pgd_range(tlb, new_end, old_end, new_end,
 			vma->vm_next ? vma->vm_next->vm_start : 0);
 	} else {
 		/*
@@ -546,7 +546,7 @@ static int shift_arg_pages(struct vm_are
 		 * have constraints on va-space that make this illegal (IA64) -
 		 * for the others its just a little faster.
 		 */
-		free_pgd_range(&tlb, old_start, old_end, new_end,
+		free_pgd_range(tlb, old_start, old_end, new_end,
 			vma->vm_next ? vma->vm_next->vm_start : 0);
 	}
 	tlb_finish_mmu(tlb, new_end, old_end);
--- linux-2.6.26-rc2/include/asm-ia64/hugetlb.h	2008-05-15 14:35:10.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-ia64/hugetlb.h	2008-05-15 16:30:02.000000000 +0200
@@ -4,7 +4,7 @@
 #include <asm/page.h>
 
 
-void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
 			    unsigned long end, unsigned long floor,
 			    unsigned long ceiling);
 
--- linux-2.6.26-rc2/include/asm-powerpc/hugetlb.h	2008-05-15 16:29:05.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-powerpc/hugetlb.h	2008-05-15 16:29:21.000000000 +0200
@@ -7,7 +7,7 @@
 int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
 			   unsigned long len);
 
-void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
 			    unsigned long end, unsigned long floor,
 			    unsigned long ceiling);
 
--- linux-2.6.26-rc2/include/asm-sh/hugetlb.h	2008-05-15 14:35:10.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-sh/hugetlb.h	2008-05-15 16:29:49.000000000 +0200
@@ -26,7 +26,7 @@ static inline int prepare_hugepage_range
 static inline void hugetlb_prefault_arch_hook(struct mm_struct *mm) {
 }
 
-static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
+static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
 					  unsigned long addr, unsigned long end,
 					  unsigned long floor,
 					  unsigned long ceiling)
--- linux-2.6.26-rc2/include/asm-sparc64/hugetlb.h	2008-05-15 14:35:11.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-sparc64/hugetlb.h	2008-05-15 16:30:43.000000000 +0200
@@ -31,7 +31,7 @@ static inline int prepare_hugepage_range
 	return 0;
 }
 
-static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
+static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
 					  unsigned long addr, unsigned long end,
 					  unsigned long floor,
 					  unsigned long ceiling)
--- linux-2.6.26-rc2/include/asm-x86/hugetlb.h	2008-05-15 14:35:11.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-x86/hugetlb.h	2008-05-15 16:31:00.000000000 +0200
@@ -26,7 +26,7 @@ static inline int prepare_hugepage_range
 static inline void hugetlb_prefault_arch_hook(struct mm_struct *mm) {
 }
 
-static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
+static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
 					  unsigned long addr, unsigned long end,
 					  unsigned long floor,
 					  unsigned long ceiling)
--- linux-2.6.26-rc2/include/linux/mm.h	2008-05-15 14:35:11.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/include/linux/mm.h	2008-05-15 12:01:17.000000000 +0200
@@ -770,10 +770,8 @@ struct mm_walk {
 int walk_page_range(const struct mm_struct *, unsigned long addr,
 		    unsigned long end, const struct mm_walk *walk,
 		    void *private);
-void free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
+void free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
 		unsigned long end, unsigned long floor, unsigned long ceiling);
-void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *start_vma,
-		unsigned long floor, unsigned long ceiling);
 int copy_page_range(struct mm_struct *dst, struct mm_struct *src,
 			struct vm_area_struct *vma);
 void unmap_mapping_range(struct address_space *mapping,
--- linux-2.6.26-rc2/mm/internal.h	2008-05-15 14:35:13.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/internal.h	2008-05-15 12:01:52.000000000 +0200
@@ -13,6 +13,9 @@
 
 #include <linux/mm.h>
 
+void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
+		unsigned long floor, unsigned long ceiling);
+
 static inline void set_page_count(struct page *page, int v)
 {
 	atomic_set(&page->_count, v);
--- linux-2.6.26-rc2/mm/memory.c	2008-05-15 14:35:13.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/memory.c	2008-05-15 12:03:17.000000000 +0200
@@ -61,6 +61,8 @@
 #include <linux/swapops.h>
 #include <linux/elf.h>
 
+#include "internal.h"
+
 #ifndef CONFIG_NEED_MULTIPLE_NODES
 /* use the per-pgdat data instead for discontigmem - mbligh */
 unsigned long max_mapnr;
@@ -211,7 +213,7 @@ static inline void free_pud_range(struct
  *
  * Must be called with pagetable lock held.
  */
-void free_pgd_range(struct mmu_gather **tlb,
+void free_pgd_range(struct mmu_gather *tlb,
 			unsigned long addr, unsigned long end,
 			unsigned long floor, unsigned long ceiling)
 {
@@ -262,16 +264,16 @@ void free_pgd_range(struct mmu_gather **
 		return;
 
 	start = addr;
-	pgd = pgd_offset((*tlb)->mm, addr);
+	pgd = pgd_offset(tlb->mm, addr);
 	do {
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd))
 			continue;
-		free_pud_range(*tlb, pgd, addr, next, floor, ceiling);
+		free_pud_range(tlb, pgd, addr, next, floor, ceiling);
 	} while (pgd++, addr = next, addr != end);
 }
 
-void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *vma,
+void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		unsigned long floor, unsigned long ceiling)
 {
 	while (vma) {
--- linux-2.6.26-rc2/mm/mmap.c	2008-05-15 14:35:13.000000000 +0200
+++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/mmap.c	2008-05-15 12:03:25.000000000 +0200
@@ -32,6 +32,8 @@
 #include <asm/tlb.h>
 #include <asm/mmu_context.h>
 
+#include "internal.h"
+
 #ifndef arch_mmap_check
 #define arch_mmap_check(addr, len, flags)	(0)
 #endif
@@ -1756,7 +1758,7 @@ static void unmap_region(struct mm_struc
 	update_hiwater_rss(mm);
 	unmap_vmas(&tlb, vma, start, end, &nr_accounted, NULL);
 	vm_unacct_memory(nr_accounted);
-	free_pgtables(&tlb, vma, prev? prev->vm_end: FIRST_USER_ADDRESS,
+	free_pgtables(tlb, vma, prev? prev->vm_end: FIRST_USER_ADDRESS,
 				 next? next->vm_start: 0);
 	tlb_finish_mmu(tlb, start, end);
 }
@@ -2056,7 +2058,7 @@ void exit_mmap(struct mm_struct *mm)
 	/* Use -1 here to ensure all VMAs in the mm are unmapped */
 	end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
 	vm_unacct_memory(nr_accounted);
-	free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, 0);
+	free_pgtables(tlb, vma, FIRST_USER_ADDRESS, 0);
 	tlb_finish_mmu(tlb, 0, end);
 
 	/*

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range() & Co

[Jeremy Fitzhardinge]

Jan Beulich wrote:
> The double indirection here is not needed anywhere and hence (at least)
> confusing.
>   

Oh, good, it wasn't just me.  I was puzzled by the double indirection 
and couldn't see the point.

Acked-by: Jeremy Fitzhardinge <jeremy@goop.org>

    J
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> ---
>  arch/ia64/mm/hugetlbpage.c    |    2 +-
>  arch/powerpc/mm/hugetlbpage.c |    8 ++++----
>  fs/exec.c                     |    4 ++--
>  include/asm-ia64/hugetlb.h    |    2 +-
>  include/asm-powerpc/hugetlb.h |    2 +-
>  include/asm-sh/hugetlb.h      |    2 +-
>  include/asm-sparc64/hugetlb.h |    2 +-
>  include/asm-x86/hugetlb.h     |    2 +-
>  include/linux/mm.h            |    4 +---
>  mm/internal.h                 |    3 +++
>  mm/memory.c                   |   10 ++++++----
>  mm/mmap.c                     |    6 ++++--
>  12 files changed, 26 insertions(+), 21 deletions(-)
>
> --- linux-2.6.26-rc2/arch/ia64/mm/hugetlbpage.c	2008-01-24 23:58:37.000000000 +0100
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/arch/ia64/mm/hugetlbpage.c	2008-05-15 11:58:09.000000000 +0200
> @@ -112,7 +112,7 @@ follow_huge_pmd(struct mm_struct *mm, un
>  	return NULL;
>  }
>  
> -void hugetlb_free_pgd_range(struct mmu_gather **tlb,
> +void hugetlb_free_pgd_range(struct mmu_gather *tlb,
>  			unsigned long addr, unsigned long end,
>  			unsigned long floor, unsigned long ceiling)
>  {
> --- linux-2.6.26-rc2/arch/powerpc/mm/hugetlbpage.c	2008-04-17 04:49:44.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/arch/powerpc/mm/hugetlbpage.c	2008-05-15 11:57:50.000000000 +0200
> @@ -255,7 +255,7 @@ static void hugetlb_free_pud_range(struc
>   *
>   * Must be called with pagetable lock held.
>   */
> -void hugetlb_free_pgd_range(struct mmu_gather **tlb,
> +void hugetlb_free_pgd_range(struct mmu_gather *tlb,
>  			    unsigned long addr, unsigned long end,
>  			    unsigned long floor, unsigned long ceiling)
>  {
> @@ -315,13 +315,13 @@ void hugetlb_free_pgd_range(struct mmu_g
>  		return;
>  
>  	start = addr;
> -	pgd = pgd_offset((*tlb)->mm, addr);
> +	pgd = pgd_offset(tlb->mm, addr);
>  	do {
> -		BUG_ON(get_slice_psize((*tlb)->mm, addr) != mmu_huge_psize);
> +		BUG_ON(get_slice_psize(tlb->mm, addr) != mmu_huge_psize);
>  		next = pgd_addr_end(addr, end);
>  		if (pgd_none_or_clear_bad(pgd))
>  			continue;
> -		hugetlb_free_pud_range(*tlb, pgd, addr, next, floor, ceiling);
> +		hugetlb_free_pud_range(tlb, pgd, addr, next, floor, ceiling);
>  	} while (pgd++, addr = next, addr != end);
>  }
>  
> --- linux-2.6.26-rc2/fs/exec.c	2008-05-15 14:35:04.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/fs/exec.c	2008-05-15 11:53:59.000000000 +0200
> @@ -537,7 +537,7 @@ static int shift_arg_pages(struct vm_are
>  		/*
>  		 * when the old and new regions overlap clear from new_end.
>  		 */
> -		free_pgd_range(&tlb, new_end, old_end, new_end,
> +		free_pgd_range(tlb, new_end, old_end, new_end,
>  			vma->vm_next ? vma->vm_next->vm_start : 0);
>  	} else {
>  		/*
> @@ -546,7 +546,7 @@ static int shift_arg_pages(struct vm_are
>  		 * have constraints on va-space that make this illegal (IA64) -
>  		 * for the others its just a little faster.
>  		 */
> -		free_pgd_range(&tlb, old_start, old_end, new_end,
> +		free_pgd_range(tlb, old_start, old_end, new_end,
>  			vma->vm_next ? vma->vm_next->vm_start : 0);
>  	}
>  	tlb_finish_mmu(tlb, new_end, old_end);
> --- linux-2.6.26-rc2/include/asm-ia64/hugetlb.h	2008-05-15 14:35:10.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-ia64/hugetlb.h	2008-05-15 16:30:02.000000000 +0200
> @@ -4,7 +4,7 @@
>  #include <asm/page.h>
>  
>  
> -void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
> +void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
>  			    unsigned long end, unsigned long floor,
>  			    unsigned long ceiling);
>  
> --- linux-2.6.26-rc2/include/asm-powerpc/hugetlb.h	2008-05-15 16:29:05.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-powerpc/hugetlb.h	2008-05-15 16:29:21.000000000 +0200
> @@ -7,7 +7,7 @@
>  int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
>  			   unsigned long len);
>  
> -void hugetlb_free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
> +void hugetlb_free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
>  			    unsigned long end, unsigned long floor,
>  			    unsigned long ceiling);
>  
> --- linux-2.6.26-rc2/include/asm-sh/hugetlb.h	2008-05-15 14:35:10.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-sh/hugetlb.h	2008-05-15 16:29:49.000000000 +0200
> @@ -26,7 +26,7 @@ static inline int prepare_hugepage_range
>  static inline void hugetlb_prefault_arch_hook(struct mm_struct *mm) {
>  }
>  
> -static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
> +static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
>  					  unsigned long addr, unsigned long end,
>  					  unsigned long floor,
>  					  unsigned long ceiling)
> --- linux-2.6.26-rc2/include/asm-sparc64/hugetlb.h	2008-05-15 14:35:11.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-sparc64/hugetlb.h	2008-05-15 16:30:43.000000000 +0200
> @@ -31,7 +31,7 @@ static inline int prepare_hugepage_range
>  	return 0;
>  }
>  
> -static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
> +static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
>  					  unsigned long addr, unsigned long end,
>  					  unsigned long floor,
>  					  unsigned long ceiling)
> --- linux-2.6.26-rc2/include/asm-x86/hugetlb.h	2008-05-15 14:35:11.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/asm-x86/hugetlb.h	2008-05-15 16:31:00.000000000 +0200
> @@ -26,7 +26,7 @@ static inline int prepare_hugepage_range
>  static inline void hugetlb_prefault_arch_hook(struct mm_struct *mm) {
>  }
>  
> -static inline void hugetlb_free_pgd_range(struct mmu_gather **tlb,
> +static inline void hugetlb_free_pgd_range(struct mmu_gather *tlb,
>  					  unsigned long addr, unsigned long end,
>  					  unsigned long floor,
>  					  unsigned long ceiling)
> --- linux-2.6.26-rc2/include/linux/mm.h	2008-05-15 14:35:11.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/include/linux/mm.h	2008-05-15 12:01:17.000000000 +0200
> @@ -770,10 +770,8 @@ struct mm_walk {
>  int walk_page_range(const struct mm_struct *, unsigned long addr,
>  		    unsigned long end, const struct mm_walk *walk,
>  		    void *private);
> -void free_pgd_range(struct mmu_gather **tlb, unsigned long addr,
> +void free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
>  		unsigned long end, unsigned long floor, unsigned long ceiling);
> -void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *start_vma,
> -		unsigned long floor, unsigned long ceiling);
>  int copy_page_range(struct mm_struct *dst, struct mm_struct *src,
>  			struct vm_area_struct *vma);
>  void unmap_mapping_range(struct address_space *mapping,
> --- linux-2.6.26-rc2/mm/internal.h	2008-05-15 14:35:13.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/internal.h	2008-05-15 12:01:52.000000000 +0200
> @@ -13,6 +13,9 @@
>  
>  #include <linux/mm.h>
>  
> +void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
> +		unsigned long floor, unsigned long ceiling);
> +
>  static inline void set_page_count(struct page *page, int v)
>  {
>  	atomic_set(&page->_count, v);
> --- linux-2.6.26-rc2/mm/memory.c	2008-05-15 14:35:13.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/memory.c	2008-05-15 12:03:17.000000000 +0200
> @@ -61,6 +61,8 @@
>  #include <linux/swapops.h>
>  #include <linux/elf.h>
>  
> +#include "internal.h"
> +
>  #ifndef CONFIG_NEED_MULTIPLE_NODES
>  /* use the per-pgdat data instead for discontigmem - mbligh */
>  unsigned long max_mapnr;
> @@ -211,7 +213,7 @@ static inline void free_pud_range(struct
>   *
>   * Must be called with pagetable lock held.
>   */
> -void free_pgd_range(struct mmu_gather **tlb,
> +void free_pgd_range(struct mmu_gather *tlb,
>  			unsigned long addr, unsigned long end,
>  			unsigned long floor, unsigned long ceiling)
>  {
> @@ -262,16 +264,16 @@ void free_pgd_range(struct mmu_gather **
>  		return;
>  
>  	start = addr;
> -	pgd = pgd_offset((*tlb)->mm, addr);
> +	pgd = pgd_offset(tlb->mm, addr);
>  	do {
>  		next = pgd_addr_end(addr, end);
>  		if (pgd_none_or_clear_bad(pgd))
>  			continue;
> -		free_pud_range(*tlb, pgd, addr, next, floor, ceiling);
> +		free_pud_range(tlb, pgd, addr, next, floor, ceiling);
>  	} while (pgd++, addr = next, addr != end);
>  }
>  
> -void free_pgtables(struct mmu_gather **tlb, struct vm_area_struct *vma,
> +void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  		unsigned long floor, unsigned long ceiling)
>  {
>  	while (vma) {
> --- linux-2.6.26-rc2/mm/mmap.c	2008-05-15 14:35:13.000000000 +0200
> +++ 2.6.26-rc2-free_pgd_range-tlb-param/mm/mmap.c	2008-05-15 12:03:25.000000000 +0200
> @@ -32,6 +32,8 @@
>  #include <asm/tlb.h>
>  #include <asm/mmu_context.h>
>  
> +#include "internal.h"
> +
>  #ifndef arch_mmap_check
>  #define arch_mmap_check(addr, len, flags)	(0)
>  #endif
> @@ -1756,7 +1758,7 @@ static void unmap_region(struct mm_struc
>  	update_hiwater_rss(mm);
>  	unmap_vmas(&tlb, vma, start, end, &nr_accounted, NULL);
>  	vm_unacct_memory(nr_accounted);
> -	free_pgtables(&tlb, vma, prev? prev->vm_end: FIRST_USER_ADDRESS,
> +	free_pgtables(tlb, vma, prev? prev->vm_end: FIRST_USER_ADDRESS,
>  				 next? next->vm_start: 0);
>  	tlb_finish_mmu(tlb, start, end);
>  }
> @@ -2056,7 +2058,7 @@ void exit_mmap(struct mm_struct *mm)
>  	/* Use -1 here to ensure all VMAs in the mm are unmapped */
>  	end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
>  	vm_unacct_memory(nr_accounted);
> -	free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, 0);
> +	free_pgtables(tlb, vma, FIRST_USER_ADDRESS, 0);
>  	tlb_finish_mmu(tlb, 0, end);
>  
>  	/*
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range() & Co

[Hugh Dickins]

On Thu, 15 May 2008, Jan Beulich wrote:
> The double indirection here is not needed anywhere and hence (at least)
> confusing.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

Please hold off on this.  I was writing a mail to say that it should be
okay (might as well include what I wrote then below), then checked and
noticed that Andrea/Christoph have an mmu_notifier patch which removes
that argument to free_pgtables (it could alternatively keep the argument,
but make use of the double indirection).  It's not yet clear whether
or when their patch goes in, but let's not obstruct their functional
enhancements with this cleanup.

What I originally wrote was...
I had to look up my original comment on it:

    Pass mmu_gather** in the
    public interfaces, since we might want to add latency lockdrops later;
    but no attempt to do so yet, going by vma should itself reduce latency.

By "latency lockdrops" I meant the tlb_finish_mmu/tlb_gather_mmu pair
you find in unmap_vmas: I expected free_pgtables to need the same soon.

free_pgtables does have a latency issue, but not so bad that we've rushed
in to reduce it, at the expense of increasing TLB flushes, and ugliness.

We'd several of us like to rework the mmu_gathering so as not to disable
preemption: I got stuck and BenH took over, a patch much like yours below
was a part of what I had - I too was glad to get rid of the **s.

So, assuming we won't suddenly need a quick hack fix to free_pgtables
latency before one of us fixes the wider mmu_gathering issues, your
patch should be fine (and could be reverted if a sudden need occurs).

But... could I ask you to omit that move from include/linux/mm.h to
mm/internal.h?  To me that's nothing but an irritation, and internal.h
is the last place I think of to look for anything.  There's a thousand
other things internal to mm which aren't in it, and personally I wouldn't
even be thrilled by a janitorial project to move stuff over to it in bulk.

But, returning to the start, please let's hold yours back after all.

Thanks,
Hugh

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range()& Co

[Jan Beulich]

>>> Hugh Dickins <hugh@veritas.com> 16.05.08 16:39 >>>
>    Pass mmu_gather** in the
>    public interfaces, since we might want to add latency lockdrops later;
>    but no attempt to do so yet, going by vma should itself reduce latency.
>...
>But, returning to the start, please let's hold yours back after all.

If you want to keep and actively use the double indirection, would you
guarantee current semantics like ->mm and ->fullmm to always be what
was specified at the start of the operation? Verifying this was what
made me go through that call tree in the first place, and obviously this
is harder to verify when there is potential for the whole structure
pointed to to be replaced by another one.

Jan

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range()& Co

[Hugh Dickins]

On Fri, 16 May 2008, Jan Beulich wrote:
> 
> If you want to keep and actively use the double indirection, would you

I'd much rather replace it with something better, but it's not easy
to solve all the problems which Ben and I variously wanted to address.

> guarantee current semantics like ->mm and ->fullmm to always be what
> was specified at the start of the operation?

Of course.  The stopping and restarting of those gathers was never
intended to switch mm midstream, and there's that odd way in which
unmap_vmas saves the current value of fullmm to provide to the next
gathering.  Not the nicest interface, I agree; but it's not widespread.

> Verifying this was what
> made me go through that call tree in the first place, and obviously this
> is harder to verify when there is potential for the whole structure
> pointed to to be replaced by another one.

Or, looking at it from the point of a cpu rather than from the point
of view of the call chain, of course not.  Typically the mmu_gather_t
(in that current implementation) is a per-cpu area, and the call chain
may switch over to a different cpu's mmu_gather_t in between stopping
and restarting, and the original cpu's be used for a different mm.

Hugh

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range() & Co

[Benjamin Herrenschmidt]

On Fri, 2008-05-16 at 15:39 +0100, Hugh Dickins wrote:
> 
> We'd several of us like to rework the mmu_gathering so as not to
> disable
> preemption: I got stuck and BenH took over, a patch much like yours
> below
> was a part of what I had - I too was glad to get rid of the **s.

Yup. I took over and somewhat froze.. I have some stuff partially done
(and that actually works) that changes the batching, moving it away from
per-cpu and improving the latency situation but I wasn't to happy with
some aspects of the resulting patches, and finally got pulled on various
other things, so this work took the dust.

I can try to dig my stuff out when I'm back from travelling, in a week
or so, if there is some interest. I was thinking about looking at it
again anyway.

Cheers,
Ben.

Re: [PATCH] remove double indirection on tlb parameter to free_pgd_range()& Co

[Benjamin Herrenschmidt]

On Fri, 2008-05-16 at 16:30 +0100, Jan Beulich wrote:
> >>> Hugh Dickins <hugh@veritas.com> 16.05.08 16:39 >>>
> >    Pass mmu_gather** in the
> >    public interfaces, since we might want to add latency lockdrops later;
> >    but no attempt to do so yet, going by vma should itself reduce latency.
> >...
> >But, returning to the start, please let's hold yours back after all.
> 
> If you want to keep and actively use the double indirection, would you
> guarantee current semantics like ->mm and ->fullmm to always be what
> was specified at the start of the operation? Verifying this was what
> made me go through that call tree in the first place, and obviously this
> is harder to verify when there is potential for the whole structure
> pointed to to be replaced by another one.

In my patches, I did drop the double indirection and just updated the
fields inside of the batch. Worked fine.

Ben.