diff mbox series

[v2] mm: proc: smaps_rollup: Fix pss_locked calculation

Message ID 20190203065425.14650-1-sspatil@android.com (mailing list archive)
State New, archived
Headers show
Series [v2] mm: proc: smaps_rollup: Fix pss_locked calculation | expand

Commit Message

Sandeep Patil Feb. 3, 2019, 6:54 a.m. UTC 
The 'pss_locked' field of smaps_rollup was being calculated incorrectly.
It accumulated the current pss everytime a locked VMA was found.  Fix
that by adding to 'pss_locked' the same time as that of 'pss' if the vma
being walked is locked.

Fixes: 493b0e9d945f ("mm: add /proc/pid/smaps_rollup")
Cc: stable@vger.kernel.org # 4.14.y 4.19.y
Signed-off-by: Sandeep Patil <sspatil@android.com>
---

v1->v2
------
- Move pss_locked accounting into smaps_account() inline with pss

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

Comments

Vlastimil Babka Feb. 8, 2019, 5:04 p.m. UTC | #1 
On 2/3/19 7:54 AM, Sandeep Patil wrote:
> The 'pss_locked' field of smaps_rollup was being calculated incorrectly.
> It accumulated the current pss everytime a locked VMA was found.  Fix
> that by adding to 'pss_locked' the same time as that of 'pss' if the vma
> being walked is locked.
> 
> Fixes: 493b0e9d945f ("mm: add /proc/pid/smaps_rollup")
> Cc: stable@vger.kernel.org # 4.14.y 4.19.y
> Signed-off-by: Sandeep Patil <sspatil@android.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks.

> ---
> 
> v1->v2
> ------
> - Move pss_locked accounting into smaps_account() inline with pss
Joel Fernandes Feb. 11, 2019, 5:41 p.m. UTC | #2 
On Sat, Feb 02, 2019 at 10:54:25PM -0800, Sandeep Patil wrote:
> The 'pss_locked' field of smaps_rollup was being calculated incorrectly.
> It accumulated the current pss everytime a locked VMA was found.  Fix
> that by adding to 'pss_locked' the same time as that of 'pss' if the vma
> being walked is locked.
> 
> Fixes: 493b0e9d945f ("mm: add /proc/pid/smaps_rollup")
> Cc: stable@vger.kernel.org # 4.14.y 4.19.y
> Signed-off-by: Sandeep Patil <sspatil@android.com>
> ---

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

- Joel


> 
> v1->v2
> ------
> - Move pss_locked accounting into smaps_account() inline with pss
> 
>  fs/proc/task_mmu.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index f0ec9edab2f3..85b0ef890b28 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -423,7 +423,7 @@ struct mem_size_stats {
>  };
>  
>  static void smaps_account(struct mem_size_stats *mss, struct page *page,
> -		bool compound, bool young, bool dirty)
> +		bool compound, bool young, bool dirty, bool locked)
>  {
>  	int i, nr = compound ? 1 << compound_order(page) : 1;
>  	unsigned long size = nr * PAGE_SIZE;
> @@ -450,24 +450,31 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
>  		else
>  			mss->private_clean += size;
>  		mss->pss += (u64)size << PSS_SHIFT;
> +		if (locked)
> +			mss->pss_locked += (u64)size << PSS_SHIFT;
>  		return;
>  	}
>  
>  	for (i = 0; i < nr; i++, page++) {
>  		int mapcount = page_mapcount(page);
> +		unsigned long pss = (PAGE_SIZE << PSS_SHIFT);
>  
>  		if (mapcount >= 2) {
>  			if (dirty || PageDirty(page))
>  				mss->shared_dirty += PAGE_SIZE;
>  			else
>  				mss->shared_clean += PAGE_SIZE;
> -			mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount;
> +			mss->pss += pss / mapcount;
> +			if (locked)
> +				mss->pss_locked += pss / mapcount;
>  		} else {
>  			if (dirty || PageDirty(page))
>  				mss->private_dirty += PAGE_SIZE;
>  			else
>  				mss->private_clean += PAGE_SIZE;
> -			mss->pss += PAGE_SIZE << PSS_SHIFT;
> +			mss->pss += pss;
> +			if (locked)
> +				mss->pss_locked += pss;
>  		}
>  	}
>  }
> @@ -490,6 +497,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
>  {
>  	struct mem_size_stats *mss = walk->private;
>  	struct vm_area_struct *vma = walk->vma;
> +	bool locked = !!(vma->vm_flags & VM_LOCKED);
>  	struct page *page = NULL;
>  
>  	if (pte_present(*pte)) {
> @@ -532,7 +540,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
>  	if (!page)
>  		return;
>  
> -	smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte));
> +	smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte), locked);
>  }
>  
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> @@ -541,6 +549,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>  {
>  	struct mem_size_stats *mss = walk->private;
>  	struct vm_area_struct *vma = walk->vma;
> +	bool locked = !!(vma->vm_flags & VM_LOCKED);
>  	struct page *page;
>  
>  	/* FOLL_DUMP will return -EFAULT on huge zero page */
> @@ -555,7 +564,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>  		/* pass */;
>  	else
>  		VM_BUG_ON_PAGE(1, page);
> -	smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd));
> +	smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd), locked);
>  }
>  #else
>  static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
> @@ -737,11 +746,8 @@ static void smap_gather_stats(struct vm_area_struct *vma,
>  		}
>  	}
>  #endif
> -
>  	/* mmap_sem is held in m_start */
>  	walk_page_vma(vma, &smaps_walk);
> -	if (vma->vm_flags & VM_LOCKED)
> -		mss->pss_locked += mss->pss;
>  }
>  
>  #define SEQ_PUT_DEC(str, val) \
> -- 
> 2.20.1.611.gfbb209baf1-goog
>
diff mbox series

