Skip to content

Commit 258f669

Browse files
tehcastertorvalds
authored andcommitted
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 [[email protected]: use seq_file infrastructure] Link: http://lkml.kernel.org/r/[email protected] Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Vlastimil Babka <[email protected]> Reported-by: Daniel Colascione <[email protected]> Reviewed-by: Alexey Dobriyan <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent f154795 commit 258f669

File tree

2 files changed

+96
-60
lines changed

2 files changed

+96
-60
lines changed

fs/proc/internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,6 @@ struct proc_maps_private {
285285
struct inode *inode;
286286
struct task_struct *task;
287287
struct mm_struct *mm;
288-
struct mem_size_stats *rollup;
289288
#ifdef CONFIG_MMU
290289
struct vm_area_struct *tail_vma;
291290
#endif

fs/proc/task_mmu.c

Lines changed: 96 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ static int proc_map_release(struct inode *inode, struct file *file)
247247
if (priv->mm)
248248
mmdrop(priv->mm);
249249

250-
kfree(priv->rollup);
251250
return seq_release_private(inode, file);
252251
}
253252

@@ -404,7 +403,6 @@ const struct file_operations proc_pid_maps_operations = {
404403

405404
#ifdef CONFIG_PROC_PAGE_MONITOR
406405
struct mem_size_stats {
407-
bool first;
408406
unsigned long resident;
409407
unsigned long shared_clean;
410408
unsigned long shared_dirty;
@@ -418,7 +416,6 @@ struct mem_size_stats {
418416
unsigned long swap;
419417
unsigned long shared_hugetlb;
420418
unsigned long private_hugetlb;
421-
unsigned long first_vma_start;
422419
u64 pss;
423420
u64 pss_locked;
424421
u64 swap_pss;
@@ -775,57 +772,75 @@ static void __show_smap(struct seq_file *m, const struct mem_size_stats *mss)
775772

776773
static int show_smap(struct seq_file *m, void *v)
777774
{
778-
struct proc_maps_private *priv = m->private;
779775
struct vm_area_struct *vma = v;
780-
struct mem_size_stats mss_stack;
781-
struct mem_size_stats *mss;
776+
struct mem_size_stats mss;
777+
778+
memset(&mss, 0, sizeof(mss));
779+
780+
smap_gather_stats(vma, &mss);
781+
782+
show_map_vma(m, vma);
783+
784+
SEQ_PUT_DEC("Size: ", vma->vm_end - vma->vm_start);
785+
SEQ_PUT_DEC(" kB\nKernelPageSize: ", vma_kernel_pagesize(vma));
786+
SEQ_PUT_DEC(" kB\nMMUPageSize: ", vma_mmu_pagesize(vma));
787+
seq_puts(m, " kB\n");
788+
789+
__show_smap(m, &mss);
790+
791+
if (arch_pkeys_enabled())
792+
seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
793+
show_smap_vma_flags(m, vma);
794+
795+
m_cache_vma(m, vma);
796+
797+
return 0;
798+
}
799+
800+
static int show_smaps_rollup(struct seq_file *m, void *v)
801+
{
802+
struct proc_maps_private *priv = m->private;
803+
struct mem_size_stats mss;
804+
struct mm_struct *mm;
805+
struct vm_area_struct *vma;
806+
unsigned long last_vma_end = 0;
782807
int ret = 0;
783-
bool rollup_mode;
784-
bool last_vma;
785-
786-
if (priv->rollup) {
787-
rollup_mode = true;
788-
mss = priv->rollup;
789-
if (mss->first) {
790-
mss->first_vma_start = vma->vm_start;
791-
mss->first = false;
792-
}
793-
last_vma = !m_next_vma(priv, vma);
794-
} else {
795-
rollup_mode = false;
796-
memset(&mss_stack, 0, sizeof(mss_stack));
797-
mss = &mss_stack;
798-
}
799808

800-
smap_gather_stats(vma, mss);
809+
priv->task = get_proc_task(priv->inode);
810+
if (!priv->task)
811+
return -ESRCH;
801812

802-
if (!rollup_mode) {
803-
show_map_vma(m, vma);
804-
} else if (last_vma) {
805-
show_vma_header_prefix(
806-
m, mss->first_vma_start, vma->vm_end, 0, 0, 0, 0);
807-
seq_pad(m, ' ');
808-
seq_puts(m, "[rollup]\n");
809-
} else {
810-
ret = SEQ_SKIP;
813+
mm = priv->mm;
814+
if (!mm || !mmget_not_zero(mm)) {
815+
ret = -ESRCH;
816+
goto out_put_task;
811817
}
812818

813-
if (!rollup_mode) {
814-
SEQ_PUT_DEC("Size: ", vma->vm_end - vma->vm_start);
815-
SEQ_PUT_DEC(" kB\nKernelPageSize: ", vma_kernel_pagesize(vma));
816-
SEQ_PUT_DEC(" kB\nMMUPageSize: ", vma_mmu_pagesize(vma));
817-
seq_puts(m, " kB\n");
818-
}
819+
memset(&mss, 0, sizeof(mss));
819820

820-
if (!rollup_mode || last_vma)
821-
__show_smap(m, mss);
821+
down_read(&mm->mmap_sem);
822+
hold_task_mempolicy(priv);
822823

823-
if (!rollup_mode) {
824-
if (arch_pkeys_enabled())
825-
seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma));
826-
show_smap_vma_flags(m, vma);
824+
for (vma = priv->mm->mmap; vma; vma = vma->vm_next) {
825+
smap_gather_stats(vma, &mss);
826+
last_vma_end = vma->vm_end;
827827
}
828-
m_cache_vma(m, vma);
828+
829+
show_vma_header_prefix(m, priv->mm->mmap->vm_start,
830+
last_vma_end, 0, 0, 0, 0);
831+
seq_pad(m, ' ');
832+
seq_puts(m, "[rollup]\n");
833+
834+
__show_smap(m, &mss);
835+
836+
release_task_mempolicy(priv);
837+
up_read(&mm->mmap_sem);
838+
mmput(mm);
839+
840+
out_put_task:
841+
put_task_struct(priv->task);
842+
priv->task = NULL;
843+
829844
return ret;
830845
}
831846
#undef SEQ_PUT_DEC
@@ -842,23 +857,45 @@ static int pid_smaps_open(struct inode *inode, struct file *file)
842857
return do_maps_open(inode, file, &proc_pid_smaps_op);
843858
}
844859

845-
static int pid_smaps_rollup_open(struct inode *inode, struct file *file)
860+
static int smaps_rollup_open(struct inode *inode, struct file *file)
846861
{
847-
struct seq_file *seq;
862+
int ret;
848863
struct proc_maps_private *priv;
849-
int ret = do_maps_open(inode, file, &proc_pid_smaps_op);
850-
851-
if (ret < 0)
852-
return ret;
853-
seq = file->private_data;
854-
priv = seq->private;
855-
priv->rollup = kzalloc(sizeof(*priv->rollup), GFP_KERNEL);
856-
if (!priv->rollup) {
857-
proc_map_release(inode, file);
864+
865+
priv = kzalloc(sizeof(*priv), GFP_KERNEL_ACCOUNT);
866+
if (!priv)
858867
return -ENOMEM;
868+
869+
ret = single_open(file, show_smaps_rollup, priv);
870+
if (ret)
871+
goto out_free;
872+
873+
priv->inode = inode;
874+
priv->mm = proc_mem_open(inode, PTRACE_MODE_READ);
875+
if (IS_ERR(priv->mm)) {
876+
ret = PTR_ERR(priv->mm);
877+
878+
single_release(inode, file);
879+
goto out_free;
859880
}
860-
priv->rollup->first = true;
881+
861882
return 0;
883+
884+
out_free:
885+
kfree(priv);
886+
return ret;
887+
}
888+
889+
static int smaps_rollup_release(struct inode *inode, struct file *file)
890+
{
891+
struct seq_file *seq = file->private_data;
892+
struct proc_maps_private *priv = seq->private;
893+
894+
if (priv->mm)
895+
mmdrop(priv->mm);
896+
897+
kfree(priv);
898+
return single_release(inode, file);
862899
}
863900

864901
const struct file_operations proc_pid_smaps_operations = {
@@ -869,10 +906,10 @@ const struct file_operations proc_pid_smaps_operations = {
869906
};
870907

871908
const struct file_operations proc_pid_smaps_rollup_operations = {
872-
.open = pid_smaps_rollup_open,
909+
.open = smaps_rollup_open,
873910
.read = seq_read,
874911
.llseek = seq_lseek,
875-
.release = proc_map_release,
912+
.release = smaps_rollup_release,
876913
};
877914

878915
enum clear_refs_types {

0 commit comments

Comments
 (0)