Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mm: /proc/pid/smaps_rollup: convert to single value seq_file

The /proc/pid/smaps_rollup file is currently implemented via the
m_start/m_next/m_stop seq_file iterators shared with the other maps files,
that iterate over vma's. However, the rollup file doesn't print anything
for each vma, only accumulate the stats.

There are some issues with the current code as reported in [1] - the
accumulated stats can get skewed if seq_file start()/stop() op is called
multiple times, if show() is called multiple times, and after seeks to
non-zero position.

Patch [1] fixed those within existing design, but I believe it is
fundamentally wrong to expose the vma iterators to the seq_file mechanism
when smaps_rollup shows logically a single set of values for the whole
address space.

This patch thus refactors the code to provide a single "value" at offset
0, with vma iteration to gather the stats done internally. This fixes the
situations where results are skewed, and simplifies the code, especially
in show_smap(), at the expense of somewhat less code reuse.

[1] https://marc.info/?l=linux-mm&m=151927723128134&w=2

[vbabka@suse.c: use seq_file infrastructure]
Link: http://lkml.kernel.org/r/bf4525b0-fd5b-4c4c-2cb3-adee3dd95a48@suse.cz
Link: http://lkml.kernel.org/r/20180723111933.15443-5-vbabka@suse.cz
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Reported-by: Daniel Colascione <dancol@google.com>
Reviewed-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Vlastimil Babka and committed by
Linus Torvalds
258f669e f1547959

