Is /proc/#/statm worth fixing?

Is /proc/#/statm worth fixing?

[Roger Luethi]

/proc/#/statm is a joke. Out of 7 columns, 2 are always zero in 2.6. Of
the remaining columns, at least one more is incorrect. You can most
certainly get all the intended values off /proc/#/status anyway [1].

In 2.4, more columns show actual data, but also more of them are wrong.
To top it off, 2.4 and 2.6 show vastly different numbers for several
colums (where they clearly shouldn't).

/proc/#/statm is bust and any tool relying on it is broken. Can we just
remove that file? Maybe print poisoned values in 2.6 to prevent the odd
program from crashing (if there are any), and remove it in 2.7.

Roger

[1] Mind you it's tricky to find out what correct behaviour would be if the
    only documentation you can find has obviously been wrong forever.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Is /proc/#/statm worth fixing?

[William Lee Irwin III]

On Mon, Aug 11, 2003 at 11:02:13AM +0200, Roger Luethi wrote:
> /proc/#/statm is a joke. Out of 7 columns, 2 are always zero in 2.6. Of
> the remaining columns, at least one more is incorrect. You can most
> certainly get all the intended values off /proc/#/status anyway [1].
> In 2.4, more columns show actual data, but also more of them are wrong.
> To top it off, 2.4 and 2.6 show vastly different numbers for several
> colums (where they clearly shouldn't).
> /proc/#/statm is bust and any tool relying on it is broken. Can we just
> remove that file? Maybe print poisoned values in 2.6 to prevent the odd
> program from crashing (if there are any), and remove it in 2.7.

I've restored a number of the fields to the 2.4.x semantics in tandem
with a forward port of bcrl's O(1) proc_pid_statm() patch.

I dumped the forward port of the patch into -wli, available at:
ftp://ftp.kernel.org/pub/linux/kernel/people/wli/kernels/

It's unclear how much traction it will get, as it's mildly overweight
as far as patches go, though I wouldn't go so far as to call it invasive
(opinions will vary, of course).


-- wli
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Is /proc/#/statm worth fixing?

[Roger Luethi]

On Mon, 11 Aug 2003 09:02:22 -0700, William Lee Irwin III wrote:
> I've restored a number of the fields to the 2.4.x semantics in tandem

So what _are_ the semantics?

# total program size (linux-2.6.0-test3/Documentation/filesystems/proc.txt)
2.6 and I believe the first field (size) should be the same (in pages, of
course) as VmSize, but 2.4 doesn't think so.

# size of memory portions (proc.txt)
The second field (resident) is actually equal to VmRSS on both 2.4 and 2.6.
That looks okay.

# number of pages that are shared (proc.txt)
Field three (shared) is text + data for program and shared libs on 2.6, but
not on on 2.4.

# number of pages that are 'code' (proc.txt)
Field four is program text + data.

# number of pages of data/stack (proc.txt; wrong, this should be field six)
Field five is set to 0 in 2.6 (lib), and AFAICT always equals 0 on 2.4
(lrs) although it pretends to work out the libraries' size (by checking for
vm_end > 0x60000000 which seems rather odd).

# number of pages of library (proc.txt; wrong, should be field five)
Field six is text + data for program and shared libs + some anonymous
mappings for 2.6 (data), but not on 2.4 (drs).

# number of dirty pages (proc.txt)
Field seven is always 0 on 2.6, but not on 2.4 (dt).

We can get all that data from /proc/#/maps and /proc/#/status (minus the
dirty pages which are always 0 in 2.6 anyway).

Are there _any_ programs using /proc/#/statm for real and producing
meaningful data from it? I doubt it. I don't think the problem is 2.6 which
has actually more values that seem correct as it is now. Since statm has
been broken in 2.4, fixing it for 2.6 means basically _introducing_ a file
full of redundant information with unclear semantics, a file which nobody
missed in 2.4. I still think the file should die.

> I dumped the forward port of the patch into -wli, available at:
> ftp://ftp.kernel.org/pub/linux/kernel/people/wli/kernels/

Is it this one? (latest one I found)
ftp://ftp.kernel.org/pub/linux/kernel/people/wli/kernels/2.6.0-test1/2.6.0-test1-wli-1B.bz2

Roger
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: Is /proc/#/statm worth fixing?

[William Lee Irwin III]

On Mon, 11 Aug 2003 09:02:22 -0700, William Lee Irwin III wrote:
>> I've restored a number of the fields to the 2.4.x semantics in tandem

On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> So what _are_ the semantics?
> # total program size (linux-2.6.0-test3/Documentation/filesystems/proc.txt)
> 2.6 and I believe the first field (size) should be the same (in pages, of
> course) as VmSize, but 2.4 doesn't think so.

