lkml.org 
[lkml]   [2008]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2.6.27-rc5 re-resubmit] Fix itimer/many thread hang.
On 09/23, Ingo Molnar wrote:
>
> below is the delta patch i've applied to tip/timers/posixtimers - might
> be easier to review for those who've seen v1 and who'd like to check the
> changes.
> ...
>
> +unsigned long long task_delta_exec(struct task_struct *p)
> {
> + struct rq *rq;
> + unsigned long flags;
> + u64 ns = 0;
> +
> + rq = task_rq_lock(p, &flags);
> if (task_current(rq, p)) {
> u64 delta_exec;
>
> update_rq_clock(rq);
> delta_exec = rq->clock - p->se.exec_start;
> if ((s64)delta_exec > 0)
> - return delta_exec;
> + ns = delta_exec;
> }
> - return 0;
> -}
> -
> -/*
> - * Return p->sum_exec_runtime plus any more ns on the sched_clock
> - * that have not yet been banked in case the task is currently running.
> - */
> -unsigned long long task_sched_runtime(struct task_struct *p)
> -{
> - unsigned long flags;
> - u64 ns;
> - struct rq *rq;
> -
> - rq = task_rq_lock(p, &flags);
> - ns = p->se.sum_exec_runtime + task_delta_exec(p, rq);
> - task_rq_unlock(rq, &flags);
> -
> - return ns;
> -}
> -
> -/*
> - * Return sum_exec_runtime for the thread group plus any more ns on the
> - * sched_clock that have not yet been banked in case the task is currently
> - * running.
> - */
> -unsigned long long thread_group_sched_runtime(struct task_struct *p)
> -{
> - unsigned long flags;
> - u64 ns;
> - struct rq *rq;
> - struct task_cputime totals;
> -
> - rq = task_rq_lock(p, &flags);
> - thread_group_cputime(p, &totals);
> - ns = totals.sum_exec_runtime + task_delta_exec(p, rq);
> - task_rq_unlock(rq, &flags);
>
> return ns;

Looks like task_delta_exec() forgets to unlock rq.

> +static inline void thread_group_cputime_account_exec_runtime(
> + struct thread_group_cputime *tgtimes,
> + unsigned long long ns)
> +{
> + if (tgtimes->totals) {
> + struct task_cputime *times;
> +
> + times = per_cpu_ptr(tgtimes->totals, get_cpu());
> + times->sum_exec_runtime += ns;
> + put_cpu_no_resched();
> + }
> +}
> +
> +#else /* CONFIG_SMP */
> +
> +static inline void thread_group_cputime_account_user(
> + struct thread_group_cputime *tgtimes,
> + cputime_t cputime)
> +{
> + tgtimes->totals->utime = cputime_add(tgtimes->totals->utime, cputime);
> +}

Confused. Unless I misread the patch, tgtimes->totals can be NULL
on UP too?

Do we really need the special version for !CONFIG_SMP ? per_cpu_ptr()
just returns the pointer. Yes, get_cpu() disables preemption, but
perhaps this is tolerable?


(of course, this can't explain the crash)

Oleg.



\
 
 \ /
  Last update: 2008-09-23 16:07    [W:0.142 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site