2009-12-07 10:26:29

by Cong Wang

[permalink] [raw]
Subject: [Patch] proc: fill 'lib' field in /proc/<pid>/statm

Currently, the 'lib' field of /proc/<pid>/statm is
always 0, we should fill it with the right value,
the same with 'VmLib'.

Signed-off-by: WANG Cong <[email protected]>
Cc: Eric W. Biederman <[email protected]>
Cc: Alexey Dobriyan <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Mel Gorman <[email protected]>
Cc: [email protected]

---
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 4badde1..d710293 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -623,7 +623,7 @@ int proc_pid_statm(struct seq_file *m, struct pid_namespace *ns,
struct mm_struct *mm = get_task_mm(task);

if (mm) {
- size = task_statm(mm, &shared, &text, &data, &resident);
+ size = task_statm(mm, &shared, &text, &data, &resident, &lib);
mmput(mm);
}
seq_printf(m, "%d %d %d %d %d %d %d\n",
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 753ca37..5b7ab52 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -98,7 +98,7 @@ extern spinlock_t proc_subdir_lock;
struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *);
int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir);
unsigned long task_vsize(struct mm_struct *);
-int task_statm(struct mm_struct *, int *, int *, int *, int *);
+int task_statm(struct mm_struct *, int *, int *, int *, int *, int *);
void task_mem(struct seq_file *, struct mm_struct *);

struct proc_dir_entry *de_get(struct proc_dir_entry *de);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2a1bef9..ea5bdd9 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -63,13 +63,14 @@ unsigned long task_vsize(struct mm_struct *mm)
}

int task_statm(struct mm_struct *mm, int *shared, int *text,
- int *data, int *resident)
+ int *data, int *resident, int *lib)
{
*shared = get_mm_counter(mm, file_rss);
*text = (PAGE_ALIGN(mm->end_code) - (mm->start_code & PAGE_MASK))
>> PAGE_SHIFT;
*data = mm->total_vm - mm->shared_vm;
*resident = *shared + get_mm_counter(mm, anon_rss);
+ *lib = mm->exec_vm - *text;
return mm->total_vm;
}

diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 8f5c05d..d851ff7 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -92,7 +92,7 @@ unsigned long task_vsize(struct mm_struct *mm)
}

int task_statm(struct mm_struct *mm, int *shared, int *text,
- int *data, int *resident)
+ int *data, int *resident, int *lib)
{
struct vm_area_struct *vma;
struct vm_region *region;


2009-12-07 11:19:47

by Hugh Dickins

[permalink] [raw]
Subject: Re: [Patch] proc: fill 'lib' field in /proc/<pid>/statm

On Mon, 7 Dec 2009, Amerigo Wang wrote:

> Currently, the 'lib' field of /proc/<pid>/statm is
> always 0, we should fill it with the right value,
> the same with 'VmLib'.

The right value (if you're looking for consistency with Linux 2.4)
is the number of currently resident "library" pages: and we don't
know that number - we can't even define what a library is.

We could add some code to make it show the same bogus number as
we show somewhere else, but it has said 0 ever since 2.5.37: so
I don't think it's worth a line of code myself, but bow to others.

>
> Signed-off-by: WANG Cong <[email protected]>
> Cc: Eric W. Biederman <[email protected]>
> Cc: Alexey Dobriyan <[email protected]>
> Cc: Al Viro <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Cc: [email protected]

What does alarm me is that you think this is fit for -stable!

Hugh

2009-12-07 11:40:19

by Cong Wang

[permalink] [raw]
Subject: Re: [Patch] proc: fill 'lib' field in /proc/<pid>/statm

Hugh Dickins wrote:
> On Mon, 7 Dec 2009, Amerigo Wang wrote:
>
>> Currently, the 'lib' field of /proc/<pid>/statm is
>> always 0, we should fill it with the right value,
>> the same with 'VmLib'.
>
> The right value (if you're looking for consistency with Linux 2.4)
> is the number of currently resident "library" pages: and we don't
> know that number - we can't even define what a library is.

Hmm, the current algorithm is just kicking out text size of itself
from ->exec_vm, it really makes some sense, but not always.

>
> We could add some code to make it show the same bogus number as
> we show somewhere else, but it has said 0 ever since 2.5.37: so
> I don't think it's worth a line of code myself, but bow to others.


If you mean 'VmLib' in /proc/<pid>/status, this is the same with it.

>
>> Signed-off-by: WANG Cong <[email protected]>
>> Cc: Eric W. Biederman <[email protected]>
>> Cc: Alexey Dobriyan <[email protected]>
>> Cc: Al Viro <[email protected]>
>> Cc: Mel Gorman <[email protected]>
>> Cc: [email protected]
>
> What does alarm me is that you think this is fit for -stable!
>

Oh, sorry, I thought missing this field is a mistake...
Dropped.

Thanks.