rtmutex: Simplify PI code

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 05:47 PM, in message <[email protected]>,

From: Steven Rostedt <[email protected]>

The commit: rtmutex: Optimize rt lock wakeup

Does not do what it was suppose to do.
This is because the adaptive waiter sets its state to TASK_(UN)INTERRUPTIBLE before going into the loop. Thus, the test in wakeup_next_waiter() will always fail on an adaptive waiter, as it only tests to see if the pending waiter never has its state set ot TASK_RUNNING unless something else had woke it up.

The smp_mb() added to make this test work is just as expensive as just calling wakeup. And since we we fail to wake up anyway, we are doing both a smp_mb() and wakeup as well.

I tested this with dbench and we run faster without this patch. I also tried a variant that instead fixed the loop, to change the state only if the spinner was to go to sleep, and that still did not show any improvement.

Cc Gregory Haskins <[email protected]>

Cc Peter Morreale <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/rtmutex.c |   29 ++---------------------------
 1 files changed, 2 insertions(+), 27 deletions(-)

kernel/rtmutex.c +2 -27

index 318d7ed..e218873 100644
@@ -554,33 +554,8 @@ static void wakeup_next_waiter(struct rt_mutex *lock, 
 int savestate)
 	 */
 	if (!savestate)
 		wake_up_process(pendowner);
-	else {
-		/*
-		 * We can skip the actual (expensive) wakeup if the
-		 * waiter is already running, but we have to be careful
-		 * of race conditions because they may be about to sleep.
-		 *
-		 * The waiter-side protocol has the following pattern:
-		 * 1: Set state != RUNNING
-		 * 2: Conditionally sleep if waiter->task != NULL;
-		 *
-		 * And the owner-side has the following:
-		 * A: Set waiter->task = NULL
-		 * B: Conditionally wake if the state != RUNNING
-		 *
-		 * As long as we ensure 1->2 order, and A->B order, we
-		 * will never miss a wakeup.
-		 *
-		 * Therefore, this barrier ensures that waiter->task = NULL
-		 * is visible before we test the pendowner->state.  The
-		 * corresponding barrier is in the sleep logic.
-		 */
-		smp_mb();
-
-		/* If !RUNNING && !RUNNING_MUTEX */
-		if (pendowner->state & ~TASK_RUNNING_MUTEX)
-			wake_up_process_mutex(pendowner);
-	}
+	else
+		wake_up_process_mutex(pendowner);
 
 	rt_mutex_set_owner(lock, pendowner, RT_MUTEX_OWNER_PENDING);

Hey Steve,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 05:47 PM, in message <[email protected]>,

Quoted 15 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

The commit: rtmutex: Optimize rt lock wakeup

Does not do what it was suppose to do.
This is because the adaptive waiter sets its state to TASK_(UN)INTERRUPTIBLE before going into the loop. Thus, the test in wakeup_next_waiter() will always fail on an adaptive waiter, as it only tests to see if the pending waiter never has its state set ot TASK_RUNNING unless something else had woke it up.

The smp_mb() added to make this test work is just as expensive as just calling wakeup. And since we we fail to wake up anyway, we are doing both a smp_mb() and wakeup as well.

Quoted 4 lines

I tested this with dbench and we run faster without this patch. I also tried a variant that instead fixed the loop, to change the state only if the spinner was to go to sleep, and that still did not show any improvement.

Just a quick note to say I am a bit skeptical of this patch. I know you are offline next week, so lets plan on hashing it out after the new year before I ack it.

Happy holidays!

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 11:54 PM, in message

Quoted 10 lines · Gregory Haskins

Hey Steve,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 05:47 PM, in message <[email protected]>,

Quoted 6 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

The commit: rtmutex: Optimize rt lock wakeup

Does not do what it was suppose to do.
This is because the adaptive waiter sets its state to

TASK_(UN)INTERRUPTIBLE

Quoted 14 lines

Quoted 13 lines

before going into the loop. Thus, the test in wakeup_next_waiter() will always fail on an adaptive waiter, as it only tests to see if the pending waiter never has its state set ot TASK_RUNNING unless something else had woke it up.

The smp_mb() added to make this test work is just as expensive as just calling wakeup. And since we we fail to wake up anyway, we are doing both a smp_mb() and wakeup as well.

I tested this with dbench and we run faster without this patch. I also tried a variant that instead fixed the loop, to change the state only if the spinner was to go to sleep, and that still did not show any improvement.

Quoted 1 line

Just a quick note to say I am a bit skeptical of this patch. I know you are

offline next week, so lets plan on hashing it out after the new year before I ack it.

Sure, but as I said, it is mostly broken anyway. I could even insert some tracepoints to show that this is always missed (heck I'll add an unlikely and do the branch profiler ;-)

The reason is that adaptive spinners spin in some other state than TASK_RUNNING, thus it does not help adaptive spinners at all. I first tried to fix that, but it made dbench run even slower.

Quoted 5 lines

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 11:54 PM, in message

Sure, but as I said, it is mostly broken anyway. I could even insert some tracepoints to show that this is always missed (heck I'll add an unlikely and do the branch profiler ;-)

Well, I think that would be a good datapoint and is one of the things I'd like to see.

Quoted 5 lines

The reason is that adaptive spinners spin in some other state than TASK_RUNNING, thus it does not help adaptive spinners at all. I first tried to fix that, but it made dbench run even slower.

This is why I am skeptical. You are essentially asserting there are two issues here, IIUC:

1) The intent of avoiding a wakeup is broken and we take the double whammy of a mb() plus the wakeup() anyway.

2) mb() is apparently slower than wakeup().

I agree (1) is plausible, though I would like to see the traces to confirm. Its been a long time since I looked at that code, but I think the original code either ran in RUNNING_MUTEX and was inadvertently broken in the mean time or the other cpu would have transitioned to RUNNING on its own when we flipped the owner before the release-side check was performed. Or perhaps we just plain screwed this up and it was racy ;) I'm not sure. But as Peter (M) stated, it seems like a shame to walk away from the concept without further investigation. I think everyone can agree that at the very least, if it is in fact taking a double whammy we should fix that.

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)


		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);

		if (sleep)
			schedule_rt_mutex(lock);
		saved_state = rt_restore_current_blocked_state(saved_state);
	}

Otherwise we can risk the wakeup_next_waiter() missing the wakeup.

To clarify, we want the adaptive_wait() to run as TASK_RUNNING. Then if we must sleep, then we must set the state to TASK_UNINTERRUPTIBLE, test again if we can still the lock, and if not then sleep. Otherwise, if a wakeup happens just before we set the state to TASK_UNINTERRUPTIBLE, then we miss the wake up all together.

I can do this change, and see what impact it makes.

I'm also curious if this ever worked? If it did not, then are you sure your tests that show the benefit of it was true. I don't have a large scale box at my disposal ATM, so I can only see what this does on 4way machines.

-- Steve

Quoted 14 lines · Steven Rostedt

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)


		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);

I may be able to remove the above locks and replace it with:

	saved_state = rt_set_current_blocked_state(saved_state);
	if (orig_owner == rt_mutex_owner(lock))
		schedule_rt_mutex(lock);

-- Steve

Quoted 24 lines

		if (sleep)
			schedule_rt_mutex(lock);
		saved_state = rt_restore_current_blocked_state(saved_state);
	}

Otherwise we can risk the wakeup_next_waiter() missing the wakeup.

To clarify, we want the adaptive_wait() to run as TASK_RUNNING. Then if we must sleep, then we must set the state to TASK_UNINTERRUPTIBLE, test again if we can still the lock, and if not then sleep. Otherwise, if a wakeup happens just before we set the state to TASK_UNINTERRUPTIBLE, then we miss the wake up all together.

I can do this change, and see what impact it makes.

I'm also curious if this ever worked? If it did not, then are you sure your tests that show the benefit of it was true. I don't have a large scale box at my disposal ATM, so I can only see what this does on 4way machines.

-- Steve

Quoted 19 lines · Steven Rostedt

Quoted 14 lines · Steven Rostedt

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)


		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);

I may be able to remove the above locks and replace it with:

Quoted 5 lines

	saved_state = rt_set_current_blocked_state(saved_state);
	if (orig_owner == rt_mutex_owner(lock))
		schedule_rt_mutex(lock);

-- Steve

Isn't it possible to miss a wakeup here if the waiter becomes preempted?

Recall that adaptive wait is a preemptive wait. Hence the (I believe) original reason we did the adaptive spin in a (transitioning) sleep state.

Quoted 20 lines · Peter W. Morreale

Quoted 19 lines · Steven Rostedt

Quoted 14 lines · Steven Rostedt

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)


		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);

I may be able to remove the above locks and replace it with:

Quoted 7 lines

Quoted 5 lines

	saved_state = rt_set_current_blocked_state(saved_state);
	if (orig_owner == rt_mutex_owner(lock))
		schedule_rt_mutex(lock);

-- Steve

Isn't it possible to miss a wakeup here if the waiter becomes preempted?

Why? Preemption doesn't change the task state.

