aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNick Piggin <npiggin@suse.de>2009-12-15 19:47:30 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2009-12-16 10:20:09 -0500
commitd4212093dca95c1f52197017d969cc66d5d962aa (patch)
treef52484ae3c7b3034715b5745ae2b7db3906ab022
parent9cad200c7686708b326520a45dd680a4147568a6 (diff)
ipc/sem.c: sem preempt improve
The strange sysv semaphore wakeup scheme has a kind of busy-wait lock involved, which could deadlock if preemption is enabled during the "lock". It is an implementation detail (due to a spinlock being held) that this is actually the case. However if "spinlocks" are made preemptible, or if the sem lock is changed to a sleeping lock for example, then the wakeup would become buggy. So this might be a bugfix for -rt kernels. Imagine waker being preempted by wakee and never clearing IN_WAKEUP -- if wakee has higher RT priority then there is a priority inversion deadlock. Even if there is not a priority inversion to cause a deadlock, then there is still time wasted spinning. Signed-off-by: Nick Piggin <npiggin@suse.de> Signed-off-by: Manfred Spraul <manfred@colorfullife.com> Cc: Pierre Peiffer <peifferp@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--ipc/sem.c38
1 files changed, 23 insertions, 15 deletions
diff --git a/ipc/sem.c b/ipc/sem.c
index d377b3adfc3d..2705fbbc4374 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -398,6 +398,27 @@ undo:
398 return result; 398 return result;
399} 399}
400 400
401/*
402 * Wake up a process waiting on the sem queue with a given error.
403 * The queue is invalid (may not be accessed) after the function returns.
404 */
405static void wake_up_sem_queue(struct sem_queue *q, int error)
406{
407 /*
408 * Hold preempt off so that we don't get preempted and have the
409 * wakee busy-wait until we're scheduled back on. We're holding
410 * locks here so it may not strictly be needed, however if the
411 * locks become preemptible then this prevents such a problem.
412 */
413 preempt_disable();
414 q->status = IN_WAKEUP;
415 wake_up_process(q->sleeper);
416 /* hands-off: q can disappear immediately after writing q->status. */
417 smp_wmb();
418 q->status = error;
419 preempt_enable();
420}
421
401/* Go through the pending queue for the indicated semaphore 422/* Go through the pending queue for the indicated semaphore
402 * looking for tasks that can be completed. 423 * looking for tasks that can be completed.
403 */ 424 */
@@ -429,17 +450,7 @@ again:
429 * continue. 450 * continue.
430 */ 451 */
431 alter = q->alter; 452 alter = q->alter;
432 453 wake_up_sem_queue(q, error);
433 /* wake up the waiting thread */
434 q->status = IN_WAKEUP;
435
436 wake_up_process(q->sleeper);
437 /* hands-off: q will disappear immediately after
438 * writing q->status.
439 */
440 smp_wmb();
441 q->status = error;
442
443 if (alter) 454 if (alter)
444 goto again; 455 goto again;
445 } 456 }
@@ -523,10 +534,7 @@ static void freeary(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
523 list_for_each_entry_safe(q, tq, &sma->sem_pending, list) { 534 list_for_each_entry_safe(q, tq, &sma->sem_pending, list) {
524 list_del(&q->list); 535 list_del(&q->list);
525 536
526 q->status = IN_WAKEUP; 537 wake_up_sem_queue(q, -EIDRM);
527 wake_up_process(q->sleeper); /* doesn't sleep */
528 smp_wmb();
529 q->status = -EIDRM; /* hands-off q */
530 } 538 }
531 539
532 /* Remove the semaphore set from the IDR */ 540 /* Remove the semaphore set from the IDR */