Any reason for passing "tlb" to "free_pgtables()" by address?

Any reason for passing "tlb" to "free_pgtables()" by address?

[Zoltan Menyhart]

Apparently, there is no reason for passing "tlb" to "free_pgtables()"
by address, because there is no need for re-scheduling inside this
function => no other "mmu_gather" can / will be used.

Thanks,

Zoltan

--- linux-2.6.16.9-save/include/linux/mm.h	2006-04-19 08:10:14.000000000 +0200
+++ linux-2.6.16.9/include/linux/mm.h	2006-05-05 11:33:56.000000000 +0200
@@ -700,9 +700,9 @@
 		struct vm_area_struct *start_vma, unsigned long start_addr,
 		unsigned long end_addr, unsigned long *nr_accounted,
 		struct zap_details *);
-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,
+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);
--- linux-2.6.16.9-save/mm/mmap.c	2006-04-19 08:10:14.000000000 +0200
+++ linux-2.6.16.9/mm/mmap.c	2006-05-05 11:31:51.000000000 +0200
@@ -1661,7 +1661,7 @@
 	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);
 }
@@ -1944,7 +1944,7 @@
 	/* 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);
 
 	/*
--- linux-2.6.16.9-save/mm/memory.c	2006-04-19 08:10:14.000000000 +0200
+++ linux-2.6.16.9/mm/memory.c	2006-05-05 11:30:27.000000000 +0200
@@ -201,7 +201,7 @@
  *
  * 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)
 {
@@ -252,19 +252,19 @@
 		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);
 
-	if (!(*tlb)->fullmm)
-		flush_tlb_pgtables((*tlb)->mm, start, end);
+	if (!tlb->fullmm)
+		flush_tlb_pgtables(tlb->mm, start, 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.16.9-save/include/asm-ia64/pgtable.h	2006-04-21 09:59:12.000000000 +0200
+++ linux-2.6.16.9/include/asm-ia64/pgtable.h	2006-05-05 11:49:21.000000000 +0200
@@ -506,7 +506,7 @@
 #define HUGETLB_PGDIR_SIZE	(__IA64_UL(1) << HUGETLB_PGDIR_SHIFT)
 #define HUGETLB_PGDIR_MASK	(~(HUGETLB_PGDIR_SIZE-1))
 struct mmu_gather;
-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);
 #endif
 
--- linux-2.6.16.9-save/arch/ia64/mm/hugetlbpage.c	2006-04-21 09:58:55.000000000 +0200
+++ linux-2.6.16.9/arch/ia64/mm/hugetlbpage.c	2006-05-05 11:48:46.000000000 +0200
@@ -107,7 +107,7 @@
 	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)
 {

--
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>

Re: Any reason for passing "tlb" to "free_pgtables()" by address?

[Hugh Dickins]

On Fri, 5 May 2006, Zoltan Menyhart wrote:
> Apparently, there is no reason for passing "tlb" to "free_pgtables()"
> by address, because there is no need for re-scheduling inside this
> function => no other "mmu_gather" can / will be used.

You're right.  Well, actually, it's been shown that there _is_ a need
for rescheduling inside that function (high latency), but we've not
yet settled on the right way to go about that - mmu_gathering needs
overhaul to avoid disabling preemption, something both Nick and I
have worked on intermittently.

Personally I'd prefer not to make your change right now - it seems
a shame to make that cosmetic change without addressing the real
latency issue; but I've no strong feeling against your patch.

Hugh

--
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>

Re: Any reason for passing "tlb" to "free_pgtables()" by address?

[Zoltan Menyhart]

Hugh Dickins wrote:

> Personally I'd prefer not to make your change right now - it seems
> a shame to make that cosmetic change without addressing the real
> latency issue; but I've no strong feeling against your patch.

Could you please explain what your plans are?

How much do you think it is worth to optimize "free_pgtables()",
knowing that:
- PTE, PMD and PUD pages are freed seldom (wrt. the leaf pages)
- The number of these pages is much more less than
   that of the leaf pages.

Thanks,

Zoltan

--
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>

Re: Any reason for passing "tlb" to "free_pgtables()" by address?

[Nick Piggin]

Zoltan Menyhart wrote:

> Hugh Dickins wrote:
>
>> Personally I'd prefer not to make your change right now - it seems
>> a shame to make that cosmetic change without addressing the real
>> latency issue; but I've no strong feeling against your patch.
>
>
> Could you please explain what your plans are?


Long term, we would like to make the mmu_gather paths preemptible and
reentrant so the latency hacks can go away, and we don't end up with
awful things like a tlb flush after unmapping every 8 pages for
CONFIG_PREEMPT.

I posted a quick RFC a while back to implement my "gather in place"
idea: http://www.ussg.iu.edu/hypermail/linux/kernel/0603.2/0499.html

Hugh has a different approach, but neither is particularly urgent at
this stage.

In the short term, we might still like to be able to do latency breaks
in free_pgtables so it would make sense to keep the code the way it is.

>
> How much do you think it is worth to optimize "free_pgtables()",
> knowing that:
> - PTE, PMD and PUD pages are freed seldom (wrt. the leaf pages)
> - The number of these pages is much more less than
>   that of the leaf pages.


Virtual address space can still be vast and sparse. Even with 32 bits,
we could probably trigger high latencies here.

Nick
--

Send instant messages to your online friends http://au.messenger.yahoo.com 

--
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>

Re: Any reason for passing "tlb" to "free_pgtables()" by address?

[Hugh Dickins]

On Mon, 8 May 2006, Zoltan Menyhart wrote:
> 
> Could you please explain what your plans are?

Nick already answered, and I agree with his answer;
but I get the feeling you'd like to hear it from me too.

My plan (as his) is to rework the mmu_gather TLB flush batching
so that it can be done without disabling preemption, so that it
does not add to latency.

Nick uses the pagetables themselves as buffering, I allocate a
temporary buffer: in each case we abandon the per-cpu arrays which
need preemption disabled.  But neither patch is good enough yet.

> How much do you think it is worth to optimize "free_pgtables()",
> knowing that:
> - PTE, PMD and PUD pages are freed seldom (wrt. the leaf pages)
> - The number of these pages is much more less than
>   that of the leaf pages.

Not worth much at all in comparison: it just falls automatically
out of the mmu_gathering rework which covers the leaf pages too.

But free_pgtables is where I got caught out inadvertently adding
latency in 2.6.15, so I've a responsibility to correct that case.

Hugh

--
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>

Re: Any reason for passing "tlb" to "free_pgtables()" by address?

[Zoltan Menyhart]

Hugh, Nick,

Thank you for the information.

> Nick uses the pagetables themselves as buffering, I allocate a
> temporary buffer: in each case we abandon the per-cpu arrays which
> need preemption disabled.  But neither patch is good enough yet.

I'd like to note that the page table walking:

	TLB = ... -> pgd[i] -> pud[j] -> pmd[k] -> pte[l]

is not safe on some of the architectures which load TLB entries
"by hand", i.e. by use of some low level assembly routines,
e.g. IA64, Power.
Please refer to the thread "RFC: RCU protected page table walking".

Only some "careful programming" in the PUD / PMD / PTE page removal
code can help.
I propose a way to make sure that the page table walkers will be
able to finish their walks in safety; we release a directory page
when no more walker can reference the page.

Therefore I'd appreciate much a solution that would not use the
directory pages themselves as buffering. I'll need to preserve them
until the RCU based reclaim side.

Thanks,

Zoltan

--
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>