Quoted 4 lines

Recall that adaptive wait is a preemptive wait. Hence the (I believe) original reason we did the adaptive spin in a (transitioning) sleep state.

Yes it is a preemptive wait, I would not have accepted the patches if it was anything else. But preemption is not affected by the state of the task. A task could be TASK_RUNNING or TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE, and that would not affect how it acts when it is preempted.

-- Steve

My bad. I thought preemption did change task state.

This still requires the owner to run through try_to_wake_up() and all its associated overhead only to find out that the waiter is running.

The assumption I made when I suggested the original concept to Greg was that if the new owner is running, there is *nothing* to do wrt scheduling. If that was a wrong assumption, then, yes, drop the patch and clean things up.

If that was a good assumption, then we are leaving 'cycles on the table' as waking up a running process is a non-zero-overhead path and that is a bad thing considering how many times spin_unlock() is called on an rt system.

Bear in mind that this savings scales directly as the number of CPUs (assuming all are vectored on the lock). We can only have nr_cpus-1 spinning waiters at any given time, regardless of the number of tasks in contention. Perhaps this is too little to worry about on a 4way system, but I suspect that it could be substantial on larger systems.

I'll be quiet now as I know little about the intricacies of preemption/scheduling (obviously) and like Greg, have been removed from RT kernel work for several years. <sigh>

Quoted 21 lines · Peter W. Morreale

My bad. I thought preemption did change task state.

This still requires the owner to run through try_to_wake_up() and all its associated overhead only to find out that the waiter is running.

The assumption I made when I suggested the original concept to Greg was that if the new owner is running, there is *nothing* to do wrt scheduling. If that was a wrong assumption, then, yes, drop the patch and clean things up.

If that was a good assumption, then we are leaving 'cycles on the table' as waking up a running process is a non-zero-overhead path and that is a bad thing considering how many times spin_unlock() is called on an rt system.

Bear in mind that this savings scales directly as the number of CPUs (assuming all are vectored on the lock). We can only have nr_cpus-1 spinning waiters at any given time, regardless of the number of tasks in contention. Perhaps this is too little to worry about on a 4way system, but I suspect that it could be substantial on larger systems.

Quoted 3 lines

I'll be quiet now as I know little about the intricacies of preemption/scheduling (obviously) and like Greg, have been removed from RT kernel work for several years. <sigh>

No need to be quiet ;-)

I'm working on making it spin in TASK_RUNNING state if possible, but it is making the code a bit more complex, as it seems that there is an assumption with the wakeup and the changing of the current->state in the rt_spin_lock_slowlock code all being under the lock->wait_lock. I think I'll scrap this idea.

That said, I think your wakeup patch may be worth while with Lai's new code. His changes causes the owner to wake up the pending owner several times, because the pending owner is never removed from the lock wait_list. If a high prio task grabs and releases the same lock over and over, if there is a waiter it will try to wake up that waiter each time.

Thus, having your patch may prevent that unnecessary wakeup.

I'll look more into it. Thanks!

-- Steve

Quoted 6 lines · Steven Rostedt

That said, I think your wakeup patch may be worth while with Lai's new code. His changes causes the owner to wake up the pending owner several times, because the pending owner is never removed from the lock wait_list. If a high prio task grabs and releases the same lock over and over, if there is a waiter it will try to wake up that waiter each time.

Oooohhhh I wonder if that would enable 'lock-stealing' for FIFO?

IIRC, you came up with a ping-pong scenario that prevented its use there.

Quoted 11 lines · Peter W. Morreale

Quoted 6 lines · Steven Rostedt

That said, I think your wakeup patch may be worth while with Lai's new code. His changes causes the owner to wake up the pending owner several times, because the pending owner is never removed from the lock wait_list. If a high prio task grabs and releases the same lock over and over, if there is a waiter it will try to wake up that waiter each time.

Oooohhhh I wonder if that would enable 'lock-stealing' for FIFO?

I guess you mean literal FIFO order and not SCHED_FIFO.

But, if I understand you, it would allow for this. If a process has a lock and two processes are blocked on it. When the first releases the lock it gives it to the higher of the waiters. But if a high prio task comes a long, it will steal that lock away, and this woken task will need to race to get it from the other lower prio task that comes along. And yes, if that other task is the same prio, we reverse the FIFO order here.

Lai's patch prevents this. Either the woken task gets the lock or a higher prio task does. No mixing around the order of taking the lock here. The better side effect of Lai's patch is that it cleans up the code quite a bit, which is what I really want, as we start to introduce SCHED_DEADLINE.

Quoted 3 lines

