Re: [PATCHv5 01/28] mm, proc: adjust PSS calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/23/2015 11:03 PM, Kirill A. Shutemov wrote:
With new refcounting all subpages of the compound page are not nessessary
have the same mapcount. We need to take into account mapcount of every
sub-page.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Tested-by: Sasha Levin <sasha.levin@xxxxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

(some nitpicks below)

---
  fs/proc/task_mmu.c | 43 ++++++++++++++++++++++---------------------
  1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 956b75d61809..95bc384ee3f7 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -449,9 +449,10 @@ struct mem_size_stats {
  };

  static void smaps_account(struct mem_size_stats *mss, struct page *page,
-		unsigned long size, bool young, bool dirty)
+		bool compound, bool young, bool dirty)
  {
-	int mapcount;
+	int i, nr = compound ? hpage_nr_pages(page) : 1;

Why not just HPAGE_PMD_NR instead of hpage_nr_pages(page)? We already came here through a pmd mapping. Even if the page stopped being a hugepage meanwhile (I'm not sure if any locking prevents that or not?), it would be more accurate to continue assuming it's a hugepage, otherwise we account only the base page (formerly head) and skip the 511 formerly tail pages?

Also, is there some shortcut way to tell us that we are the only one mapping the whole compound page, and nobody has any base pages, so we don't need to loop on each tail page? I guess not under the new design, right...

+	unsigned long size = nr * PAGE_SIZE;

  	if (PageAnon(page))
  		mss->anonymous += size;
@@ -460,23 +461,23 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
  	/* Accumulate the size in pages that have been accessed. */
  	if (young || PageReferenced(page))
  		mss->referenced += size;
-	mapcount = page_mapcount(page);
-	if (mapcount >= 2) {
-		u64 pss_delta;

-		if (dirty || PageDirty(page))
-			mss->shared_dirty += size;
-		else
-			mss->shared_clean += size;
-		pss_delta = (u64)size << PSS_SHIFT;
-		do_div(pss_delta, mapcount);
-		mss->pss += pss_delta;
-	} else {
-		if (dirty || PageDirty(page))
-			mss->private_dirty += size;
-		else
-			mss->private_clean += size;
-		mss->pss += (u64)size << PSS_SHIFT;
+	for (i = 0; i < nr; i++) {
+		int mapcount = page_mapcount(page + i);
+
+		if (mapcount >= 2) {
+			if (dirty || PageDirty(page + i))
+				mss->shared_dirty += PAGE_SIZE;
+			else
+				mss->shared_clean += PAGE_SIZE;
+			mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount;
+		} else {
+			if (dirty || PageDirty(page + i))
+				mss->private_dirty += PAGE_SIZE;
+			else
+				mss->private_clean += PAGE_SIZE;
+			mss->pss += PAGE_SIZE << PSS_SHIFT;
+		}

That's 3 instances of "page + i", why not just use page and do a page++ in the for loop?

  	}
  }

@@ -500,7 +501,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,

  	if (!page)
  		return;
-	smaps_account(mss, page, PAGE_SIZE, pte_young(*pte), pte_dirty(*pte));
+
+	smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte));
  }

  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
@@ -516,8 +518,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
  	if (IS_ERR_OR_NULL(page))
  		return;
  	mss->anonymous_thp += HPAGE_PMD_SIZE;
-	smaps_account(mss, page, HPAGE_PMD_SIZE,
-			pmd_young(*pmd), pmd_dirty(*pmd));
+	smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd));
  }
  #else
  static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]