Weirdness in /proc/<pid>/maps and /proc/<pid>/stat.
Subsystems (7)
With commit d899bf7b, the behavior of field 28 of /proc/<pid>/stat was changed as was /proc/<pid>/maps. I don't know if that change was correct, but its resulting behavior is much more difficult to explain. I was wondering if we could determine what the "correct" behavior is before I spend much more time trying to give it the wrong behavior.
My test program is attached below. Essentially:fork() -> pthread_create() -> fork()
x86_64 2.6.32 stable kernel:
Step stat-28 maps-threadstack
p (parent) 0x7fff5607ddc0 N/A
c (child) 0x7fff55c7dc50 N/A
ppthread 0x7f2cf5c9bff0 0x7f2cf5c9d000:007feff0
ppthread+fork 0x7f2cf589be30 0x7f2cf5c9d000:003fee30
cpthread 0x7f2cf589be30 0x7f2cf5c9d000:007feff0
cpthread+fork 0x7f2cf589be30 0x7f2cf5c9d000:003fee30
Note: For all of the above, the /proc/<pids>/task/*/maps files had the stack line like:
7fff55c7d000-7fff56081000 rw-p 00000000 00:00 0 [stack]
The problems I see are:
1) In the fork() case, we are using the current userland stack pointer for task->stack_start. This appears wrong as the function which called fork() may be returned to and may further return to higher level callers, finding sp now beyond the value reported in /proc/self/stat. Additionally, the value reported for the child of the fork has no relationship to the stack size rlimit any longer.
2) In the pthread + fork case, in addition to the problem above, the size information in /proc/self/maps is incorrect as it does not take into consideration the same return paths.
The problem I am running into is coming up with any way to make the task->stack_start value usable.
Thanks,
Robin Holt
------------------------------- Example -------------------------------- #include <pthread.h>
#include <stdio.h>
#include <unistd.h>
#include <asm-generic/unistd.h>
#include <sys/types.h>
pid_tmy_fork(void)
{
char test_array[2097152];
pid_t child;
printf("pid %d my_fork array at %p\n", getpid(), test_array);
child = fork();
return child;
}void
*as_pthread(void *ignore)
{
char test_array[2097152];
printf("pid %d pthread array at %p\n", getpid(), test_array);
my_fork();
sleep(600);
return NULL;
}int
main (int argc, char **argv)
{
char test_array[2097152];
pid_t child;
pthread_t pt;
printf("pid %d is parent 0x%p\n", getpid(), test_array);
child = my_fork();
if (child)
printf("child of main is %d\n", child);
else
sleep(1);
pthread_create(&pt, NULL, as_pthread, NULL);
sleep(600);
return 0;
}Am Dienstag, den 27.04.2010, 13:53 -0500 schrieb Robin Holt:
Quoted 9 lines
With commit
d899bf7b, the behavior of field 28 of /proc/<pid>/stat was changed as was /proc/<pid>/maps. I don't know if that change was correct, but its resulting behavior is much more difficult to explain. I was wondering if we could determine what the "correct" behavior is before I spend much more time trying to give it the wrong behavior.My test program is attached below. Essentially:
fork()->pthread_create()->fork()
Quoted 8 lines
x86_642.6.32 stable kernel:
Step stat-28 maps-threadstack
p (parent)0x7fff5607ddc0N/A
c (child)0x7fff55c7dc50N/A
ppthread0x7f2cf5c9bff00x7f2cf5c9d000:007feff0
ppthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
cpthread0x7f2cf589be300x7f2cf5c9d000:007feff0
cpthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
I must guess, because the output of your example code does not fit to your description. But the stat-28 make sense for me. A thread gets a new stack, a fork inherits the stack of the process.
Quoted 4 lines
Note: For all of the above, the /proc/<pids>/task/*/maps files had the stack line like:
7fff55c7d000-7fff56081000 rw-p 00000000 00:00 0 [stack]
This should be okay, because this is the stack of the main thread. The stack of the thread is marked as [thread stack: xxxxxxxx]. Where xxxxxxxx is the max size of the stack, because the thread stack could start at a arbitrary position inside the VMA.
Quoted 16 lines
The problems I see are:
1) In thefork()case, we are using the current userland stack pointer fortask->stack_start. This appears wrong as the function which calledfork()may be returned to and may further return to higher level callers, finding sp now beyond the value reported in/proc/self/stat. Additionally, the value reported for the child of the fork has no relationship to the stack size rlimit any longer.2) In the pthread + fork case, in addition to the problem above, the size information in
/proc/self/mapsis incorrect as it does not take into consideration the same return paths.The problem I am running into is coming up with any way to make the
task->stack_startvalue usable.
Quoted 19 lines · Stefani Seibold
Am Dienstag, den 27.04.2010, 13:53 -0500 schrieb Robin Holt:
Quoted 9 lines
With commit
d899bf7b, the behavior of field 28 of /proc/<pid>/stat was changed as was /proc/<pid>/maps. I don't know if that change was correct, but its resulting behavior is much more difficult to explain. I was wondering if we could determine what the "correct" behavior is before I spend much more time trying to give it the wrong behavior.My test program is attached below. Essentially:
fork()->pthread_create()->fork()Quoted 8 lines
x86_642.6.32 stable kernel:
Step stat-28 maps-threadstackp (parent) 0x7fff5607ddc0 N/A c (child) 0x7fff55c7dc50 N/A ppthread 0x7f2cf5c9bff0 0x7f2cf5c9d000:007feff0 ppthread+fork 0x7f2cf589be30 0x7f2cf5c9d000:003fee30 cpthread 0x7f2cf589be30 0x7f2cf5c9d000:007feff0 cpthread+fork 0x7f2cf589be30 0x7f2cf5c9d000:003fee30
Quoted 3 lines
I must guess, because the output of your example code does not fit to your description. But the stat-28 make sense for me. A thread gets a new stack, a fork inherits the stack of the process.
In the fork case, the child's stack is not limited to its location at the time of the fork call, but rather the top which the parent used. The first two lines illustrate this. Additionally, as the child returns from the my_fork() function, its sp is much closer to the stat-28 field of the parent above and much (in my test code 2MB) higher than what field 28 is reporting as the start of the stack.
My point was field 28 is currently a nearly random value due to the setting task->stack_start at the time fork is called. The current stack pointer at that time has a very limited relationship to the start of the stack.
This point further factors into the max stack size calculation. In the case of the pthread'd tasks which did the fork, their max stack size is really the same as their parents. Just because part of the stack is consumed at the time of the fork call does not mean that portion of the stack is not available to the task, merely that it was used at the time of the system call.
Quoted 6 lines
Quoted 5 lines
Note: For all of the above, the /proc/<pids>/task/*/maps files had the stack line like:
7fff55c7d000-7fff56081000 rw-p 00000000 00:00 0 [stack]
Quoted 21 lines
This should be okay, because this is the stack of the main thread. The stack of the thread is marked as [thread stack: xxxxxxxx]. Where xxxxxxxx is the max size of the stack, because the thread stack could start at a arbitrary position inside the VMA.
Quoted 16 lines
The problems I see are:
1) In thefork()case, we are using the current userland stack pointer fortask->stack_start. This appears wrong as the function which calledfork()may be returned to and may further return to higher level callers, finding sp now beyond the value reported in/proc/self/stat. Additionally, the value reported for the child of the fork has no relationship to the stack size rlimit any longer.2) In the pthread + fork case, in addition to the problem above, the size information in
/proc/self/mapsis incorrect as it does not take into consideration the same return paths.The problem I am running into is coming up with any way to make the
task->stack_startvalue usable.
Thanks,
Robin
Quoted 22 lines
With commit
d899bf7b, the behavior of field 28 of /proc/<pid>/stat was changed as was /proc/<pid>/maps. I don't know if that change was correct, but its resulting behavior is much more difficult to explain. I was wondering if we could determine what the "correct" behavior is before I spend much more time trying to give it the wrong behavior.My test program is attached below. Essentially:
fork()->pthread_create()->fork()
x86_642.6.32 stable kernel:
Step stat-28 maps-threadstack
p (parent)0x7fff5607ddc0N/A
c (child)0x7fff55c7dc50N/A
ppthread0x7f2cf5c9bff00x7f2cf5c9d000:007feff0
ppthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
cpthread0x7f2cf589be300x7f2cf5c9d000:007feff0
cpthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
Note: For all of the above, the /proc/<pids>/task/*/maps files had the stack line like:
7fff55c7d000-7fff56081000 rw-p 00000000 00:00 0 [stack]
Quoted 8 lines
The problems I see are:
1) In thefork()case, we are using the current userland stack pointer fortask->stack_start. This appears wrong as the function which calledfork()may be returned to and may further return to higher level callers, finding sp now beyond the value reported in/proc/self/stat. Additionally, the value reported for the child of the fork has no relationship to the stack size rlimit any longer.
BUG.
Quoted 4 lines
2) In the pthread + fork case, in addition to the problem above, the size information in
/proc/self/mapsis incorrect as it does not take into consideration the same return paths.
BUG.
Robin, do you really need this feature? if not, I'll revert this one.
sidenote: if anyone really need to know thread stack range, I think we need to prevent vma consoliation of thread stack, iow need to implement MAP_STACK.
Quoted 3 lines
The problem I am running into is coming up with any way to make the
task->stack_startvalue usable.
Quoted 34 lines · KOSAKI Motohiro
Quoted 22 lines
With commit
d899bf7b, the behavior of field 28 of /proc/<pid>/stat was changed as was /proc/<pid>/maps. I don't know if that change was correct, but its resulting behavior is much more difficult to explain. I was wondering if we could determine what the "correct" behavior is before I spend much more time trying to give it the wrong behavior.My test program is attached below. Essentially:
fork()->pthread_create()->fork()
x86_642.6.32 stable kernel:
Step stat-28 maps-threadstack
p (parent)0x7fff5607ddc0N/A
c (child)0x7fff55c7dc50N/A
ppthread0x7f2cf5c9bff00x7f2cf5c9d000:007feff0
ppthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
cpthread0x7f2cf589be300x7f2cf5c9d000:007feff0
cpthread+fork0x7f2cf589be300x7f2cf5c9d000:003fee30
Note: For all of the above, the /proc/<pids>/task/*/maps files had the stack line like:
7fff55c7d000-7fff56081000 rw-p 00000000 00:00 0 [stack]Quoted 8 lines
The problems I see are:
1) In thefork()case, we are using the current userland stack pointer fortask->stack_start. This appears wrong as the function which calledfork()may be returned to and may further return to higher level callers, finding sp now beyond the value reported in/proc/self/stat. Additionally, the value reported for the child of the fork has no relationship to the stack size rlimit any longer.BUG.
Quoted 8 lines
Quoted 4 lines
2) In the pthread + fork case, in addition to the problem above, the size information in
/proc/self/mapsis incorrect as it does not take into consideration the same return paths.BUG.
Robin, do you really need this feature? if not, I'll revert this one.
The person who reported the problem does not need the threadstack feature. They do need a predictable way to find the start_stack value.
I do not see any way to make it consistent without finding the vma for the stack and seeing if it covers both the task->stack_start and the stack_start and if not, splitting the vma to fit the stack_start and stack_size range.
As noted, that means we need to make these vma's non-mergable and also means a heavily pthreaded application will have considerably more vmas. Also, as pthreads exit, we need to ensure that userland takes care of cleaning up the vmas or we are likely to have trouble when a pthread_exit() is followed by another pthread_create().
My head is spinning. I really feel like a revert and then clearly scoping the intended behavior and finding a way to implement that behavior is the best way forward. I can be convinced otherwise, but I hope to not do the work ;)
Thanks,
Robin
PATCH 1/1 Revert commit d899bf7b and its fixup commits -V1.
Originally, commit d899bf7b attempted to introduce a new feature for showing where the threadstack was located and how many pages are being utilized by the stack.
Commit c44972f1 was applied to fix the NO_MMU case.
Commit 89240ba0 was applied to fix a bug in ia32 executables being loaded.
Commit 9ebd4eba7 was applied to fix a bug which had kernel threads printing a userland stack address.
Commit 1306d603f was then applied to revert the stack pages being used to solve a significant performance regression.
This patch nearly undoes the effect of all these patches.
The reason for reverting these is it provides an unusable value in field 28. For x86_64, a fork will result in the task->stack_start value being updated to the current user top of stack and not the stack start address. This unpredictability of the stack_start value makes it worthless. That includes the intended use of showing how much stack space a thread has.
Other architectures will get different values. As an example, ia64 gets 0. The do_fork() and copy_process() functions appear to treat the stack_start and stack_size parameters as architecture specific.
I only partially reverted c44972f1. If I had completely reverted it, I would have had to change mm/Makefile only build pagewalk.o when CONFIG_PROC_PAGE_MONITOR is configured. Since I could not test the builds without significant effort, I decided to not change mm/Makefile.
I only partially reverted 89240ba0. I left the KSTK_ESP() change in place as that seemed worthwhile.
---
Documentation/filesystems/proc.txt | 3 +-- fs/compat.c | 2 -- fs/exec.c | 2 -- fs/proc/array.c | 3 +-- fs/proc/task_mmu.c | 19 ------------------- include/linux/sched.h | 1 - kernel/fork.c | 2 -- 7 files changed, 2 insertions(+), 30 deletions(-)
Documentation/filesystems/proc.txt +1 -2
@@ -316,7 +316,7 @@ address perms offset dev in 08049000-0804a000 rw-p 00001000 03:00 8312 /opt/test 0804a000-0806b000 rw-p 00000000 00:00 0 [heap] a7cb1000-a7cb2000 ---p 00000000 00:00 0 -a7cb2000-a7eb2000 rw-p 00000000 00:00 0 [threadstack:001ff4b4] +a7cb2000-a7eb2000 rw-p 00000000 00:00 0 a7eb2000-a7eb3000 ---p 00000000 00:00 0 a7eb3000-a7ed5000 rw-p 00000000 00:00 0 a7ed5000-a8008000 r-xp 00000000 03:00 4222 /lib/libc.so.6
@@ -352,7 +352,6 @@ is not associated with a file: [stack] = the stack of the main process [vdso] = the "virtual dynamic shared object", the kernel system call handler - [threadstack:xxxxxxxx] = the stack of the thread, xxxxxxxx is the stack size or if empty, the mapping is anonymous.
fs/compat.c +0 -2
@@ -1531,8 +1531,6 @@ int compat_do_execve(char * filename,if (retval <0 ) goto out; - current->stack_start = current->mm->start_stack; -/* execve succeeded */ current->fs->in_exec =0 ; current->in_execve =0 ;
fs/exec.c +0 -2
@@ -1387,8 +1387,6 @@ int do_execve(char * filename,if (retval <0 ) goto out; - current->stack_start = current->mm->start_stack; -/* execve succeeded */ current->fs->in_exec =0 ; current->in_execve =0 ;
fs/proc/array.c +1 -2
@@ -81,7 +81,6 @@#include <linux/pid_namespace.h> #include <linux/ptrace.h> #include <linux/tracehook.h> -#include <linux/swapops.h> #include <asm/pgtable.h> #include <asm/processor.h>
@@ -495,7 +494,7 @@ static int do_task_stat(struct seq_file rsslim, mm ? mm->start_code :0 , mm ? mm->end_code :0 , - (permitted && mm) ? task->stack_start :0 , + (permitted && mm) ? mm->start_stack :0 , esp, eip,/* The signal information here is obsolete.
fs/proc/task_mmu.c +0 -19
@@ -247,25 +247,6 @@ static void show_map_vma(struct seq_file }else if (vma->vm_start <= mm->start_stack && vma->vm_end >= mm->start_stack) { name ="[stack]" ; - }else { -unsigned long stack_start; -struct proc_maps_private *pmp; - - pmp = m->private; - stack_start = pmp->task->stack_start; - -if (vma->vm_start <= stack_start && - vma->vm_end >= stack_start) { -pad_len_spaces (m, len); -seq_printf (m, -"[threadstack:%08lx]" , -#ifdef CONFIG_STACK_GROWSUP - vma->vm_end - stack_start -#else - stack_start - vma->vm_start -#endif - ); - } } }else { name ="[vdso]" ;
include/linux/sched.h +0 -1
@@ -1497,7 +1497,6 @@ struct task_struct {/* bitmask of trace recursion */ unsigned long trace_recursion;#endif /* CONFIG_TRACING */ -unsigned long stack_start; #ifdef CONFIG_CGROUP_MEM_RES_CTLR /* memcg uses this to do batch job */ struct memcg_batch_info { int do_batch; /* incremented when batch uncharge started */
kernel/fork.c +0 -2
@@ -1114,8 +1114,6 @@ static struct task_struct *copy_process( p->bts =NULL ; - p->stack_start = stack_start; -/* Perform scheduler related setup. Assign this task to a CPU. */ sched_fork (p, clone_flags);