IIRC, you came up with a ping-pong scenario that prevented its use there.

Probably.

-- Steve

Quoted 20 lines · Peter W. Morreale

Quoted 19 lines · Steven Rostedt

Quoted 14 lines · Steven Rostedt

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)


		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);

I may be able to remove the above locks and replace it with:

Quoted 7 lines

Quoted 5 lines

	saved_state = rt_set_current_blocked_state(saved_state);
	if (orig_owner == rt_mutex_owner(lock))
		schedule_rt_mutex(lock);

-- Steve

Isn't it possible to miss a wakeup here if the waiter becomes preempted?

Why? Preemption doesn't change the task state.

Quoted 4 lines

Recall that adaptive wait is a preemptive wait. Hence the (I believe) original reason we did the adaptive spin in a (transitioning) sleep state.

Yes it is a preemptive wait, I would not have accepted the patches if it was anything else. But preemption is not affected by the state of the task. A task could be TASK_RUNNING or TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE, and that would not affect how it acts when it is preempted.

-- Steve

Quoted 1 line

Quoted 1 line

On 1/3/2011 at 02:06 PM, in message

Quoted 7 lines · Gregory Haskins

Quoted 5 lines

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 11:54 PM, in message

Sure, but as I said, it is mostly broken anyway. I could even insert some tracepoints to show that this is always missed (heck I'll add an unlikely and do the branch profiler ;-)

Well, I think that would be a good datapoint and is one of the things I'd

like to see.

OK, here it is, after running a simple "dbench 10":

correct incorrect % Function File
Line
------- --------- - -------- ---- ----
150 726979 99 wakeup_next_waiter rtmutex.c
581

Interesting that we hit 150 times that the new owner was already awake. Perhaps it was because the new owner was about to spin on a spinlock after it had put itself into the TASK_INTERRUPTIBLE state, and it was woken up by someone else?

Quoted 9 lines

Quoted 7 lines

The reason is that adaptive spinners spin in some other state than TASK_RUNNING, thus it does not help adaptive spinners at all. I first tried to fix that, but it made dbench run even slower.

This is why I am skeptical. You are essentially asserting there are two

issues here, IIUC:

Quoted 2 lines

1) The intent of avoiding a wakeup is broken and we take the double whammy

of a mb()

Quoted 1 line

plus the wakeup() anyway.

Yep, this is what I believe is happening.

Quoted 2 lines

2) mb() is apparently slower than wakeup().

Forget this point, as #1 seems to be the main issue. Also, I'm not sure it is safe to "fix" this, as I started to try.

Quoted 4 lines

I agree (1) is plausible, though I would like to see the traces to confirm.

Its been a long time

Quoted 1 line

since I looked at that code, but I think the original code either ran in

RUNNING_MUTEX and was

Quoted 1 line

inadvertently broken in the mean time or the other cpu would have

transitioned to RUNNING on

Quoted 1 line

its own when we flipped the owner before the release-side check was

performed. Or perhaps

Quoted 1 line

we just plain screwed this up and it was racy ;) I'm not sure. But as

Peter (M) stated, it seems

Quoted 1 line

like a shame to walk away from the concept without further investigation. I

think everyone can

Quoted 1 line

agree that at the very least, if it is in fact taking a double whammy we

should fix that.

[ cut out all the 2's since I'm not worried about that now, even if it
is not a problem. ]

Now, the way I was going to fix this is to have the adaptive wait be in TASK_RUNNING state, and then change the state iff we are going to sleep.

But this can be a bit more race. Especially with Lai's new changes. With the new changes, the waiter->task does not get nulled anymore.

The test to take the lock, now needs to be under the lock->wait_lock spinlock. We have to grab that lock, and see if the owner is null, and that we are the next waiter in the queue. Thus, on taking the lock we need to have something like this:

	if (adaptive_wait(&waiter, orig_owner))
		sleep = 1;
	else
		sleep = 0;

	if (sleep)
		raw_spin_lock(&lock->wait_lock);
		saved_state = rt_set_current_block_state(saved_state);
		if (!lock->owner && &waiter == rt_mutex_top_waiter(lock))
			sleep = 0;
		raw_spin_unlock(&lock->wait_lock);
		if (sleep)
			schedule_rt_mutex(lock);
		saved_state = rt_restore_current_blocked_state(saved_state);
	}

Otherwise we can risk the wakeup_next_waiter() missing the wakeup.

To clarify, we want the adaptive_wait() to run as TASK_RUNNING. Then if we must sleep, then we must set the state to TASK_UNINTERRUPTIBLE, test again if we can still the lock, and if not then sleep. Otherwise, if a wakeup happens just before we set the state to TASK_UNINTERRUPTIBLE, then we miss the wake up all together.

