Messages in this thread | | | Date | Mon, 30 Nov 2009 15:54:43 +0100 | From | Stanislaw Gruszka <> | Subject | Re: [PATCH 2/2] cputime: introduce thread_group_times() |
| |
On Mon, Nov 30, 2009 at 06:21:36PM +0900, Hidetoshi Seto wrote: > This patch fixes the above problem in the following way: > > - Modify thread's exit (= __exit_signal()) not to use task_times(). > It means {u,s}time in signal struct accumulates raw values instead > of adjusted values. As the result it makes thread_group_cputime() > to return pure sum of "raw" values. > > - Introduce a new function thread_group_times(*task, *utime, *stime) > that converts "raw" values of thread_group_cputime() to "adjusted" > values, in same calculation procedure as task_times(). > > - Modify group's exit (= wait_task_zombie()) to use this introduced > thread_group_times(). It make c{u,s}time in signal struct to > have adjusted values like before this patch. > > - Replace some thread_group_cputime() by thread_group_times(). > This replacements are only applied where conveys the "adjusted" > cputime to users, and where already uses task_times() near by it. > (i.e. sys_times(), getrusage(), and /proc/<PID>/stat.) > > This patch have a positive side effect: > > - Before this patch, if a group contains many short-life threads > (e.g. runs 0.9ms and not interrupted by ticks), the group's > cputime could be invisible since thread's cputime was accumulated > after adjusted: imagine adjustment function as adj(ticks, runtime), > {adj(0, 0.9) + adj(0, 0.9) + ....} = {0 + 0 + ....} = 0. > After this patch it will not happen because the adjustment is > applied after accumulated.
Idea is very good IMHO.
> Signed-off-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com> > --- > fs/proc/array.c | 5 +--- > include/linux/sched.h | 3 +- > kernel/exit.c | 23 +++++++++++---------- > kernel/fork.c | 2 + > kernel/sched.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++- > kernel/sys.c | 18 +++++++--------- > 6 files changed, 75 insertions(+), 28 deletions(-)
[snip]
> #ifndef CONFIG_VIRT_CPU_ACCOUNTING > - cputime_t prev_utime, prev_stime; > + cputime_t prev_utime, prev_stime, prev_tgutime, prev_tgstime; > #endif
I think the new values should be part of struct_signal (see below)
> +static void __thread_group_times(struct task_struct *p, > + struct task_cputime *cputime) > +{ > + cputime_t rtime, utime, total; > + > + thread_group_cputime(p, cputime); > + > + total = cputime_add(cputime->utime, cputime->stime); > + rtime = nsecs_to_cputime(cputime->sum_exec_runtime); > + > + if (total) { > + u64 temp; > + > + temp = (u64)(rtime * cputime->utime); > + do_div(temp, total); > + utime = (cputime_t)temp; > + } else > + utime = rtime; > + > + p->prev_tgutime = max(p->prev_tgutime, utime); > + p->prev_tgstime = max(p->prev_tgstime, > + cputime_sub(rtime, p->prev_tgutime));
p->sig->prev_utime = max(p->sig->prev_utime, utime); p->sig->prev_stime = max(p->sig->prev_stime, cputime_sub(rtime, p->sig->prev_utime));
> /* > + * Must be called with siglock held. > + */ > +void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *st) > +{ > + struct task_cputime cputime; > + > + __thread_group_times(p, &cputime); > + > + if (ut) > + *ut = cputime.utime; > + if (st) > + *st = cputime.stime;
No thread_group_times() nor task_times() is called with NULL arguments, we can get rid of "if ({u,s}t)" checks. Perhaps thread_group_times() should have "struct task_cputime" argument as it is wrapper for thread_group_cputime();
Thanks Stanislaw
| |