Patch

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index f0ec9edab2f3..85b0ef890b28 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -423,7 +423,7 @@  struct mem_size_stats {
 };
 
 static void smaps_account(struct mem_size_stats *mss, struct page *page,
-		bool compound, bool young, bool dirty)
+		bool compound, bool young, bool dirty, bool locked)
 {
 	int i, nr = compound ? 1 << compound_order(page) : 1;
 	unsigned long size = nr * PAGE_SIZE;
@@ -450,24 +450,31 @@  static void smaps_account(struct mem_size_stats *mss, struct page *page,
 		else
 			mss->private_clean += size;
 		mss->pss += (u64)size << PSS_SHIFT;
+		if (locked)
+			mss->pss_locked += (u64)size << PSS_SHIFT;
 		return;
 	}
 
 	for (i = 0; i < nr; i++, page++) {
 		int mapcount = page_mapcount(page);
+		unsigned long pss = (PAGE_SIZE << PSS_SHIFT);
 
 		if (mapcount >= 2) {
 			if (dirty || PageDirty(page))
 				mss->shared_dirty += PAGE_SIZE;
 			else
 				mss->shared_clean += PAGE_SIZE;
-			mss->pss += (PAGE_SIZE << PSS_SHIFT) / mapcount;
+			mss->pss += pss / mapcount;
+			if (locked)
+				mss->pss_locked += pss / mapcount;
 		} else {
 			if (dirty || PageDirty(page))
 				mss->private_dirty += PAGE_SIZE;
 			else
 				mss->private_clean += PAGE_SIZE;
-			mss->pss += PAGE_SIZE << PSS_SHIFT;
+			mss->pss += pss;
+			if (locked)
+				mss->pss_locked += pss;
 		}
 	}
 }
@@ -490,6 +497,7 @@  static void smaps_pte_entry(pte_t *pte, unsigned long addr,
 {
 	struct mem_size_stats *mss = walk->private;
 	struct vm_area_struct *vma = walk->vma;
+	bool locked = !!(vma->vm_flags & VM_LOCKED);
 	struct page *page = NULL;
 
 	if (pte_present(*pte)) {
@@ -532,7 +540,7 @@  static void smaps_pte_entry(pte_t *pte, unsigned long addr,
 	if (!page)
 		return;
 
-	smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte));
+	smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte), locked);
 }
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
@@ -541,6 +549,7 @@  static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
 {
 	struct mem_size_stats *mss = walk->private;
 	struct vm_area_struct *vma = walk->vma;
+	bool locked = !!(vma->vm_flags & VM_LOCKED);
 	struct page *page;
 
 	/* FOLL_DUMP will return -EFAULT on huge zero page */
@@ -555,7 +564,7 @@  static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
 		/* pass */;
 	else
 		VM_BUG_ON_PAGE(1, page);
-	smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd));
+	smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd), locked);
 }
 #else
 static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
@@ -737,11 +746,8 @@  static void smap_gather_stats(struct vm_area_struct *vma,
 		}
 	}
 #endif
-
 	/* mmap_sem is held in m_start */
 	walk_page_vma(vma, &smaps_walk);
-	if (vma->vm_flags & VM_LOCKED)
-		mss->pss_locked += mss->pss;
 }
 
 #define SEQ_PUT_DEC(str, val) \