Re: [PATCH v0.9.1 3/6] sched/umcg: implement UMCG syscalls

On Mon, Nov 29, 2021 at 10:08:41PM +0100, Peter Zijlstra wrote:

I'm not sure I'm following. For this to be true A and C must be running
on a different server right?

So we have something like:

	S0 running A			S1 running B

Therefore:

	S0::state == RUNNABLE		S1::state == RUNNABLE
	A::server_tid == S0.tid		B::server_tid == S1.tid
	A::state == RUNNING		B::state == RUNNING

Now, you want A to switch to C, therefore C had better be with S0, eg we
have:

	C::server_tid == S0.tid
	C::state == RUNNABLE

So then A does:

	A::next_tid = C.tid;
	sys_umcg_wait();

Which will:

	pin(A);
	pin(S0);

	cmpxchg(A::state, RUNNING, RUNNABLE);

	next_tid = A::next_tid; // C

	enqueue(S0::runnable, A);

At which point B steals S0's runnable queue, and tries to make A go.

					runnable = xchg(S0::runnable_list_ptr, NULL); // == A
					A::server_tid = S1.tid;
					B::next_tid = A.tid;
					sys_umcg_wait();

	wake(C)
	  cmpxchg(C::state, RUNNABLE, RUNNING); <-- *fault*


Something like that, right?

And note that there's an XXX in the code about exactly this case; it has
a question whether we want to add pin(next) to umcg_pin_pages().

That would not in fact help here, because sys_umcg_wait() is faultable
and the only reason it'll return -EFAULT is because, as stated below, C
is garbage. But it does make a difference for when we do something like:

	self->next_tid = someone;
	sys_something_we_expect_to_block();
	// handle not blocking

Because in that case userspace must have taken 'someone' from the
runnable queue and made it 'next', but then we'll not wake next but the
server, which then needs to figure out something went sideways.

So I'm tempted to add that optional 3rd pin, simply to reduce the
failure cases.

What currently happens is that S0 goes back to S0 and S1 ends up in A.
That is, if, for any reason we fail to wake next_tid, we'll wake
server_tid.

So then S0 wakes up and gets to re-evaluate life. If it has another
worker it can go run that, otherwise it can try and steal a worker
somewhere or just idle out.

Now arguably, the only reason A->C can fault is because C is garbage, at
which point your program is malformed and it doesn't matter what
happens one way or the other.