The 2.4.x behavior is a bug; it's counting ptes beneath vmas in already-
instantiated pagetable pages, which are frequently an underestimate.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # size of memory portions (proc.txt)
> The second field (resident) is actually equal to VmRSS on both 2.4 and 2.6.
> That looks okay.

This one's hard to screw up.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # number of pages that are shared (proc.txt)
> Field three (shared) is text + data for program and shared libs on 2.6, but
> not on on 2.4.

2.4.x semantics are unclear; it's counting ptes pointing to pages with
page_count(page) > 1; this is somewhat ugly (and in fact is buggy on
various discontiguous memory machines and others where iospace isn't
covered by mem_map[]) and can misclassify pages with reference counts
elevated due to the buffer cache or other operations as shared.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # number of pages that are 'code' (proc.txt)
> Field four is program text + data.

Odd that "code" should include non-executable data.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # number of pages of data/stack (proc.txt; wrong, this should be field six)
> Field five is set to 0 in 2.6 (lib), and AFAICT always equals 0 on 2.4
> (lrs) although it pretends to work out the libraries' size (by checking for
> vm_end > 0x60000000 which seems rather odd).

0x60000000 should be TASK_UNMAPPED_BASE for starters, but it's unclear
why 2.4.x thinks all of this is "library".


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # number of pages of library (proc.txt; wrong, should be field five)
> Field six is text + data for program and shared libs + some anonymous
> mappings for 2.6 (data), but not on 2.4 (drs).

This should be "data" of some kind. Apparently 2.6.x got a bit liberal.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> # number of dirty pages (proc.txt)
> Field seven is always 0 on 2.6, but not on 2.4 (dt).

This is hard to count; the O(1) proc_pid_statm() patch ticks a counter
every time a pte is set dirty, which is better than scanning but still
not accurate.


On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> We can get all that data from /proc/#/maps and /proc/#/status (minus the
> dirty pages which are always 0 in 2.6 anyway).
> Are there _any_ programs using /proc/#/statm for real and producing
> meaningful data from it? I doubt it. I don't think the problem is 2.6 which
> has actually more values that seem correct as it is now. Since statm has
> been broken in 2.4, fixing it for 2.6 means basically _introducing_ a file
> full of redundant information with unclear semantics, a file which nobody
> missed in 2.4. I still think the file should die.

Not entirely unreasonable.


On Mon, 11 Aug 2003 09:02:22 -0700, William Lee Irwin III wrote:
>> I dumped the forward port of the patch into -wli, available at:
>> ftp://ftp.kernel.org/pub/linux/kernel/people/wli/kernels/

On Mon, Aug 11, 2003 at 11:52:35PM +0200, Roger Luethi wrote:
> Is it this one? (latest one I found)
> ftp://ftp.kernel.org/pub/linux/kernel/people/wli/kernels/2.6.0-test1/2.6.0-test1-wli-1B.bz2

It's better to look at the tarballs; the patches are broken out in them,
which will help isolate it from the other changes.


-- wli
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

[PATCH] Deprecate /proc/#/statm

[Roger Luethi]

On Mon, 11 Aug 2003 15:16:46 -0700, William Lee Irwin III wrote:
> > has actually more values that seem correct as it is now. Since statm has
> > been broken in 2.4, fixing it for 2.6 means basically _introducing_ a file
> > full of redundant information with unclear semantics, a file which nobody
> > missed in 2.4. I still think the file should die.
> 
> Not entirely unreasonable.

Alright. We have established that /proc/#/statm has been useless at least
since 2.4. Procps doesn't even bother reading it.

I propose this very non-invasive patch for 2.6. It replaces all values
printed in statm (all of which are either redundant or bogus) with 0s (for
kblockd and others statm is a line of zeroes already). IMO the real surgery
should happen in 2.7.

Comments? Andrew?

Roger

