1
0
Fork 0
mirror of https://github.com/sockspls/badfish synced 2025-04-30 08:43:09 +00:00

Revert small state change optimization in idle_loop()

Joona says:

1. We should not be afraid of "AllThreadsShouldExit" flag.
Because when this is set to true we _must_not_ be searching (= All
splits must have been undone).
And if we are not searching it's impossible that some other thread
could give us work to do. So setting state to THREAD_AVAILABLE
doesn't do any harm. If you want to add check for this, you could do
it like this:

 if (threads[threadID].state == THREAD_WORKISWAITING)
 {
+    assert(!AllThreadsShouldExit)
     threads[threadID].state = THREAD_SEARCHING;

2a. If waitSp->cpus == 0, setting state to THREAD_AVAILABLE makes
no harm either, because helpful master concept dictates that _only_
our own slave can book us. If we don't have any slaves, noone has the
right to book us.

2b. If point (2a) is not correct then your extra check only adds extra race:
In smp code checking for waitSp->cpus > 0 is not enough. It's possible that
our slave immediately exits and another thread
books us as a slave when our state is still
THREAD_AVAILABLE. So instead of adding extra level of security we have
just introduced extra race.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This commit is contained in:
Marco Costalba 2010-02-20 18:36:07 +01:00
parent 512a4e4ff0
commit 8b99e94562

View file

@ -2650,6 +2650,8 @@ namespace {
// If this thread has been assigned work, launch a search
if (threads[threadID].state == THREAD_WORKISWAITING)
{
assert(!AllThreadsShouldExit);
threads[threadID].state = THREAD_SEARCHING;
if (threads[threadID].splitPoint->pvNode)
@ -2659,13 +2661,7 @@ namespace {
assert(threads[threadID].state == THREAD_SEARCHING);
// If this is a slave thread reset to available, instead
// if it is a master thread and all slaves have finished
// then leave as is to avoid booking by another master,
// we will leave idle loop shortly anyhow.
if ( !AllThreadsShouldExit
&& (!waitSp || waitSp->cpus > 0))
threads[threadID].state = THREAD_AVAILABLE;
threads[threadID].state = THREAD_AVAILABLE;
}
// If this thread is the master of a split point and all threads have