Messages in this thread | | | Date | Mon, 2 Mar 1998 17:38:06 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: [PATCH] 2.1.88 Hanging Processes (Uninterruptible Sleep) |
| |
On Tue, 3 Mar 1998, MOLNAR Ingo wrote: > > solution could be to put the state setting within add_waitqueue and > remove_waitqueue operations? Maybe as a third parameter?
No, that's why you have "sleep()".
The real problem is that you need to set yourself to sleep a lot more often than you need to do an "add_wait_queue()" - so usually this loop is run completely without any add_wait_queue() in the loop itself because it has been factored to outside the loop.
But because it has been factored outside the loop, the loop now has fewer "synchronization points" - ie atomic isntructions (or cpuid). So now the PPro/PentiumII starts to re-order instructions inside the queue a lot more aggressively, so it might re-order the read of a status variable to be before the "current->state = TASK_UNINTERRUPTIBLE" - so now we have the _old_ knowledge of the thing being locked, and we're going to sleep on that.
So you have code that looks like
repeat: current->state = TASK_UNINTERRUPTIBLE; if (empty) { schedule(); goto repeat; }
but is actually executed as:
CPU #0 CPU #1
repeat: if (empty) empty = 0; tsk->state = TASK_RUNNING;
tsk->state = TASK_UNINTERRUPTIBLE; schedule()
because CPU #0 has done the (locally legal) optimization of doing the empty test before setting the task state.
The CPUID patch inserts a CPUID instruction in between setting the state and testing the test-thing, which due to intel rules force the two to be done in the correct order, and then everything works ok.
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu
| |