diff options
| author | Peter Zijlstra <a.p.zijlstra@chello.nl> | 2011-10-17 05:50:30 -0400 |
|---|---|---|
| committer | Thomas Gleixner <tglx@linutronix.de> | 2011-10-18 05:36:59 -0400 |
| commit | bcd5cff7216f9b2de0a148cc355eac199dc6f1cf (patch) | |
| tree | 384674b2b0e16e489f591148982046bf2d25608b /kernel | |
| parent | 899e3ee404961a90b828ad527573aaaac39f0ab1 (diff) | |
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 e8abccb7193 ("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: Dave Jones <davej@redhat.com>
Reported-by: Simon Kirby <sim@hostway.ca>
Signed-off-by: Peter 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@twins
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/posix-cpu-timers.c | 7 |
1 files changed, 4 insertions, 3 deletions
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index c8008dd58ef2..640ded8f5c48 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c | |||
| @@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times) | |||
| 274 | struct task_cputime sum; | 274 | struct task_cputime sum; |
| 275 | unsigned long flags; | 275 | unsigned long flags; |
| 276 | 276 | ||
| 277 | spin_lock_irqsave(&cputimer->lock, flags); | ||
| 278 | if (!cputimer->running) { | 277 | if (!cputimer->running) { |
| 279 | cputimer->running = 1; | ||
| 280 | /* | 278 | /* |
| 281 | * The POSIX timer interface allows for absolute time expiry | 279 | * The POSIX timer interface allows for absolute time expiry |
| 282 | * values through the TIMER_ABSTIME flag, therefore we have | 280 | * 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 | * it. | 282 | * it. |
| 285 | */ | 283 | */ |
| 286 | thread_group_cputime(tsk, &sum); | 284 | thread_group_cputime(tsk, &sum); |
| 285 | spin_lock_irqsave(&cputimer->lock, flags); | ||
| 286 | cputimer->running = 1; | ||
| 287 | update_gt_cputime(&cputimer->cputime, &sum); | 287 | update_gt_cputime(&cputimer->cputime, &sum); |
| 288 | } | 288 | } else |
| 289 | spin_lock_irqsave(&cputimer->lock, flags); | ||
| 289 | *times = cputimer->cputime; | 290 | *times = cputimer->cputime; |
| 290 | spin_unlock_irqrestore(&cputimer->lock, flags); | 291 | spin_unlock_irqrestore(&cputimer->lock, flags); |
| 291 | } | 292 | } |
