Commit bcd5cff7 authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Thomas Gleixner

cputimer: Cure lock inversion

There's a lock inversion between the cputimer->lock and rq->lock;
notably the two callchains involved are:

 update_rlimit_cpu()
   sighand->siglock
   set_process_cpu_timer()
     cpu_timer_sample_group()
       thread_group_cputimer()
         cputimer->lock
         thread_group_cputime()
           task_sched_runtime()
             ->pi_lock
             rq->lock

 scheduler_tick()
   rq->lock
   task_tick_fair()
     update_curr()
       account_group_exec()
         cputimer->lock

Where the first one is enabling a CLOCK_PROCESS_CPUTIME_ID timer, and
the second one is keeping up-to-date.

This problem was introduced by e8abccb7 ("posix-cpu-timers: Cure
SMP accounting oddities").

Cure the problem by removing the cputimer->lock and rq->lock nesting,
this leaves concurrent enablers doing duplicate work, but the time
wasted should be on the same order otherwise wasted spinning on the
lock and the greater-than assignment filter should ensure we preserve
monotonicity.
Reported-by: default avatarDave Jones <davej@redhat.com>
Reported-by: default avatarSimon Kirby <sim@hostway.ca>
Signed-off-by: default avatarPeter Zijlstra <a.p.zijlstra@chello.nl>
Cc: stable@kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/1318928713.21167.4.camel@twinsSigned-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
parent 899e3ee4
...@@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times) ...@@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
struct task_cputime sum; struct task_cputime sum;
unsigned long flags; unsigned long flags;
spin_lock_irqsave(&cputimer->lock, flags);
if (!cputimer->running) { if (!cputimer->running) {
cputimer->running = 1;
/* /*
* The POSIX timer interface allows for absolute time expiry * The POSIX timer interface allows for absolute time expiry
* values through the TIMER_ABSTIME flag, therefore we have * values through the TIMER_ABSTIME flag, therefore we have
...@@ -284,8 +282,11 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times) ...@@ -284,8 +282,11 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
* it. * it.
*/ */
thread_group_cputime(tsk, &sum); thread_group_cputime(tsk, &sum);
spin_lock_irqsave(&cputimer->lock, flags);
cputimer->running = 1;
update_gt_cputime(&cputimer->cputime, &sum); update_gt_cputime(&cputimer->cputime, &sum);
} } else
spin_lock_irqsave(&cputimer->lock, flags);
*times = cputimer->cputime; *times = cputimer->cputime;
spin_unlock_irqrestore(&cputimer->lock, flags); spin_unlock_irqrestore(&cputimer->lock, flags);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment