linux-2.6.17-rc3-git13 fs/proc/array.c
Subsystems (2)
PATCH 1/1 linux-2.6.17-rc3-git13 fs/proc/array.c
do_task_stat() was using too much stack
from make checkstack:0x080eae08 do_task_stat: 1752
So I took almost all the variables used in the function and made a new structure do_task_stat_vars, and dynamically allocated it in the function. This is supposed to be a quick hack, and hopefully the authors will really do something about it.
Signed off by: Anne Thrax <[email protected]>
--- linux-2.6.17-rc3-git13/fs/proc/array.c 2006-05-07
23:01:50.000000000 -0400
+++ linux-uml/fs/proc/array.c 2006-05-08 01:31:12.000000000 -0400
@@ -316,7 +316,7 @@
static int do_task_stat(struct task_struct *task, char * buffer, int
whole)
{
- unsigned long vsize, eip, esp, wchan = ~0UL;
+ /* unsigned long vsize, eip, esp, wchan = ~0UL;
long priority, nice;
int tty_pgrp = -1, tty_nr = 0;
sigset_t sigign, sigcatch;
@@ -332,35 +332,49 @@
unsigned long rsslim = 0;
struct task_struct *t;
char tcomm[sizeof(task->comm)];
+ */
+ struct do_task_stat_vars *vars;
+ vars = kmalloc(sizeof(struct do_task_stat_vars), GFP_KERNEL);
+ vars->vsize = vars->eip = vars->esp = vars->wchan = ~0UL;
+ vars->tty_pgrp = -1;
+ vars->tty_nr = 0;
+ vars->pgid = -1;
+ vars->sid = -1;
+ vars->num_threads = 0;
+ vars->cmin_flt = vars->cmaj_flt = vars->min_flt = vars->maj_flt
= vars->rsslim = 0;
+ struct mm_struct *mm;
+ struct task_struct *t;
+ char tcomm[sizeof(task->comm)];
+ char state; state = *get_task_state(task);
- vsize = eip = esp = 0;
+ vars->vsize = vars->eip = vars->esp = 0;
mm = get_task_mm(task);
if (mm) {
- vsize = task_vsize(mm);
- eip = KSTK_EIP(task);
- esp = KSTK_ESP(task);
+ vars->vsize = task_vsize(mm);
+ vars->eip = KSTK_EIP(task);
+ vars->esp = KSTK_ESP(task);
}
get_task_comm(tcomm, task);- sigemptyset(&sigign);
- sigemptyset(&sigcatch);
- cutime = cstime = utime = stime = cputime_zero;
+ sigemptyset(&vars->sigign);
+ sigemptyset(&vars->sigcatch);
+ vars->cutime = vars->cstime = vars->utime = vars->stime =
cputime_zero;
read_lock(&tasklist_lock);
if (task->sighand) {
spin_lock_irq(&task->sighand->siglock);
- num_threads = atomic_read(&task->signal->count);
- collect_sigign_sigcatch(task, &sigign, &sigcatch);
+ vars->num_threads = atomic_read(&task->signal->count);
+ collect_sigign_sigcatch(task, &vars->sigign, &vars->sigcatch);
/* add up live thread stats at the group level */
if (whole) {
t = task;
do {
- min_flt += t->min_flt;
- maj_flt += t->maj_flt;
- utime = cputime_add(utime, t->utime);
- stime = cputime_add(stime, t->stime);
+ vars->min_flt += t->min_flt;
+ vars->maj_flt += t->maj_flt;
+ vars->utime = cputime_add(vars->utime,
t->utime);
+ vars->stime = cputime_add(vars->stime,
t->stime);
t = next_thread(t);
} while (t != task);
}
@@ -369,88 +383,88 @@
}
if (task->signal) {
if (task->signal->tty) {- tty_pgrp = task->signal->tty->pgrp;
- tty_nr =
new_encode_dev(tty_devnum(task->signal->tty));
+ vars->tty_pgrp = task->signal->tty->pgrp;
+ vars->tty_nr =
new_encode_dev(tty_devnum(task->signal->tty));
}
- pgid = process_group(task);
- sid = task->signal->session;
- cmin_flt = task->signal->cmin_flt;
- cmaj_flt = task->signal->cmaj_flt;
- cutime = task->signal->cutime;
- cstime = task->signal->cstime;
- rsslim = task->signal->rlim[RLIMIT_RSS].rlim_cur;
+ vars->pgid = process_group(task);
+ vars->sid = task->signal->session;
+ vars->cmin_flt = task->signal->cmin_flt;
+ vars->cmaj_flt = task->signal->cmaj_flt;
+ vars->cutime = task->signal->cutime;
+ vars->cstime = task->signal->cstime;
+ vars->rsslim = task->signal->rlim[RLIMIT_RSS].rlim_cur;
if (whole) {
- min_flt += task->signal->min_flt;
- maj_flt += task->signal->maj_flt;
- utime = cputime_add(utime, task->signal->utime);
- stime = cputime_add(stime, task->signal->stime);
+ vars->min_flt += task->signal->min_flt;
+ vars->maj_flt += task->signal->maj_flt;
+ vars->utime = cputime_add(vars->utime,
task->signal->utime);
+ vars->stime = cputime_add(vars->stime,
task->signal->stime);
}
}
- ppid = pid_alive(task) ? task->group_leader->real_parent->tgid : 0;
+ vars->ppid = pid_alive(task) ?
task->group_leader->real_parent->tgid : 0;
read_unlock(&tasklist_lock);- if (!whole || num_threads<2)
- wchan = get_wchan(task);
+ if (!whole || vars->num_threads<2)
+ vars->wchan = get_wchan(task);
if (!whole) {
- min_flt = task->min_flt;
- maj_flt = task->maj_flt;
- utime = task->utime;
- stime = task->stime;
+ vars->min_flt = task->min_flt;
+ vars->maj_flt = task->maj_flt;
+ vars->utime = task->utime;
+ vars->stime = task->stime;
}
/* scale priority and nice values from timeslices to -20..20 */
/* to make it look like a "normal" Unix priority/nice value */
- priority = task_prio(task);
- nice = task_nice(task);
+ vars->priority = task_prio(task);
+ vars->nice = task_nice(task); /* Temporary variable needed for gcc-2.96 */
/* convert timespec -> nsec*/- start_time = (unsigned long long)task->start_time.tv_sec *NSEC_PER_SEC
+ vars->start_time = (unsigned long long)task->start_time.tv_sec *NSEC_PER_SEC
+ task->start_time.tv_nsec;
/* convert nsec -> ticks */
- start_time = nsec_to_clock_t(start_time);
+ vars->start_time = nsec_to_clock_t(vars->start_time);
- res = sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %lu \
+ vars->res = sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %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 %lu %lu\n",
task->pid,
tcomm,
state,
- ppid,
- pgid,
- sid,
- tty_nr,
- tty_pgrp,
+ vars->ppid,
+ vars->pgid,
+ vars->sid,
+ vars->tty_nr,
+ vars->tty_pgrp,
task->flags,
- min_flt,
- cmin_flt,
- maj_flt,
- cmaj_flt,
- cputime_to_clock_t(utime),
- cputime_to_clock_t(stime),
- cputime_to_clock_t(cutime),
- cputime_to_clock_t(cstime),
- priority,
- nice,
- num_threads,
- start_time,
- vsize,
+ vars->min_flt,
+ vars->cmin_flt,
+ vars->maj_flt,
+ vars->cmaj_flt,
+ cputime_to_clock_t(vars->utime),
+ cputime_to_clock_t(vars->stime),
+ cputime_to_clock_t(vars->cutime),
+ cputime_to_clock_t(vars->cstime),
+ vars->priority,
+ vars->nice,
+ vars->num_threads,
+ vars->start_time,
+ vars->vsize,
mm ? get_mm_rss(mm) : 0,
- rsslim,
+ vars->rsslim,
mm ? mm->start_code : 0,
mm ? mm->end_code : 0,
mm ? mm->start_stack : 0,
- esp,
- eip,
+ vars->esp,
+ vars->eip,
/* The signal information here is obsolete.
* It must be decimal for Linux 2.0 compatibility.
* Use /proc/#/status for real-time signals.
*/
task->pending.signal.sig[0] & 0x7fffffffUL,
task->blocked.sig[0] & 0x7fffffffUL,
- sigign .sig[0] & 0x7fffffffUL,
- sigcatch .sig[0] & 0x7fffffffUL,
- wchan,
+ vars->sigign .sig[0] & 0x7fffffffUL,
+ vars->sigcatch .sig[0] & 0x7fffffffUL,
+ vars->wchan,
0UL,
0UL,
task->exit_signal,
@@ -459,6 +473,9 @@
task->policy);
if(mm)
mmput(mm);
+
+ int res = vars->res;
+ kfree(vars);
return res;}
--- linux-2.6.17-rc3-git13/include/linux/proc_fs.h 2006-05-07
23:01:54.000000000 -0400
+++ linux-uml/include/linux/proc_fs.h 2006-05-08 01:26:54.000000000 -0400
@@ -83,6 +83,25 @@
loff_t offset;
};
+/* do_task_stat() from fs/proc/array.c uses *WAY* too much stack
+ * I'll just make a struct containing all the variables in there
+ * and dynamicaly allocate it
+ * this is a quick hack, and someone should probably really do
+ * something about it
+ */
+
+struct do_task_stat_vars {
+ unsigned long vsize, eip, esp, wchan, cmin_flt, cmaj_flt, + min_flt, maj_flt, rsslim; + unsigned long long start_time; + long priority, nice; + int tty_pgrp, tty_nr, res, num_threads; + pid_t ppid, pgid, sid; + sigset_t sigign, sigcatch; + cputime_t cutime, cstime, utime, stime;
+};
+
+
#ifdef CONFIG_PROC_FS
extern struct proc_dir_entry proc_root;
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 10 lines · Anne Thrax
do_task_stat()was using too much stack
from make checkstack:0x080eae08do_task_stat: 1752So I took almost all the variables used in the function and made a new structure
do_task_stat_vars, and dynamically allocated it in the function. This is supposed to be a quick hack, and hopefully the authors will really do something about it.Signed off by: Anne Thrax <[email protected]>
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
Review
Quoted 4 lines
---
linux-2.6.17-rc3-git13/fs/proc/array.c2006-05-07
23:01:50.000000000 -0400
+++linux-uml/fs/proc/array.c2006-05-08 01:31:12.000000000 -0400(quoted diff) +0
-0@@ -316,7 +316,7 @@
Patch appears to be line-wrapped :(
Quoted 9 lines
(quoted diff) +1
-1static int do_task_stat(struct task_struct *task, char * buffer, int whole) { - unsigned long vsize, eip, esp, wchan = ~0UL; + /* unsigned long vsize, eip, esp, wchan = ~0UL; long priority, nice; int tty_pgrp = -1, tty_nr = 0; sigset_t sigign, sigcatch;
Review line 337
Quoted 6 lines
(quoted diff) +3
-0@@ -332,35 +332,49 @@ unsigned long rsslim = 0; struct task_struct *t; char tcomm[sizeof(task->comm)]; + */ + struct do_task_stat_vars *vars; + vars = kmalloc(sizeof(struct do_task_stat_vars), GFP_KERNEL);
What happens if this kmalloc fails? Can do_task_stat() return ENOMEM? What happens if it does -- will that change userspace in some way (generally unacceptable)?
<snip>
Review line 477
Quoted 6 lines
(quoted diff) +2
-0@@ -459,6 +473,9 @@ task->policy); if(mm) mmput(mm); + + int res = vars->res;
I'm almost positive that compilation would have complained about declaring a variable in the middle of a block. Did you compile-test?
Thanks,
Nish
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
Quoted 4 lines
What happens if this kmalloc fails? Can
do_task_stat()return ENOMEM? What happens if it does -- will that change userspace in some way (generally unacceptable)?
I think it's okay to return -ENOMEM...
It's a static function, that is only used by two other functions in fs/proc/array.c I don't know who uses those functions, but I would assume that it would not change userspace as it's just used for the proc filesystem. I would think that that the proc filesystem just wouldn't work.
Quoted 2 lines
I'm almost positive that compilation would have complained about declaring a variable in the middle of a block. Did you compile-test?
I removed res from struct do_task_stat_vars and declared it on the stack. It did get yelly at me about being against C99 standards before, it compiles fine now.
fs/proc/array.c 2006-05-08 03:04:09.000000000 -0400 renamed from fs/proc/array.c 2006-05-07 23:01:50.000000000 -0400 +83 -62
@@ -316,7 +316,7 @@ static int do_task_stat(struct task_struct *task, char * buffer, int whole) { - unsigned long vsize, eip, esp, wchan = ~0UL; + /*unsigned long vsize, eip, esp, wchan = ~0UL; long priority, nice; int tty_pgrp = -1, tty_nr = 0; sigset_t sigign, sigcatch;
@@ -332,35 +332,54 @@ unsigned long rsslim = 0; struct task_struct *t; char tcomm[sizeof(task->comm)]; + */ + struct do_task_stat_vars *vars; + vars = kmalloc(sizeof(struct do_task_stat_vars), GFP_KERNEL); + + if(vars = NULL) + return -ENOMEM; + + vars->vsize = vars->eip = vars->esp = vars->wchan = ~0UL; + vars->tty_pgrp = -1; + vars->tty_nr = 0; + vars->pgid = -1; + vars->sid = -1; + vars->num_threads = 0; + vars->cmin_flt = vars->cmaj_flt = vars->min_flt = vars->maj_flt = vars->rsslim = 0; + struct mm_struct *mm; + struct task_struct *t; + char tcomm[sizeof(task->comm)]; + char state; + int res; state = *get_task_state(task); - vsize = eip = esp = 0; + vars->vsize = vars->eip = vars->esp = 0; mm = get_task_mm(task); if (mm) { - vsize = task_vsize(mm); - eip = KSTK_EIP(task); - esp = KSTK_ESP(task); + vars->vsize = task_vsize(mm); + vars->eip = KSTK_EIP(task); + vars->esp = KSTK_ESP(task); } get_task_comm(tcomm, task); - sigemptyset(&sigign); - sigemptyset(&sigcatch); - cutime = cstime = utime = stime = cputime_zero; + sigemptyset(&vars->sigign); + sigemptyset(&vars->sigcatch); + vars->cutime = vars->cstime = vars->utime = vars->stime = cputime_zero; read_lock(&tasklist_lock); if (task->sighand) { spin_lock_irq(&task->sighand->siglock); - num_threads = atomic_read(&task->signal->count); - collect_sigign_sigcatch(task, &sigign, &sigcatch); + vars->num_threads = atomic_read(&task->signal->count); + collect_sigign_sigcatch(task, &vars->sigign, &vars->sigcatch); /* add up live thread stats at the group level */ if (whole) { t = task; do { - min_flt += t->min_flt; - maj_flt += t->maj_flt; - utime = cputime_add(utime, t->utime); - stime = cputime_add(stime, t->stime); + vars->min_flt += t->min_flt; + vars->maj_flt += t->maj_flt; + vars->utime = cputime_add(vars->utime, t->utime); + vars->stime = cputime_add(vars->stime, t->stime); t = next_thread(t); } while (t != task); }
@@ -369,46 +388,46 @@ } if (task->signal) { if (task->signal->tty) { - tty_pgrp = task->signal->tty->pgrp; - tty_nr = new_encode_dev(tty_devnum(task->signal->tty)); + vars->tty_pgrp = task->signal->tty->pgrp; + vars->tty_nr = new_encode_dev(tty_devnum(task->signal->tty)); } - pgid = process_group(task); - sid = task->signal->session; - cmin_flt = task->signal->cmin_flt; - cmaj_flt = task->signal->cmaj_flt; - cutime = task->signal->cutime; - cstime = task->signal->cstime; - rsslim = task->signal->rlim[RLIMIT_RSS].rlim_cur; + vars->pgid = process_group(task); + vars->sid = task->signal->session; + vars->cmin_flt = task->signal->cmin_flt; + vars->cmaj_flt = task->signal->cmaj_flt; + vars->cutime = task->signal->cutime; + vars->cstime = task->signal->cstime; + vars->rsslim = task->signal->rlim[RLIMIT_RSS].rlim_cur; if (whole) { - min_flt += task->signal->min_flt; - maj_flt += task->signal->maj_flt; - utime = cputime_add(utime, task->signal->utime); - stime = cputime_add(stime, task->signal->stime); + vars->min_flt += task->signal->min_flt; + vars->maj_flt += task->signal->maj_flt; + vars->utime = cputime_add(vars->utime, task->signal->utime); + vars->stime = cputime_add(vars->stime, task->signal->stime); } } - ppid = pid_alive(task) ? task->group_leader->real_parent->tgid : 0; + vars->ppid = pid_alive(task) ? task->group_leader->real_parent->tgid : 0; read_unlock(&tasklist_lock); - if (!whole || num_threads<2) - wchan = get_wchan(task); + if (!whole || vars->num_threads<2) + vars->wchan = get_wchan(task); if (!whole) { - min_flt = task->min_flt; - maj_flt = task->maj_flt; - utime = task->utime; - stime = task->stime; + vars->min_flt = task->min_flt; + vars->maj_flt = task->maj_flt; + vars->utime = task->utime; + vars->stime = task->stime; } /* scale priority and nice values from timeslices to -20..20 */ /* to make it look like a "normal" Unix priority/nice value */ - priority = task_prio(task); - nice = task_nice(task); + vars->priority = task_prio(task); + vars->nice = task_nice(task); /* Temporary variable needed for gcc-2.96 */ /* convert timespec -> nsec*/ - start_time = (unsigned long long)task->start_time.tv_sec * NSEC_PER_SEC + vars->start_time = (unsigned long long)task->start_time.tv_sec * NSEC_PER_SEC + task->start_time.tv_nsec; /* convert nsec -> ticks */ - start_time = nsec_to_clock_t(start_time); + vars->start_time = nsec_to_clock_t(vars->start_time); res = sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %lu \ %lu %lu %lu %lu %lu %ld %ld %ld %ld %d 0 %llu %lu %ld %lu %lu %lu %lu %lu \
@@ -416,41 +435,41 @@ task->pid, tcomm, state, - ppid, - pgid, - sid, - tty_nr, - tty_pgrp, + vars->ppid, + vars->pgid, + vars->sid, + vars->tty_nr, + vars->tty_pgrp, task->flags, - min_flt, - cmin_flt, - maj_flt, - cmaj_flt, - cputime_to_clock_t(utime), - cputime_to_clock_t(stime), - cputime_to_clock_t(cutime), - cputime_to_clock_t(cstime), - priority, - nice, - num_threads, - start_time, - vsize, + vars->min_flt, + vars->cmin_flt, + vars->maj_flt, + vars->cmaj_flt, + cputime_to_clock_t(vars->utime), + cputime_to_clock_t(vars->stime), + cputime_to_clock_t(vars->cutime), + cputime_to_clock_t(vars->cstime), + vars->priority, + vars->nice, + vars->num_threads, + vars->start_time, + vars->vsize, mm ? get_mm_rss(mm) : 0, - rsslim, + vars->rsslim, mm ? mm->start_code : 0, mm ? mm->end_code : 0, mm ? mm->start_stack : 0, - esp, - eip, + vars->esp, + vars->eip, /* The signal information here is obsolete. * It must be decimal for Linux 2.0 compatibility. * Use /proc/#/status for real-time signals. */ task->pending.signal.sig[0] & 0x7fffffffUL, task->blocked.sig[0] & 0x7fffffffUL, - sigign .sig[0] & 0x7fffffffUL, - sigcatch .sig[0] & 0x7fffffffUL, - wchan, + vars->sigign .sig[0] & 0x7fffffffUL, + vars->sigcatch .sig[0] & 0x7fffffffUL, + vars->wchan, 0UL, 0UL, task->exit_signal,
@@ -459,6 +478,8 @@ task->policy); if(mm) mmput(mm); + + kfree(vars); return res; }
include/linux/proc_fs.h 2006-05-08 02:55:49.000000000 -0400 renamed from include/linux/proc_fs.h 2006-05-07 23:01:54.000000000 -0400 +19 -0
@@ -83,6 +83,25 @@ loff_t offset; }; +/* do_task_stat() from fs/proc/array.c uses *WAY* too much stack + * I'll just make a struct containing all the variables in there + * and dynamicaly allocate it + * this is a quick hack, and someone should probably really do + * something about it + */ + +struct do_task_stat_vars { + unsigned long vsize, eip, esp, wchan, cmin_flt, cmaj_flt, + min_flt, maj_flt, rsslim; + unsigned long long start_time; + long priority, nice; + int tty_pgrp, tty_nr, num_threads; + pid_t ppid, pgid, sid; + sigset_t sigign, sigcatch; + cputime_t cutime, cstime, utime, stime; +}; + + #ifdef CONFIG_PROC_FS extern struct proc_dir_entry proc_root;
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 7 lines · Anne Thrax
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 10 lines
Quoted 3 lines
What happens if this kmalloc fails? Can
do_task_stat()return ENOMEM? What happens if it does -- will that change userspace in some way (generally unacceptable)?I think it's okay to return
-ENOMEM...
It's a static function, that is only used by two other functions infs/proc/array.c
I don't know who uses those functions, but I would assume that it would not change userspace as it's just used for the proc filesystem. I would think that that the proc filesystem just wouldn't work.
Please see who uses those functions. Userspace can see mounted proc filesystems, right? And more than likely can cat a file (or files) that somehow exports some of this data, I'd think. If that data can now not appear, but used to always appear with a large stack, then it's a userspace regression and unacceptable. Please try and figure out what this code is used for (hint: /proc/pid/stat would be my first guess), and make sure everything still works.
Quoted 6 lines
Quoted 2 lines
I'm almost positive that compilation would have complained about declaring a variable in the middle of a block. Did you compile-test?
I removed res from struct
do_task_stat_varsand declared it on the stack. It did get yelly at me about being against C99 standards before, it compiles fine now.
<snip>
Review
Quoted 5 lines
(quoted diff) +5
-0+ struct mm_struct *mm; + struct task_struct *t; + char tcomm[sizeof(task->comm)]; + char state; + int res;
<snip>
Please just put this at the top of the corresponding block. It's against accepted CodingStyle (although I'm not sure if it's explicitly so) to put these declarations in the middle of the block.
<snip>
Quoted 6 lines
+/*
do_task_stat()fromfs/proc/array.cuses *WAY* too much stack
+ * I'll just make a struct containing all the variables in there
+ * and dynamicaly allocate it
+ * this is a quick hack, and someone should probably really do
+ * something about it
+ */
Generally, we prefer comments of the form
/*
* text ...
*/
Also, it would be good, probably, to indicate who the "I" is in the comment -- it's pretty useless unless your name is somewhere in the file. Or make it imperative ("Just make a struct ..."), and let git take care of tracking your name.
Review
Quoted 10 lines
(quoted diff) +10
-0+struct do_task_stat_vars { + unsigned long vsize, eip, esp, wchan, cmin_flt, cmaj_flt, + min_flt, maj_flt, rsslim; + unsigned long long start_time; + long priority, nice; + int tty_pgrp, tty_nr, num_threads; + pid_t ppid, pgid, sid; + sigset_t sigign, sigcatch; + cputime_t cutime, cstime, utime, stime; +};
That is one big struct :)
Review
Quoted 2 lines
(quoted diff) +2
-0+ +
Extraneous newline insertion?
Thanks,
Nish
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 9 lines · Nishanth Aravamudan
Quoted 7 lines · Anne Thrax
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
Quoted 7 lines
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
thanks,
greg k-h
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 18 lines · Greg KH
Quoted 9 lines · Nishanth Aravamudan
Quoted 7 lines · Anne Thrax
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
Quoted 7 lines
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 2 lines
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
Ok, thanks for the information, Greg. Perhaps (although it may be unnecessary), this should be added to SubmittingPatches?
Thanks,
Nish
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 12 lines · Greg KH
Quoted 10 lines · Nishanth Aravamudan
Quoted 1 line · Anne Thrax
I would rather not use my real name, as I am only 14 years old.
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 2 lines
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
A pseudonym + email address performs acceptably for "trace the blame". So I don't see how your two statements are related.
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 13 lines · Matthew Wilcox
Quoted 12 lines · Greg KH
Quoted 10 lines · Nishanth Aravamudan
Quoted 1 line · Anne Thrax
I would rather not use my real name, as I am only 14 years old.
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 5 lines
Quoted 2 lines
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
A pseudonym + email address performs acceptably for "trace the blame". So I don't see how your two statements are related.
It's a matter of trust. If you want to trust us to take your changes, you need to show that you trust us enough to be willing to accept recgonition for those changes. By using a pseudonym, you are not showing a whole lot of trust, and by using it, it take a whole lot more effort to get that trust.
That being said, there are a few pseudonyms out there that I do trust, just because they have been around for years and show that they are trustworthy people.
thanks,
greg k-h
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 14 lines · Greg KH
Quoted 6 lines · Matthew Wilcox
Quoted 2 lines · Greg KH
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
A pseudonym + email address performs acceptably for "trace the blame". So I don't see how your two statements are related.
It's a matter of trust. If you want to trust us to take your changes, you need to show that you trust us enough to be willing to accept recgonition for those changes. By using a pseudonym, you are not showing a whole lot of trust, and by using it, it take a whole lot more effort to get that trust.
Quoted 3 lines
That being said, there are a few pseudonyms out there that I do trust, just because they have been around for years and show that they are trustworthy people.
So ... there's no reason to turn down someone's changes, simply because they use a pseudonym? Thanks for clearing that up.
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 3 lines · Anne Thrax
do_task_stat()was using too much stack
from make checkstack:0x080eae08do_task_stat: 1752
Here it takes 10 times less stack.
$ make checkstack | grep do_task_stat 0xc0167add do_task_stat: 172 0xc01675b9 do_task_stat: 152 0xc0167af0 do_task_stat: 152
I don't understand where such huge amount of space is wasted. Probably "tcomm" is the biggest stack consumer of do_task_stat(). Others are just longs and pointers. Is checkstack script is buggy?
Quoted 7 lines
(quoted diff) +7
-0+/* do_task_stat() from fs/proc/array.c uses *WAY* too much stack + * I'll just make a struct containing all the variables in there + * and dynamicaly allocate it + * this is a quick hack, and someone should probably really do + * something about it + */ +
Review
Quoted 10 lines
(quoted diff) +10
-0+struct do_task_stat_vars { + unsigned long vsize, eip, esp, wchan, cmin_flt, cmaj_flt, + min_flt, maj_flt, rsslim; + unsigned long long start_time; + long priority, nice; + int tty_pgrp, tty_nr, res, num_threads; + pid_t ppid, pgid, sid; + sigset_t sigign, sigcatch; + cputime_t cutime, cstime, utime, stime; +};
Well, this is ugly regardless.
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 18 lines · On Mon, 8 May 2006 09:40:10 -0700 Greg KH wrote:
Quoted 9 lines · Nishanth Aravamudan
Quoted 7 lines · Anne Thrax
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
Quoted 7 lines
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 2 lines
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
The DCO is about rights and licenses. Tracing the blame isn't mentioned.
---
~Randy
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 21 lines · Randy.Dunlap
Quoted 19 lines · On Mon, 8 May 2006 09:40:10 -0700 Greg KH wrote:
Quoted 17 lines · Nishanth Aravamudan
Quoted 7 lines · Anne Thrax
Quoted 5 lines
Signed-off-by, please.
And, I must ask, is your name actually Anne Thrax? If not, please use your real name.
I would rather not use my real name, as I am only 14 years old.
I'm not sure I entirely understand how the one stems from the other, but I think the spirit of the "personal information" part of the DCO (Developer's Certificate of Origin, in
Documentation/SubmittingPatches) is something like full disclosure. Also, I wonder how, if at all, the DCO can be enforced when it involves a minor. I'll defer to Greg on this one... at least on whatever precedent exists, I don't expect a legal decision, Greg :)
Quoted 5 lines
Quoted 2 lines
Doesn't matter, we still need a real name. It's not a legal issue but a "trace the blame" issue.
The DCO is about rights and licenses. Tracing the blame isn't mentioned.
So I guess there are two issues, both of which are still somewhat up in the air.
1) Can a minor, or more generally a person who does not divulge a real name, legitimately certify anything mentioned in the DCO? Specifically wrt. the validity of any rights or licenses.
2) Do we require a real name, or a well-known/verified pseudonym, for the Signed-off-by for keeping track of commits and "tracing the blame"?
A no to 1) or a yes to 2) are roughly equivalent for this particular case, but maybe only a lawyer can really answer 1)...
Thanks,
Nish
Signature · 2 lines
Nishanth Aravamudan <[email protected]> IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors