fs, proc: Add start_data, end_data, start_brk members to /proc/$pid/stat v2

It helps to dump and restore this mm_struct members.

 fs/proc/array.c |    7 +++++--

Quoted 1 line · On Mon, 5 Dec 2011 22:20:11 +0400 Cyrill Gorcunov <[email protected]> wrote:

It helps to dump and restore this mm_struct members.

Sorry, I'm not going to apply a patch with a changelog like that.

It needs a full description of how this will be used and why it is needed. So that others can understand and evaluate the proposal, and any possible alternatives.

Such a description would also let us see whether this is just some "hey, maybe we'll use this sometime" thing, in which case it should not yet be applied.

Quoted 2 lines

 fs/proc/array.c |    7 +++++--

Also, every change to any procfs interface should have an associated change to procfs documentation. If no relevant document exists then heck, write one. No exceptions.

Quoted 7 lines · Andrew Morton

Quoted 1 line · On Mon, 5 Dec 2011 22:20:11 +0400 Cyrill Gorcunov <[email protected]> wrote:

It helps to dump and restore this mm_struct members.

Sorry, I'm not going to apply a patch with a changelog like that.

OK, I'll update it. The problem is that while we "read" this entries from procfs, the restoration is supposed to be done via prctl interface (the patch is in fly, and I'm trying to improve it at moment).

Quoted 7 lines

It needs a full description of how this will be used and why it is needed. So that others can understand and evaluate the proposal, and any possible alternatives.

Such a description would also let us see whether this is just some "hey, maybe we'll use this sometime" thing, in which case it should not yet be applied.

I understand.

Quoted 3 lines

Quoted 2 lines

 fs/proc/array.c |    7 +++++--

Quoted 4 lines

Also, every change to any procfs interface should have an associated change to procfs documentation. If no relevant document exists then heck, write one. No exceptions.

ok, thanks.

Cyrill

+  start_data    address above which program data is placed
+  end_data      address above which program data is placed

Quoted 2 lines · Cyrill Gorcunov

+  start_data    address above which program data is placed
+  end_data      address above which program data is placed

Should the latter be "address below which program data is placed"?

Documentation/filesystems/proc.txt Table 1-4: Contents of the stat files (a · line 310

Quoted 13 lines · On Wed, 7 Dec 2011 02:53:15 +0400 Cyrill Gorcunov <[email protected]> wrote:

Index: linux-2.6.git/Documentation/filesystems/proc.txt
===================================================================

Documentation/filesystems/proc.txt +3 -0

@@ -305,6 +305,9 @@ Table 1-4: Contents of the stat files (a
   blkio_ticks   time spent waiting for block IO
   gtime         guest time of the task in jiffies
   cgtime        guest time of the task children in jiffies
+  start_data    address above which program data is placed
+  end_data      address above which program data is placed
+  start_brk     address above which program data can be expanded with brk() call
..............................................................................

nitpick.

After reading Wikipedia, http://en.wikipedia.org/wiki/Data_segment

I prefer

start_data   address above program data+bss is placed.
end_data     address below program data+bss is placed
start_break  address above which program heap can be expaned with brk() call

rather than just 'data'.

No problem, I'll update (These segments are filled by binfmt handlers).

Quoted 3 lines · Cyrill Gorcunov

No problem, I'll update (These segments are filled by binfmt handlers).

This one should fit.

Cyrill
---
From: Cyrill Gorcunov <[email protected]>
Subject: [PATCH] fs, proc: Add start_data, end_data, start_brk members to /proc/$pid/stat v4

These members are involved into calculation of program data segment (which might be seen in /proc/<pid>/statm) and into brk() call, so in a sake of restore we need these entries to be set exactly to the same value the process had at dumpimg time.

While setting these entries back to a running task is supposed to be done via prctl interface (which is not addressed in this patch) the export of these values are done via /proc/$pid/stat, which already have start_code, end_code and start_stack entries.

v2:
- Kees and Alexey pointed out that "1" hack is unnecessary, so make it to have plain (mm && permitted) ? mm->member : 0 form.

v3:
- Andrew pointed that documentation update is missed. Chagelog is updated as well.

v4:
- Update proc.txt to address concerns from Kees and Kame.

Signed-off-by Cyrill Gorcunov <[email protected]>

Acked-by Serge Hallyn <[email protected]>

Reviewed-by Kees Cook <[email protected]>

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

CC Alexey Dobriyan <[email protected]>

---
 Documentation/filesystems/proc.txt |    3 +++
 fs/proc/array.c                    |    7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Index: linux-2.6.git/Documentation/filesystems/proc.txt
===================================================================

Documentation/filesystems/proc.txt +3 -0

@@ -305,6 +305,9 @@ Table 1-4: Contents of the stat files (a
   blkio_ticks   time spent waiting for block IO
   gtime         guest time of the task in jiffies
   cgtime        guest time of the task children in jiffies
+  start_data    address above which program data+bss is placed
+  end_data      address below which program data+bss is placed
+  start_brk     address above which program heap can be expaned with brk() call
..............................................................................
 
 The /proc/PID/maps file containing the currently mapped memory regions and

Index: linux-2.6.git/fs/proc/array.c
===================================================================

fs/proc/array.c +5 -3

@@ -464,7 +464,7 @@ static int do_task_stat(struct seq_file
 
 	seq_printf(m, "%d (%s) %c %d %d %d %d %d %u %lu \
 %lu %lu %lu %lu %lu %ld %ld %ld %ld %d 0 %llu %lu %ld %lu %lu %lu %lu %lu \
-%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld\n",
+%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld %lu %lu %lu\n",
 		pid_nr_ns(pid, ns),
 		tcomm,
 		state,

@@ -511,7 +511,10 @@ static int do_task_stat(struct seq_file
 		task->policy,
 		(unsigned long long)delayacct_blkio_ticks(task),
 		cputime_to_clock_t(gtime),
-		cputime_to_clock_t(cgtime));
+		cputime_to_clock_t(cgtime),
+		(mm && permitted) ? mm->start_data : 0,
+		(mm && permitted) ? mm->end_data : 0,
+		(mm && permitted) ? mm->start_brk : 0);
 	if (mm)
 		mmput(mm);
 	return 0;
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 2 lines · Cyrill Gorcunov

Quoted 1 line · Cyrill Gorcunov

Quoted 4 lines

Quoted 2 lines

No problem, I'll update (These segments are filled by binfmt handlers).

This one should fit.

How extensively have you tested this. I remember a few other occasions where adding elements to a line in a proc file have resulted in breaking readers.

I expect it is much safer to add this to /proc/<pid>/status rather than /proc/<pid>/stat.