--- fs/proc/array.c.orig	2003-08-12 11:21:54.599717655 +0200
+++ fs/proc/array.c	2003-08-12 12:09:28.912397328 +0200
@@ -392,6 +392,8 @@
 int proc_pid_statm(struct task_struct *task, char *buffer)
 {
 	int size = 0, resident = 0, shared = 0, text = 0, lib = 0, data = 0;
+	/* TODO Rip out /proc/#/statm in 2.7 */
+#if 0
 	struct mm_struct *mm = get_task_mm(task);
 	
 	if (mm) {
@@ -401,6 +403,7 @@
 
 		mmput(mm);
 	}
+#endif
 
 	return sprintf(buffer,"%d %d %d %d %d %d %d\n",
 		       size, resident, shared, text, lib, data, 0);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Deprecate /proc/#/statm

[William Lee Irwin III]

On Mon, 11 Aug 2003 15:16:46 -0700, William Lee Irwin III wrote:
>> Not entirely unreasonable.

On Tue, Aug 12, 2003 at 12:40:46PM +0200, Roger Luethi wrote:
> Alright. We have established that /proc/#/statm has been useless at least
> since 2.4. Procps doesn't even bother reading it.
> I propose this very non-invasive patch for 2.6. It replaces all values
> printed in statm (all of which are either redundant or bogus) with 0s (for
> kblockd and others statm is a line of zeroes already). IMO the real surgery
> should happen in 2.7.
> Comments? Andrew?

Best to just delete the code instead of the #if 0


-- wli
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Deprecate /proc/#/statm

[Roger Luethi]

On Tue, 12 Aug 2003 17:36:20 -0700, William Lee Irwin III wrote:
> Best to just delete the code instead of the #if 0

Like this?

Roger

diff -ur linux-2.5.orig/fs/proc/array.c linux-2.5/fs/proc/array.c
--- linux-2.5.orig/fs/proc/array.c	2003-08-13 09:50:15.485989476 +0200
+++ linux-2.5/fs/proc/array.c	2003-08-13 09:45:03.646004248 +0200
@@ -388,19 +388,10 @@
 	return res;
 }
 
-extern int task_statm(struct mm_struct *, int *, int *, int *, int *);
 int proc_pid_statm(struct task_struct *task, char *buffer)
 {
 	int size = 0, resident = 0, shared = 0, text = 0, lib = 0, data = 0;
-	struct mm_struct *mm = get_task_mm(task);
-	
-	if (mm) {
-		down_read(&mm->mmap_sem);
-		size = task_statm(mm, &shared, &text, &data, &resident);
-		up_read(&mm->mmap_sem);
-
-		mmput(mm);
-	}
+	/* TODO Rip /proc/#/statm out in 2.7 */
 
 	return sprintf(buffer,"%d %d %d %d %d %d %d\n",
 		       size, resident, shared, text, lib, data, 0);
diff -ur linux-2.5.orig/fs/proc/task_mmu.c linux-2.5/fs/proc/task_mmu.c
--- linux-2.5.orig/fs/proc/task_mmu.c	2003-08-13 09:45:57.507057766 +0200
+++ linux-2.5/fs/proc/task_mmu.c	2003-08-13 09:41:41.844772701 +0200
@@ -48,33 +48,6 @@
 	return PAGE_SIZE * mm->total_vm;
 }
 
-int task_statm(struct mm_struct *mm, int *shared, int *text,
-	       int *data, int *resident)
-{
-	struct vm_area_struct *vma;
-	int size = 0;
-
-	*resident = mm->rss;
-	for (vma = mm->mmap; vma; vma = vma->vm_next) {
-		int pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
-
-		size += pages;
-		if (is_vm_hugetlb_page(vma)) {
-			if (!(vma->vm_flags & VM_DONTCOPY))
-				*shared += pages;
-			continue;
-		}
-		if (vma->vm_flags & VM_SHARED || !list_empty(&vma->shared))
-			*shared += pages;
-		if (vma->vm_flags & VM_EXECUTABLE)
-			*text += pages;
-		else
-			*data += pages;
-	}
-
-	return size;
-}
-
 static int show_map(struct seq_file *m, void *v)
 {
 	struct vm_area_struct *map = v;
diff -ur linux-2.5.orig/fs/proc/task_nommu.c linux-2.5/fs/proc/task_nommu.c
--- linux-2.5.orig/fs/proc/task_nommu.c	2003-08-13 09:45:57.509057471 +0200
+++ linux-2.5/fs/proc/task_nommu.c	2003-08-13 09:42:06.212178570 +0200
@@ -75,28 +75,6 @@
 	return vsize;
 }
 
-int task_statm(struct mm_struct *mm, int *shared, int *text,
-	       int *data, int *resident)
-{
-	struct mm_tblock_struct *tbp;
-	int size = kobjsize(mm);
-	
-	for (tbp = &mm->context.tblock; tbp; tbp = tbp->next) {
-		if (tbp->next)
-			size += kobjsize(tbp->next);
-		if (tbp->rblock) {
-			size += kobjsize(tbp->rblock);
-			size += kobjsize(tbp->rblock->kblock);
-		}
-	}
-
-	size += (*text = mm->end_code - mm->start_code);
-	size += (*data = mm->start_stack - mm->start_data);
-
-	*resident = size;
-	return size;
-}
-
 /*
  * Albert D. Cahalan suggested to fake entries for the traditional
  * sections here.  This might be worth investigating.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Deprecate /proc/#/statm

[William Lee Irwin III]

On Tue, 12 Aug 2003 17:36:20 -0700, William Lee Irwin III wrote:
>> Best to just delete the code instead of the #if 0

On Wed, Aug 13, 2003 at 09:58:14AM +0200, Roger Luethi wrote:
> Like this?
> Roger

Yep, thanks.


-- wli
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>