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

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

@@ -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);

@@ -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);

	/*

@@ -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) {

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

@@ -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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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.

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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.

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hugh, Nick,

Thank you for the information.

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>