I can do this change, and see what impact it makes.

I'm also curious if this ever worked?

If it did not, then are you sure
your tests that show the benefit of it was true. I don't have a large scale box at my disposal ATM, so I can only see what this does on 4way machines.

Hey Steve,

Quoted 1 line

Quoted 1 line

Quoted 1 line

On 12/23/2010 at 05:47 PM, in message <[email protected]>,

Quoted 15 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>

The commit: rtmutex: Optimize rt lock wakeup

Does not do what it was suppose to do.
This is because the adaptive waiter sets its state to TASK_(UN)INTERRUPTIBLE before going into the loop. Thus, the test in wakeup_next_waiter() will always fail on an adaptive waiter, as it only tests to see if the pending waiter never has its state set ot TASK_RUNNING unless something else had woke it up.

The smp_mb() added to make this test work is just as expensive as just calling wakeup. And since we we fail to wake up anyway, we are doing both a smp_mb() and wakeup as well.

Quoted 4 lines

I tested this with dbench and we run faster without this patch. I also tried a variant that instead fixed the loop, to change the state only if the spinner was to go to sleep, and that still did not show any improvement.

Just a quick note to say I am a bit skeptical of this patch. I know you are offline next week, so lets plan on hashing it out after the new year before I ack it.

Happy holidays!
-Greg

Quoted 49 lines

Cc Gregory Haskins <[email protected]>

Cc Peter Morreale <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/rtmutex.c |   29 ++---------------------------
 1 files changed, 2 insertions(+), 27 deletions(-)

kernel/rtmutex.c +2 -27

index 318d7ed..e218873 100644
@@ -554,33 +554,8 @@ static void wakeup_next_waiter(struct rt_mutex *lock, 
 int savestate)
 	 */
 	if (!savestate)
 		wake_up_process(pendowner);
-	else {
-		/*
-		 * We can skip the actual (expensive) wakeup if the
-		 * waiter is already running, but we have to be careful
-		 * of race conditions because they may be about to sleep.
-		 *
-		 * The waiter-side protocol has the following pattern:
-		 * 1: Set state != RUNNING
-		 * 2: Conditionally sleep if waiter->task != NULL;
-		 *
-		 * And the owner-side has the following:
-		 * A: Set waiter->task = NULL
-		 * B: Conditionally wake if the state != RUNNING
-		 *
-		 * As long as we ensure 1->2 order, and A->B order, we
-		 * will never miss a wakeup.
-		 *
-		 * Therefore, this barrier ensures that waiter->task = NULL
-		 * is visible before we test the pendowner->state.  The
-		 * corresponding barrier is in the sleep logic.
-		 */
-		smp_mb();
-
-		/* If !RUNNING && !RUNNING_MUTEX */
-		if (pendowner->state & ~TASK_RUNNING_MUTEX)
-			wake_up_process_mutex(pendowner);
-	}
+	else
+		wake_up_process_mutex(pendowner);
 
 	rt_mutex_set_owner(lock, pendowner, RT_MUTEX_OWNER_PENDING);

plain text document attachment (0004-rtmutex-Ensure-only-the-top-waiter-or-higher-priorit.patch)
From: Lai Jiangshan <[email protected]>

In current rtmutex, the pending owner may be boosted by the tasks in the rtmutex's waitlist when the pending owner is deboosted or a task in the waitlist is boosted. This boosting is unrelated, because the pending owner does not really take the rtmutex. It is not reasonable.

Quoted 10 lines · Steven Rostedt

plain text document attachment (0004-rtmutex-Ensure-only-the-top-waiter-or-higher-priorit.patch)
From: Lai Jiangshan <[email protected]>

In current rtmutex, the pending owner may be boosted by the tasks in the rtmutex's waitlist when the pending owner is deboosted or a task in the waitlist is boosted. This boosting is unrelated, because the pending owner does not really take the rtmutex. It is not reasonable.

I'm still hitting some bugs with the port to -rt, but I also noticed something that doesn't look too good.

There's several places in the kernel where a task may release and acquire the same lock multiple times in a row.

The old way of removing the pending owner from the lists and waking it up once, would have the high prio task wake it up once, and then it can grab the locks multiple times without modifying the list, since the pending owner is already awake and not in the pi list anymore.

The new way has the owner remove the woken task from its pi list and wakes it up, but when it steals the lock again, it adds this owner back to its pi list. When it releases the lock, it wakes it up again and removes it from its pi list again. This happens over and over again.