Messages in this thread | | | Date | Fri, 28 Mar 2008 11:28:44 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 2.6.25-rc6] Fix itimer/many thread hang. |
| |
* Frank Mayhar <fmayhar@google.com> wrote:
> +static inline void thread_group_times_init(struct signal_struct *sig) > +{ > + sig->thread_group_times = NULL; > +} > + > +static inline void thread_group_times_free(struct signal_struct *sig) > +{ > + if (sig->thread_group_times) > + free_percpu(sig->thread_group_times); > +} > + > +/* > + * Allocate the thread_group_cputime struct appropriately and fill in the current > + * values of the fields. Called from do_setitimer() when setting an interval > + * timer (ITIMER_PROF or ITIMER_VIRTUAL). Assumes interrupts are enabled when > + * it's called. Note that there is no corresponding deallocation done from > + * do_setitimer(); the structure is freed at process exit. > + */ > +static inline int thread_group_times_alloc(struct task_struct *tsk) > +{ > + struct signal_struct *sig = tsk->signal; > + struct thread_group_cputime *thread_group_times; > + struct task_struct *t; > + cputime_t utime, stime; > + unsigned long long sum_exec_runtime; > + > + /* > + * If we don't already have a thread_group_cputime struct, allocate > + * one and fill it in with the accumulated times. > + */ > + if (sig->thread_group_times) > + return 0; > + thread_group_times = alloc_percpu(struct thread_group_cputime); > + if (thread_group_times == NULL) > + return -ENOMEM; > + read_lock(&tasklist_lock); > + spin_lock_irq(&tsk->sighand->siglock); > + if (sig->thread_group_times) { > + spin_unlock_irq(&tsk->sighand->siglock); > + read_unlock(&tasklist_lock); > + free_percpu(thread_group_times); > + return 0; > + } > + sig->thread_group_times = thread_group_times; > + utime = sig->utime; > + stime = sig->stime; > + sum_exec_runtime = tsk->se.sum_exec_runtime; > + t = tsk; > + do { > + utime = cputime_add(utime, t->utime); > + stime = cputime_add(stime, t->stime); > + sum_exec_runtime += t->se.sum_exec_runtime; > + } while_each_thread(tsk, t); > + thread_group_times = per_cpu_ptr(sig->thread_group_times, get_cpu()); > + thread_group_times->utime = utime; > + thread_group_times->stime = stime; > + thread_group_times->sum_exec_runtime = sum_exec_runtime; > + put_cpu_no_resched(); > + spin_unlock_irq(&tsk->sighand->siglock); > + read_unlock(&tasklist_lock); > + return 0; > +}
please dont put such a huge inline into a header.
> + > +#define thread_group_update(sig, field, val, op) ({ \ > + if (sig && sig->thread_group_times) { \ > + int cpu; \ > + struct thread_group_cputime *thread_group_times; \ > + \ > + cpu = get_cpu(); \ > + thread_group_times = \ > + per_cpu_ptr(sig->thread_group_times, cpu); \ > + thread_group_times->field = \ > + op(thread_group_times->field, val); \ > + put_cpu_no_resched(); \ > + } \ > +})
nor use any macros that includes code.
> +static inline int thread_group_cputime(struct thread_group_cputime *thread_group_times, > + struct signal_struct *sig)
ditto, line length as well.
> + if (!sig->thread_group_times) > + return(0);
return 0 is the proper form - please run your patch through scripts/checkpatch.pl.
Ingo
| |