more detailed per-process transparent hugepage statistics

The khugepaged process collapses transparent hugepages for us. Whenever it collapses a page into a transparent hugepage, we increment a nice global counter exported in sysfs:

/sys/kernel/mm/transparent_hugepage/khugepaged/pages_collapsed

But, transparent hugepages also get broken down in quite a few places in the kernel. We do not have a good idea how how many of those collpased pages are "new" versus how many are just fixing up spots that got split a moment before.

Note: "splits" and "collapses" are opposites in this context.

This patch adds a new sysfs file:

/sys/kernel/mm/transparent_hugepage/pages_split

It is global, like "pages_collapsed", and is incremented whenever any transparent hugepage on the system has been broken down in to normal PAGE_SIZE base pages. This way, we can get an idea how well khugepaged is keeping up collapsing pages that have been split.

I put it under /sys/kernel/mm/transparent_hugepage/ instead of the khugepaged/ directory since it is not strictly related to khugepaged; it can get incremented on pages other than those collapsed by khugepaged.

The variable storing this is a plain integer. I needs the same amount of locking that 'khugepaged_pages_collapsed' has, for instance.

Signed-off-by Dave Hansen <[email protected]>

The khugepaged process collapses transparent hugepages for us. Whenever it collapses a page into a transparent hugepage, we increment a nice global counter exported in sysfs:

/sys/kernel/mm/transparent_hugepage/khugepaged/pages_collapsed

But, transparent hugepages also get broken down in quite a few places in the kernel. We do not have a good idea how how many of those collpased pages are "new" versus how many are just fixing up spots that got split a moment before.

Note: "splits" and "collapses" are opposites in this context.

This patch adds a new sysfs file:

/sys/kernel/mm/transparent_hugepage/pages_split

It is global, like "pages_collapsed", and is incremented whenever any transparent hugepage on the system has been broken down in to normal PAGE_SIZE base pages. This way, we can get an idea how well khugepaged is keeping up collapsing pages that have been split.

I put it under /sys/kernel/mm/transparent_hugepage/ instead of the khugepaged/ directory since it is not strictly related to khugepaged; it can get incremented on pages other than those collapsed by khugepaged.

The variable storing this is a plain integer. I needs the same amount of locking that 'khugepaged_pages_collapsed' has, for instance.

Signed-off-by Dave Hansen <[email protected]>

i.e. no global locking, but we've accepted the occassional off-by-one error (even though splitting of hugepages isn't by any means lightning fast and the overhead of atomic ops would be negligible).

Quoted 3 lines · David Rientjes

i.e. no global locking, but we've accepted the occassional off-by-one error (even though splitting of hugepages isn't by any means lightning fast and the overhead of atomic ops would be negligible).

Agreed losing an increment is not a problem, but in very large systems it will become a bottleneck. It's not super urgent, but I think it needs to become a per-cpu counter sooner than later (not needed immediately but I would appreciate an incremental patch soon to address that).

Right now, if a mm_walk has either ->pte_entry or ->pmd_entry set, it will unconditionally split and transparent huge pages it runs in to. In practice, that means that anyone doing a

cat /proc/$pid/smaps

will unconditionally break down every huge page in the process and depend on khugepaged to re-collapse it later. This is fairly suboptimal.

This patch changes that behavior. It teaches each ->pmd_entry handler (there are three) that they must break down the THPs themselves. Also, the _generic_ code will never break down a THP unless a ->pte_entry handler is actually set.

This means that the ->pmd_entry handlers can now choose to deal with THPs without breaking them down.

Quoted 15 lines · Dave Hansen

Right now, if a mm_walk has either ->pte_entry or ->pmd_entry set, it will unconditionally split and transparent huge pages it runs in to. In practice, that means that anyone doing a

cat /proc/$pid/smaps

will unconditionally break down every huge page in the process and depend on khugepaged to re-collapse it later. This is fairly suboptimal.

This patch changes that behavior. It teaches each ->pmd_entry handler (there are three) that they must break down the THPs themselves. Also, the _generic_ code will never break down a THP unless a ->pte_entry handler is actually set.

