[PATCH 1/2] hurd: Remove the ecx kludge

[Luca]

Hi,

Il 01/03/23 17:23, Sergey Bugaev ha scritto:
> "We don't need it any more"
> 
> The INTR_MSG_TRAP macro in intr-msg.h used to play little trick with
> the stack pointer: it would temporarily save the "real" stack pointer
> into ecx, while setting esp to point to just before the message buffer,
> and then invoke the mach_msg trap. This way, INTR_MSG_TRAP reused the
> on-stack arguments laid out for the containing call of
> _hurd_intr_rpc_mach_msg (), passing them to the mach_msg trap directly.
> 
> This, however, required special support in hurdsig.c and trampoline.c,
> since they now had to recognize when a thread is inside the piece of
> code where esp doesn't point to the real tip of the stack, and handle
> this situation specially.
> 
> Commit 1d20f33ff4fb634310f27493b7b87d0b20f4a0b0 has removed the actual
> temporary change of esp by actually re-pushing mach_msg arguments onto
> the stack, and popping them back at end. It did not, however, deal with
> the rest of "the ecx kludge" code in other files, resulting in potential
> crashes if a signal arrives in the middle of pushing arguments onto the
> stack.
> 
> Fix that by removing "the ecx kludge". Instead, when we want a thread
> to skip the RPC, but cannot make just make it jump to after the trap
> since it's not done adjusting the stack yet, set the SYSRETURN register
> to MACH_SEND_INTERRUPTED (as we do anyway), and rely on the thread
> itself for detecting this case and skipping the RPC.

I still have to fully understand the existing code, so this might be 
something completely wrong... but if interrupting an rpc is a complex 
thing to do reliably in user space, why not add some kernel support? 
Also, how do you test this code?

For example, we could add a thread_interrupt_rpc() where the kernel 
could just check the state of the suspended thread:
* if the thread has a continuation set it means it still has to finish 
the send/recv operation, so we replace the continuation, discard the 
previous syscall state, and we just return an RPC_INTERRUPTED_BY_USER 
code or MACH_SEND/RCV_INTERRUPTED.
* if the thread finished the syscall, but it was de-scheduled before 
returning to userspace (by AST) it will just return normally, as the 
syscall was executed completely.

There could also be a new option to mach_msg(), so we can have 
uninterruptible mach_msg() if needed, or reuse MACH_SEND/RCV_INTERRUPT.

Does it make sense?

Also, if I understand correctly, in case the thread finished the syscall 
and successfully received a message, but didn't return to userspace yet, 
isn't there the risk of losing the message, with the current approach?


Luca