arch: arm: restrict IRQ lock to minimum during pendSV exception

[ioannisg]

When performing thread context-switch it is not necessary to
have IRQs locked while saving the current thread's callee-saved
(and possibly floating point) registers. We only need to lock
the interrupts when accessing the thread ready queue cache.

Signed-off-by: Ioannis Glaropoulos [email protected]

[ioannisg]

This patch is an optimization; reduces the time we lock the IRQs during context switch.
The principle is simple - we only need to protect during accessing the thread ready queue, not when we are saving the context of the current thread.

The patch is based on the assumption that _current is only changed by this function, a few instructions later, when the new thread is switched in. @andyross @pizi-nordic could you confirm this?

[andrewboie]

can you illustrate what happens if, during this window where we have partially saved the register state, we get an interrupt which causes the system to decide to schedule some other thread?

[andyross]

So... yes, the _current pointer only gets assigned in the arch-defined z_swap() implementation (or for USE_SWITCH architectures, the kernel-provided portable one). You're safe as far as that goes.

But remember that PendSV is not atomic with respect to the original call to z_swap(). The thread to which you are swapping is not necessarily the one the kernel thread "thought" it was switching to when it called z_swap() in the first place. If , as I understand the commit message, all the unlocked code here is doing is writing to stack memory (and the stack pointer is always consistent!) then this should be OK. But if you're inspecting or writing to other memory it's likely to have holes.

[ioannisg]

@andyross thanks very much for your feedback.

@andrewboie sure, I can briefly comment on this:
The ARM call procedure standard ensures that the register content remains the same after any interrupt that preempts a lower-priority interrupt - the context-switch (pendSV) in our case:

• caller-saved registers are stacked and restored on current MSP by the exception entry/return
• callee-saved registers are always restored on sub-routine return

A preempting ISR will, therefore, allow the context saving to safely resume when returning to the pendSV.

Also, for ARM an interrupt cannot be, normally, nested in its own. Therefore, the pendSV cannot be interrupted by another pendSV playing with context saving!

All this patch is doing is to un-protect the context saving of the current thread; as I argued above, this seems not needed, as the _current pointer is not changed by any other Kernel/ARCH code. It is only touched later in the swap routine.

If a preempting interrupt decides to schedule some other thread, it will modify the ready queue cache, but that is fine, until the very last moment we will need to read this! And that part of the swap-routine is still protected inside interrupts lock-unlock.

[ioannisg]

@pizi-nordic pls, have a quick look, as well