Quoted 2 lines

This means that the ->pmd_entry handlers can now choose to deal with THPs without breaking them down.

Makes perfect sense. But you forgot to push down the splitting into the two handlers in mm/memcontrol.c.

Right now, if a mm_walk has either ->pte_entry or ->pmd_entry set, it will unconditionally split and transparent huge pages it runs in to. In practice, that means that anyone doing a

cat /proc/$pid/smaps

will unconditionally break down every huge page in the process and depend on khugepaged to re-collapse it later. This is fairly suboptimal.

This patch changes that behavior. It teaches each ->pmd_entry handler (there are three) that they must break down the THPs themselves. Also, the _generic_ code will never break down a THP unless a ->pte_entry handler is actually set.

This means that the ->pmd_entry handlers can now choose to deal with THPs without breaking them down.

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |    6 ++++++
 linux-2.6.git-dave/mm/pagewalk.c      |   24 ++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff -puN mm/pagewalk.c~pagewalk-dont-always-split-thp mm/pagewalk.c

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +14 -2

 		if (walk->pmd_entry)
 			err = walk->pmd_entry(pmd, addr, next, walk);
-		if (!err && walk->pte_entry)
-			err = walk_pte_range(pmd, addr, next, walk);
+		if (err)
+			break;
+
+		/*
+		 * Check this here so we only break down trans_huge
+		 * pages when we _need_ to
+		 */
+		if (!walk->pte_entry)
+			continue;
+
+		split_huge_page_pmd(walk->mm, pmd);
+		if (pmd_none_or_clear_bad(pmd))
+			goto again;
+		err = walk_pte_range(pmd, addr, next, walk);
 		if (err)
 			break;
 	} while (pmd++, addr = next, addr != end);

diff -puN fs/proc/task_mmu.c~pagewalk-dont-always-split-thp fs/proc/task_mmu.c

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~pagewalk-dont-always-split-thp +4 -0

