| Message ID | 20210917164756.8586-2-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/smaps: Fixes and optimizations on shmem swap handling | expand |
On 9/17/21 18:47, Peter Xu wrote: > The shmem swap calculation on the privately writable mappings are using wrong > parameters as spotted by Vlastimil. Fix them. That's introduced in commit > 48131e03ca4e, when rework shmem_swap_usage to shmem_partial_swap_usage. > > Test program: > > ================== > > void main(void) > { > char *buffer, *p; > int i, fd; > > fd = memfd_create("test", 0); > assert(fd > 0); > > /* isize==2M*3, fill in pages, swap them out */ > ftruncate(fd, SIZE_2M * 3); > buffer = mmap(NULL, SIZE_2M * 3, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > assert(buffer); > for (i = 0, p = buffer; i < SIZE_2M * 3 / 4096; i++) { > *p = 1; > p += 4096; > } > madvise(buffer, SIZE_2M * 3, MADV_PAGEOUT); > munmap(buffer, SIZE_2M * 3); > > /* > * Remap with private+writtable mappings on partial of the inode (<= 2M*3), > * while the size must also be >= 2M*2 to make sure there's a none pmd so > * smaps_pte_hole will be triggered. > */ > buffer = mmap(NULL, SIZE_2M * 2, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); > printf("pid=%d, buffer=%p\n", getpid(), buffer); > > /* Check /proc/$PID/smap_rollup, should see 4MB swap */ > sleep(1000000); > } > ================== > > Before the patch, smaps_rollup shows <4MB swap and the number will be random > depending on the alignment of the buffer of mmap() allocated. After this > patch, it'll show 4MB. > > Fixes: 48131e03ca4e ("mm, proc: reduce cost of /proc/pid/smaps for unpopulated shmem mappings") Thanks, too bad I didn't spot it when sending that patch :) > Reported-by: Vlastimil Babka <vbabka@suse.cz> > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> > --- > fs/proc/task_mmu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index cf25be3e0321..2197f669e17b 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -478,9 +478,11 @@ static int smaps_pte_hole(unsigned long addr, unsigned long end, > __always_unused int depth, struct mm_walk *walk) > { > struct mem_size_stats *mss = walk->private; > + struct vm_area_struct *vma = walk->vma; > > - mss->swap += shmem_partial_swap_usage( > - walk->vma->vm_file->f_mapping, addr, end); > + mss->swap += shmem_partial_swap_usage(walk->vma->vm_file->f_mapping, > + linear_page_index(vma, addr), > + linear_page_index(vma, end)); > > return 0; > } >
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index cf25be3e0321..2197f669e17b 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -478,9 +478,11 @@ static int smaps_pte_hole(unsigned long addr, unsigned long end, __always_unused int depth, struct mm_walk *walk) { struct mem_size_stats *mss = walk->private; + struct vm_area_struct *vma = walk->vma; - mss->swap += shmem_partial_swap_usage( - walk->vma->vm_file->f_mapping, addr, end); + mss->swap += shmem_partial_swap_usage(walk->vma->vm_file->f_mapping, + linear_page_index(vma, addr), + linear_page_index(vma, end)); return 0; }
The shmem swap calculation on the privately writable mappings are using wrong parameters as spotted by Vlastimil. Fix them. That's introduced in commit 48131e03ca4e, when rework shmem_swap_usage to shmem_partial_swap_usage. Test program: ================== void main(void) { char *buffer, *p; int i, fd; fd = memfd_create("test", 0); assert(fd > 0); /* isize==2M*3, fill in pages, swap them out */ ftruncate(fd, SIZE_2M * 3); buffer = mmap(NULL, SIZE_2M * 3, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); assert(buffer); for (i = 0, p = buffer; i < SIZE_2M * 3 / 4096; i++) { *p = 1; p += 4096; } madvise(buffer, SIZE_2M * 3, MADV_PAGEOUT); munmap(buffer, SIZE_2M * 3); /* * Remap with private+writtable mappings on partial of the inode (<= 2M*3), * while the size must also be >= 2M*2 to make sure there's a none pmd so * smaps_pte_hole will be triggered. */ buffer = mmap(NULL, SIZE_2M * 2, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); printf("pid=%d, buffer=%p\n", getpid(), buffer); /* Check /proc/$PID/smap_rollup, should see 4MB swap */ sleep(1000000); } ================== Before the patch, smaps_rollup shows <4MB swap and the number will be random depending on the alignment of the buffer of mmap() allocated. After this patch, it'll show 4MB. Fixes: 48131e03ca4e ("mm, proc: reduce cost of /proc/pid/smaps for unpopulated shmem mappings") Reported-by: Vlastimil Babka <vbabka@suse.cz> Signed-off-by: Peter Xu <peterx@redhat.com> --- fs/proc/task_mmu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)