On Tue, Jan 29, 2019 at 04:52:21PM +0100, Vlastimil Babka wrote: > On 1/29/19 1:15 AM, Andrew Morton wrote: > > On Sun, 20 Jan 2019 17:10:49 -0800 Sandeep Patil <sspatil@xxxxxxxxxxx> wrote: > > > >> The 'pss_locked' field of smaps_rollup was being calculated incorrectly > >> as it accumulated the current pss everytime a locked VMA was found. > >> > >> Fix that by making sure we record the current pss value before each VMA > >> is walked. So, we can only add the delta if the VMA was found to be > >> VM_LOCKED. > >> > >> ... > >> > >> --- a/fs/proc/task_mmu.c > >> +++ b/fs/proc/task_mmu.c > >> @@ -709,6 +709,7 @@ static void smap_gather_stats(struct vm_area_struct *vma, > >> #endif > >> .mm = vma->vm_mm, > >> }; > >> + unsigned long pss; > >> > >> smaps_walk.private = mss; > >> > >> @@ -737,11 +738,12 @@ static void smap_gather_stats(struct vm_area_struct *vma, > >> } > >> } > >> #endif > >> - > >> + /* record current pss so we can calculate the delta after page walk */ > >> + pss = mss->pss; > >> /* mmap_sem is held in m_start */ > >> walk_page_vma(vma, &smaps_walk); > >> if (vma->vm_flags & VM_LOCKED) > >> - mss->pss_locked += mss->pss; > >> + mss->pss_locked += mss->pss - pss; > >> } > > > > This seems to be a rather obscure way of accumulating > > mem_size_stats.pss_locked. Wouldn't it make more sense to do this in > > smaps_account(), wherever we increment mem_size_stats.pss? > > > > It would be a tiny bit less efficient but I think that the code cleanup > > justifies such a cost? > > Yeah, Sandeep could you add 'bool locked' param to smaps_account() and check it > there? We probably don't need the whole vma param yet. Agree, I will send -v2 shortly. - ssp