@@ -343,6 +343,8 @@ static int smaps_pte_range(pmd_t *pmd, u
 	struct page *page;
 	int mapcount;
 
+	split_huge_page_pmd(walk->mm, pmd);
+
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE) {
 		ptent = *pte;

@@ -467,6 +469,8 @@ static int clear_refs_pte_range(pmd_t *p
 	spinlock_t *ptl;
 	struct page *page;
 
+	split_huge_page_pmd(walk->mm, pmd);
+
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
 	for (; addr != end; pte++, addr += PAGE_SIZE) {
 		ptent = *pte;

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 36

Quoted 8 lines

diff -puN mm/pagewalk.c~pagewalk-dont-always-split-thp mm/pagewalk.c

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +1 -0

@@ -33,19 +33,35 @@ static int walk_pmd_range(pud_t *pud, un
 
 	pmd = pmd_offset(pud, addr);
 	do {
+	again:

checkpatch will warn about the indent.

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 38

Quoted 4 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +1 -2

 		next = pmd_addr_end(addr, end);
-		split_huge_page_pmd(walk->mm, pmd);
-		if (pmd_none_or_clear_bad(pmd)) {
+		if (pmd_none(*pmd)) {

Not sure why this has been changed from pmd_none_or_clear_bad(), that's been done even prior to THP.

Quoted 4 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +0 -0

 			if (walk->pte_hole)
 				err = walk->pte_hole(addr, next, walk);
 			if (err)
 				break;

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 48

Quoted 6 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +4 -0

 			continue;
 		}
+		/*
+		 * This implies that each ->pmd_entry() handler
+		 * needs to know about pmd_trans_huge() pmds
+		 */

Probably needs to be documented somewhere for users of pagewalk?

Quoted 11 lines

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 36

Quoted 8 lines

diff -puN mm/pagewalk.c~pagewalk-dont-always-split-thp mm/pagewalk.c

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +1 -0

@@ -33,19 +33,35 @@ static int walk_pmd_range(pud_t *pud, un
 
 	pmd = pmd_offset(pud, addr);
 	do {
+	again:

checkpatch will warn about the indent.

Quoted 7 lines

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 38

Quoted 4 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +1 -2

 		next = pmd_addr_end(addr, end);
-		split_huge_page_pmd(walk->mm, pmd);
-		if (pmd_none_or_clear_bad(pmd)) {
+		if (pmd_none(*pmd)) {

Not sure why this has been changed from pmd_none_or_clear_bad(), that's been done even prior to THP.

The bad check will trigger on huge pmds. We can not use it here. We can, however, use pmd_none(). The bad check was moved below to where we actually dereference the pmd.

Quoted 4 lines

Quoted 4 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +0 -0

 			if (walk->pte_hole)
 				err = walk->pte_hole(addr, next, walk);
 			if (err)
 				break;

Quoted 8 lines

mm/pagewalk.c static int walk_pmd_range(pud_t *pud, un · line 48

Quoted 6 lines

mm/pagewalk.c renamed from mm/pagewalk.c~pagewalk-dont-always-split-thp +4 -0

 			continue;
 		}
+		/*
+		 * This implies that each ->pmd_entry() handler
+		 * needs to know about pmd_trans_huge() pmds
+		 */

Probably needs to be documented somewhere for users of pagewalk?

Probably, but we don't currently have any central documentation for it. Guess we could make some, or just ensure that all the users got updated. Any ideas where to put it other than the mm_walk struct?

Quoted 3 lines

Probably, but we don't currently have any central documentation for it. Guess we could make some, or just ensure that all the users got updated. Any ideas where to put it other than the mm_walk struct?

I think noting it where struct mm_walk is declared would be best (just a "/* must handle pmd_trans_huge() */" would be sufficient) although eventually it might be cleaner to add a ->pmd_huge_entry().

For code maintenance, I really like _not_ hiding this in the API somewhere. This way, we have a great, self-explanatory tag wherever code (possibly) hasn't properly dealt with THPs. We get a nice, greppable, cscope'able:

split_huge_page_pmd()

wherever we need to "teach" the code about THP.

It's kinda like the BKL. :)

We will use smaps_pte_entry() in a moment to handle both small and transparent large pages. But, we must break it out of smaps_pte_range() first.

We will use smaps_pte_entry() in a moment to handle both small and transparent large pages. But, we must break it out of smaps_pte_range() first.

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |   85 ++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 39 deletions(-)

diff -puN fs/proc/task_mmu.c~break-out-smaps_pte_entry fs/proc/task_mmu.c

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~break-out-smaps_pte_entry +46 -39

@@ -333,56 +333,63 @@ struct mem_size_stats {
 	u64 pss;
 };
 
-static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
-			   struct mm_walk *walk)
+
+static void smaps_pte_entry(pte_t ptent, unsigned long addr,
+		struct mm_walk *walk)
 {
 	struct mem_size_stats *mss = walk->private;
 	struct vm_area_struct *vma = mss->vma;
-	pte_t *pte, ptent;
-	spinlock_t *ptl;
 	struct page *page;
 	int mapcount;
 
-	split_huge_page_pmd(walk->mm, pmd);
-
-	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
-	for (; addr != end; pte++, addr += PAGE_SIZE) {
-		ptent = *pte;
+	if (is_swap_pte(ptent)) {
+		mss->swap += PAGE_SIZE;
+		return;
+	}
 
-		if (is_swap_pte(ptent)) {
-			mss->swap += PAGE_SIZE;
-			continue;
-		}
+	if (!pte_present(ptent))
+		return;
 
-		if (!pte_present(ptent))
-			continue;
+	page = vm_normal_page(vma, addr, ptent);
+	if (!page)
+		return;
+
+	if (PageAnon(page))
+		mss->anonymous += PAGE_SIZE;
+
+	mss->resident += PAGE_SIZE;
+	/* Accumulate the size in pages that have been accessed. */
+	if (pte_young(ptent) || PageReferenced(page))
+		mss->referenced += PAGE_SIZE;
+	mapcount = page_mapcount(page);
+	if (mapcount >= 2) {
+		if (pte_dirty(ptent) || PageDirty(page))
+			mss->shared_dirty += PAGE_SIZE;
+		else
+			mss->shared_clean += PAGE_SIZE;
+		mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount;
+	} else {
+		if (pte_dirty(ptent) || PageDirty(page))
+			mss->private_dirty += PAGE_SIZE;
+		else
+			mss->private_clean += PAGE_SIZE;
+		mss->pss += (PAGE_SIZE << PSS_SHIFT);
+	}
+}
 
-		page = vm_normal_page(vma, addr, ptent);
-		if (!page)
-			continue;
+static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
+			   struct mm_walk *walk)
+{
+	struct mem_size_stats *mss = walk->private;
+	struct vm_area_struct *vma = mss->vma;
+	pte_t *pte;
+	spinlock_t *ptl;
 
-		if (PageAnon(page))
-			mss->anonymous += PAGE_SIZE;
+	split_huge_page_pmd(walk->mm, pmd);
 
-		mss->resident += PAGE_SIZE;
-		/* Accumulate the size in pages that have been accessed. */
-		if (pte_young(ptent) || PageReferenced(page))
-			mss->referenced += PAGE_SIZE;
-		mapcount = page_mapcount(page);
-		if (mapcount >= 2) {
-			if (pte_dirty(ptent) || PageDirty(page))
-				mss->shared_dirty += PAGE_SIZE;
-			else
-				mss->shared_clean += PAGE_SIZE;
-			mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount;
-		} else {
-			if (pte_dirty(ptent) || PageDirty(page))
-				mss->private_dirty += PAGE_SIZE;
-			else
-				mss->private_clean += PAGE_SIZE;
-			mss->pss += (PAGE_SIZE << PSS_SHIFT);
-		}
-	}
+	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+	for (; addr != end; pte++, addr += PAGE_SIZE)
+		smaps_pte_entry(*pte, addr, walk);
 	pte_unmap_unlock(pte - 1, ptl);
 	cond_resched();
 	return 0;

diff -puN mm/huge_memory.c~break-out-smaps_pte_entry mm/huge_memory.c _

Quoted 3 lines · Dave Hansen

We will use smaps_pte_entry() in a moment to handle both small and transparent large pages. But, we must break it out of smaps_pte_range() first.

The extraction from smaps_pte_range() looks good. What's the performance impact on very frequent consumers of /proc/pid/smaps, though, as the result of the calls throughout the iteration if smaps_pte_entry() doesn't get inlined (supposedly because you'll be reusing the extracted function again elsewhere)?

Quoted 9 lines

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~break-out-smaps_pte_entry +3 -1

-	}
+	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+	for (; addr != end; pte++, addr += PAGE_SIZE)
+		smaps_pte_entry(*pte, addr, walk);
 	pte_unmap_unlock(pte - 1, ptl);
 	cond_resched();
 	return 0;

diff -puN mm/huge_memory.c~break-out-smaps_pte_entry mm/huge_memory.c _

Is there a missing change to mm/huge_memory.c?

This patch adds an argument to the new smaps_pte_entry() function to let it account in things other than PAGE_SIZE units. I changed all of the PAGE_SIZE sites, even though not all of them can be reached for transparent huge pages, just so this will continue to work without changes as THPs are improved.

This patch adds an argument to the new smaps_pte_entry() function to let it account in things other than PAGE_SIZE units. I changed all of the PAGE_SIZE sites, even though not all of them can be reached for transparent huge pages, just so this will continue to work without changes as THPs are improved.

This adds code to explicitly detect and handle pmd_trans_huge() pmds. It then passes HPAGE_SIZE units in to the smap_pte_entry() function instead of PAGE_SIZE.

This means that using /proc/$pid/smaps now will no longer cause THPs to be broken down in to small pages.

Signed-off-by Dave Hansen <[email protected]>

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff -puN fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds fs/proc/task_mmu.c

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +3 -0

@@ -1,5 +1,6 @@
 #include <linux/mm.h>
 #include <linux/hugetlb.h>
+#include <linux/huge_mm.h>
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/highmem.h>

@@ -385,6 +386,17 @@ static int smaps_pte_range(pmd_t *pmd, u
 	pte_t *pte;
 	spinlock_t *ptl;
 
+	if (pmd_trans_huge(*pmd)) {
+		if (pmd_trans_splitting(*pmd)) {

fs/proc/task_mmu.c static int smaps_pte_range(pmd_t *pmd, u · line 400

Quoted 12 lines · Dave Hansen

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +11 -0

+	if (pmd_trans_huge(*pmd)) {
+		if (pmd_trans_splitting(*pmd)) {
+			spin_unlock(&walk->mm->page_table_lock);
+			wait_split_huge_page(vma->anon_vma, pmd);
+			spin_lock(&walk->mm->page_table_lock);
+			goto normal_ptes;
+		}
+		smaps_pte_entry(*(pte_t *)pmd, addr, HPAGE_SIZE, walk);
+		return 0;
+	}
+normal_ptes:
 	split_huge_page_pmd(walk->mm, pmd);

This line can go away now...?

Quoted 15 lines · Johannes Weiner

fs/proc/task_mmu.c static int smaps_pte_range(pmd_t *pmd, u · line 400

Quoted 12 lines · Dave Hansen

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +11 -0

+	if (pmd_trans_huge(*pmd)) {
+		if (pmd_trans_splitting(*pmd)) {
+			spin_unlock(&walk->mm->page_table_lock);
+			wait_split_huge_page(vma->anon_vma, pmd);
+			spin_lock(&walk->mm->page_table_lock);
+			goto normal_ptes;
+		}
+		smaps_pte_entry(*(pte_t *)pmd, addr, HPAGE_SIZE, walk);
+		return 0;
+	}
+normal_ptes:
 	split_huge_page_pmd(walk->mm, pmd);

This line can go away now...?

I did this because I was unsure what keeps khugepaged away from the newly-split ptes between the wait_split_huge_page() and the reacquisition of the mm->page_table_lock. mmap_sem, perhaps?

Looking at follow_page() and some of the other wait_split_huge_page(), it looks like this is unnecessary.

This adds code to explicitly detect and handle pmd_trans_huge() pmds. It then passes HPAGE_SIZE units in to the smap_pte_entry() function instead of PAGE_SIZE.

This means that using /proc/$pid/smaps now will no longer cause THPs to be broken down in to small pages.

Signed-off-by Dave Hansen <[email protected]>

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff -puN fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds fs/proc/task_mmu.c

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +2 -0

@@ -1,5 +1,6 @@
 #include <linux/mm.h>
 #include <linux/hugetlb.h>
+#include <linux/huge_mm.h>
 #include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/highmem.h>

@@ -385,6 +386,17 @@ static int smaps_pte_range(pmd_t *pmd, u
 	pte_t *pte;
 	spinlock_t *ptl;
 
+	if (pmd_trans_huge(*pmd)) {

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +0 -0

 	split_huge_page_pmd(walk->mm, pmd);
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);

diff -puN mm/vmscan.c~teach-smaps_pte_range-about-thp-pmds mm/vmscan.c
diff -puN include/trace/events/vmscan.h~teach-smaps_pte_range-about-thp-pmds include/trace/events/vmscan.h diff -puN mm/pagewalk.c~teach-smaps_pte_range-about-thp-pmds mm/pagewalk.c
diff -puN mm/huge_memory.c~teach-smaps_pte_range-about-thp-pmds mm/huge_memory.c
diff -puN mm/memory.c~teach-smaps_pte_range-about-thp-pmds mm/memory.c
diff -puN include/linux/huge_mm.h~teach-smaps_pte_range-about-thp-pmds include/linux/huge_mm.h diff -puN mm/internal.h~teach-smaps_pte_range-about-thp-pmds mm/internal.h
_

Quoted 4 lines

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +0 -0

@@ -385,6 +386,17 @@ static int smaps_pte_range(pmd_t *pmd, u
 	pte_t *pte;
 	spinlock_t *ptl;
 

fs/proc/task_mmu.c static int smaps_pte_range(pmd_t *pmd, u · line 399

Quoted 11 lines

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +11 -0

+	if (pmd_trans_huge(*pmd)) {
+		if (pmd_trans_splitting(*pmd)) {
+			spin_unlock(&walk->mm->page_table_lock);
+			wait_split_huge_page(vma->anon_vma, pmd);
+			spin_lock(&walk->mm->page_table_lock);
+			goto normal_ptes;
+		}
+		smaps_pte_entry(*(pte_t *)pmd, addr, HPAGE_SIZE, walk);
+		return 0;
+	}
+normal_ptes:

Small nitpick: the label isn't necessary, just use an else-clause on your nested conditional.

Quoted 11 lines

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~teach-smaps_pte_range-about-thp-pmds +0 -0

 	split_huge_page_pmd(walk->mm, pmd);
 
 	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);

diff -puN mm/vmscan.c~teach-smaps_pte_range-about-thp-pmds mm/vmscan.c
diff -puN include/trace/events/vmscan.h~teach-smaps_pte_range-about-thp-pmds include/trace/events/vmscan.h diff -puN mm/pagewalk.c~teach-smaps_pte_range-about-thp-pmds mm/pagewalk.c
diff -puN mm/huge_memory.c~teach-smaps_pte_range-about-thp-pmds mm/huge_memory.c
diff -puN mm/memory.c~teach-smaps_pte_range-about-thp-pmds mm/memory.c
diff -puN include/linux/huge_mm.h~teach-smaps_pte_range-about-thp-pmds include/linux/huge_mm.h diff -puN mm/internal.h~teach-smaps_pte_range-about-thp-pmds mm/internal.h
_

What are all these?

Now that the mere act of _looking_ at /proc/$pid/smaps will not destroy transparent huge pages, tell how much of the VMA is actually mapped with them.

This way, we can make sure that we're getting THPs where we expect to see them.

Signed-off-by Dave Hansen <[email protected]>

Now that the mere act of _looking_ at /proc/$pid/smaps will not destroy transparent huge pages, tell how much of the VMA is actually mapped with them.

This way, we can make sure that we're getting THPs where we expect to see them.

Signed-off-by Dave Hansen <[email protected]>

I'm working on some more reports that transparent huge pages and KSM do not play nicely together. Basically, whenever THP's are present along with KSM, there is a lot of attrition over time, and we do not see much overall progress keeping THP's around:

	http://sr71.net/~dave/ibm/038_System_Anonymous_Pages.png

(That's Karl Rister's graph, thanks Karl!)

However, I realized that we do not currently have a nice way to find out where individual THP's might be on the system. We have an overall count, but no way of telling which processes or VMAs they might be in.

I started to implement this in the /proc/$pid/smaps code, but quickly realized that the lib/pagewalk.c code unconditionally splits THPs up. This set reworks that code a bit and, in the end, gives you a per-map count of the numbers of huge pages. It also makes it possible for page walks to _not_ split THPs.

Quoted 5 lines · Dave Hansen

I'm working on some more reports that transparent huge pages and KSM do not play nicely together. Basically, whenever THP's are present along with KSM, there is a lot of attrition over time, and we do not see much overall progress keeping THP's around:

Quoted 3 lines

	http://sr71.net/~dave/ibm/038_System_Anonymous_Pages.png

(That's Karl Rister's graph, thanks Karl!)

Well if the pages_sharing/pages_shared count goes up, this is a feature not a bug.... You need to print that too in the chart to show this is not ok

KSM will slowdown performance also during copy-on-writes when pages_sharing goes up, not only because of creating non-linearity inside 2m chunks (which makes mandatory to use ptes and not hugepmd, it's not an inefficiency of some sort that can be optimized away unfortunately). We sure could change KSM to merge 2M pages instead of 4k pages, but then the memory-density would decrease of several order of magnitudes making the KSM scan almost useless (ok, with guest heavily using THP that may change, but all pagecache is still 4k... so for now it'd be next to useless).

I would prefer to close the issues that you just previously reported, sometime with mmap_sem and issues like that, before adding more features though but I don't want to defer things either so it's up to you.

Quoted 6 lines · Andrea Arcangeli

Quoted 5 lines · Dave Hansen

I'm working on some more reports that transparent huge pages and KSM do not play nicely together. Basically, whenever THP's are present along with KSM, there is a lot of attrition over time, and we do not see much overall progress keeping THP's around:

Quoted 7 lines

Quoted 3 lines

	http://sr71.net/~dave/ibm/038_System_Anonymous_Pages.png

(That's Karl Rister's graph, thanks Karl!)

Well if the pages_sharing/pages_shared count goes up, this is a feature not a bug.... You need to print that too in the chart to show this is not ok

Here are the KSM sharing bits for the same run:

	http://sr71.net/~dave/ibm/009_KSM_Pages.png

It bounces around a little bit on the ends, but it's fairly static during the test, even when there's a good downward slope on the THP's.

Hot of the presses, Karl also managed to do a run last night with the khugepaged scanning rates turned all the way up:

	http://sr71.net/~dave/ibm/038_System_Anonymous_Pages-scan-always.png

The THP's there are a lot more stable. I'd read that as saying that the scanning probably just isn't keeping up with whatever is breaking the pages up.

I'm happy to hold on to them for another release. I'm actually going to go look at the freezes I saw now that I have these out in the wild.

I'll probably stick them in a git tree and keep them up to date.

Are there any other THP issues you're chasing at the moment?

So now the speedup
from hugepages needs to also offset the cost of the more frequent split/collapse events that didn't happen before.

So I guess considering the time is of the order of 2/3 hours and there are "only" 88G of memory, speeding up khugepaged is going to be beneficial considering how big boost hugepages gives to the guest with NPT/EPT and even bigger boost for regular shadow paging, but it also depends on guest. In short khugepaged by default is tuned in a way that can't run in the way of the CPU.

Quoted 3 lines · Andrea Arcangeli

So now the speedup
from hugepages needs to also offset the cost of the more frequent split/collapse events that didn't happen before.

My concern here is the downward slope. I interpret that as saying that we'll eventually have _zero_ THPs. Plus, the benefits are decreasing constantly, even though the scanning overhead is fixed (or increasing even).

I guess we could also try and figure out whether the khugepaged CPU overhead really comes from the scanning or the collapsing operations themselves. Should be as easy as some oprofiling.

If it really is the scanning, I bet we could be a lot more efficient with khugepaged as well. In the case of KVM guests, we're going to have awfully fixed virtual addresses and processes where collapsing can take place.

It might make sense to just have split_huge_page() stick the vaddr and the mm in a queue. khugepaged could scan those addresses first instead of just going after the system as a whole.

For cases where the page got split, but wasn't modified, should we have a non-copying, non-allocating fastpath to re-merge it?

Quoted 3 lines

I guess we could also try and figure out whether the khugepaged CPU overhead really comes from the scanning or the collapsing operations themselves. Should be as easy as some oprofiling.

Actually I already know, the scanning is super fast. So it's no real big deal to increase the scanning. It's big deal only if there are plenty more of collapse/split. Compared to the KSM scan, the khugepaged scan costs nothing.

Just FYI, I did some profiling on a workload that constantly split and joined pages. Very little of the overhead was in the scanning itself, so I think you're dead-on here.

I'm working on some more reports that transparent huge pages and KSM do not play nicely together. Basically, whenever THP's are present along with KSM, there is a lot of attrition over time, and we do not see much overall progress keeping THP's around:

	http://sr71.net/~dave/ibm/038_System_Anonymous_Pages.png

(That's Karl Rister's graph, thanks Karl!)

However, I realized that we do not currently have a nice way to find out where individual THP's might be on the system. We have an overall count, but no way of telling which processes or VMAs they might be in.

I started to implement this in the /proc/$pid/smaps code, but quickly realized that the lib/pagewalk.c code unconditionally splits THPs up. This set reworks that code a bit and, in the end, gives you a per-map count of the numbers of huge pages. It also makes it possible for page walks to _not_ split THPs.