+96 -60
-1
fs/proc/internal.h
··· 285 285 struct inode *inode; 286 286 struct task_struct *task; 287 287 struct mm_struct *mm; 288 - struct mem_size_stats *rollup; 289 288 #ifdef CONFIG_MMU 290 289 struct vm_area_struct *tail_vma; 291 290 #endif
+96 -59
fs/proc/task_mmu.c
··· 247 247 if (priv->mm) 248 248 mmdrop(priv->mm); 249 249 250 - kfree(priv->rollup); 251 250 return seq_release_private(inode, file); 252 251 } 253 252 ··· 403 404 404 405 #ifdef CONFIG_PROC_PAGE_MONITOR 405 406 struct mem_size_stats { 406 - bool first; 407 407 unsigned long resident; 408 408 unsigned long shared_clean; 409 409 unsigned long shared_dirty; ··· 416 418 unsigned long swap; 417 419 unsigned long shared_hugetlb; 418 420 unsigned long private_hugetlb; 419 - unsigned long first_vma_start; 420 421 u64 pss; 421 422 u64 pss_locked; 422 423 u64 swap_pss; ··· 772 775 773 776 static int show_smap(struct seq_file *m, void *v) 774 777 { 775 - struct proc_maps_private *priv = m->private; 776 778 struct vm_area_struct *vma = v; 777 - struct mem_size_stats mss_stack; 778 - struct mem_size_stats *mss; 779 - int ret = 0; 780 - bool rollup_mode; 781 - bool last_vma; 779 + struct mem_size_stats mss; 782 780 783 - if (priv->rollup) { 784 - rollup_mode = true; 785 - mss = priv->rollup; 786 - if (mss->first) { 787 - mss->first_vma_start = vma->vm_start; 788 - mss->first = false; 789 - } 790 - last_vma = !m_next_vma(priv, vma); 791 - } else { 792 - rollup_mode = false; 793 - memset(&mss_stack, 0, sizeof(mss_stack)); 794 - mss = &mss_stack; 795 - } 781 + memset(&mss, 0, sizeof(mss)); 796 782 797 - smap_gather_stats(vma, mss); 783 + smap_gather_stats(vma, &mss); 798 784 799 - if (!rollup_mode) { 800 - show_map_vma(m, vma); 801 - } else if (last_vma) { 802 - show_vma_header_prefix( 803 - m, mss->first_vma_start, vma->vm_end, 0, 0, 0, 0); 804 - seq_pad(m, ' '); 805 - seq_puts(m, "[rollup]\n"); 806 - } else { 807 - ret = SEQ_SKIP; 808 - } 785 + show_map_vma(m, vma); 809 786 810 - if (!rollup_mode) { 811 - SEQ_PUT_DEC("Size: ", vma->vm_end - vma->vm_start); 812 - SEQ_PUT_DEC(" kB\nKernelPageSize: ", vma_kernel_pagesize(vma)); 813 - SEQ_PUT_DEC(" kB\nMMUPageSize: ", vma_mmu_pagesize(vma)); 814 - seq_puts(m, " kB\n"); 815 - } 787 + SEQ_PUT_DEC("Size: ", vma->vm_end - vma->vm_start); 788 + SEQ_PUT_DEC(" kB\nKernelPageSize: ", vma_kernel_pagesize(vma)); 789 + SEQ_PUT_DEC(" kB\nMMUPageSize: ", vma_mmu_pagesize(vma)); 790 + seq_puts(m, " kB\n"); 816 791 817 - if (!rollup_mode || last_vma) 818 - __show_smap(m, mss); 792 + __show_smap(m, &mss); 819 793 820 - if (!rollup_mode) { 821 - if (arch_pkeys_enabled()) 822 - seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); 823 - show_smap_vma_flags(m, vma); 824 - } 794 + if (arch_pkeys_enabled()) 795 + seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); 796 + show_smap_vma_flags(m, vma); 797 + 825 798 m_cache_vma(m, vma); 799 + 800 + return 0; 801 + } 802 + 803 + static int show_smaps_rollup(struct seq_file *m, void *v) 804 + { 805 + struct proc_maps_private *priv = m->private; 806 + struct mem_size_stats mss; 807 + struct mm_struct *mm; 808 + struct vm_area_struct *vma; 809 + unsigned long last_vma_end = 0; 810 + int ret = 0; 811 + 812 + priv->task = get_proc_task(priv->inode); 813 + if (!priv->task) 814 + return -ESRCH; 815 + 816 + mm = priv->mm; 817 + if (!mm || !mmget_not_zero(mm)) { 818 + ret = -ESRCH; 819 + goto out_put_task; 820 + } 821 + 822 + memset(&mss, 0, sizeof(mss)); 823 + 824 + down_read(&mm->mmap_sem); 825 + hold_task_mempolicy(priv); 826 + 827 + for (vma = priv->mm->mmap; vma; vma = vma->vm_next) { 828 + smap_gather_stats(vma, &mss); 829 + last_vma_end = vma->vm_end; 830 + } 831 + 832 + show_vma_header_prefix(m, priv->mm->mmap->vm_start, 833 + last_vma_end, 0, 0, 0, 0); 834 + seq_pad(m, ' '); 835 + seq_puts(m, "[rollup]\n"); 836 + 837 + __show_smap(m, &mss); 838 + 839 + release_task_mempolicy(priv); 840 + up_read(&mm->mmap_sem); 841 + mmput(mm); 842 + 843 + out_put_task: 844 + put_task_struct(priv->task); 845 + priv->task = NULL; 846 + 826 847 return ret; 827 848 } 828 849 #undef SEQ_PUT_DEC ··· 857 842 return do_maps_open(inode, file, &proc_pid_smaps_op); 858 843 } 859 844 860 - static int pid_smaps_rollup_open(struct inode *inode, struct file *file) 845 + static int smaps_rollup_open(struct inode *inode, struct file *file) 861 846 { 862 - struct seq_file *seq; 847 + int ret; 863 848 struct proc_maps_private *priv; 864 - int ret = do_maps_open(inode, file, &proc_pid_smaps_op); 865 849 866 - if (ret < 0) 867 - return ret; 868 - seq = file->private_data; 869 - priv = seq->private; 870 - priv->rollup = kzalloc(sizeof(*priv->rollup), GFP_KERNEL); 871 - if (!priv->rollup) { 872 - proc_map_release(inode, file); 850 + priv = kzalloc(sizeof(*priv), GFP_KERNEL_ACCOUNT); 851 + if (!priv) 873 852 return -ENOMEM; 853 + 854 + ret = single_open(file, show_smaps_rollup, priv); 855 + if (ret) 856 + goto out_free; 857 + 858 + priv->inode = inode; 859 + priv->mm = proc_mem_open(inode, PTRACE_MODE_READ); 860 + if (IS_ERR(priv->mm)) { 861 + ret = PTR_ERR(priv->mm); 862 + 863 + single_release(inode, file); 864 + goto out_free; 874 865 } 875 - priv->rollup->first = true; 866 + 876 867 return 0; 868 + 869 + out_free: 870 + kfree(priv); 871 + return ret; 872 + } 873 + 874 + static int smaps_rollup_release(struct inode *inode, struct file *file) 875 + { 876 + struct seq_file *seq = file->private_data; 877 + struct proc_maps_private *priv = seq->private; 878 + 879 + if (priv->mm) 880 + mmdrop(priv->mm); 881 + 882 + kfree(priv); 883 + return single_release(inode, file); 877 884 } 878 885 879 886 const struct file_operations proc_pid_smaps_operations = { ··· 906 869 }; 907 870 908 871 const struct file_operations proc_pid_smaps_rollup_operations = { 909 - .open = pid_smaps_rollup_open, 872 + .open = smaps_rollup_open, 910 873 .read = seq_read, 911 874 .llseek = seq_lseek, 912 - .release = proc_map_release, 875 + .release = smaps_rollup_release, 913 876 }; 914 877 915 878 enum clear_refs_types {