arch: custom _current implementation

[ycsin]

Allow architecture to implement their way to get the current thread for the current implementation.

---

Benchmarks

scheduler microbenchmark

Based on the upstream scheduler microbenchmark, split (and cropped) for better readability

unpend, ready, switch, pend

tot

avg

latency measurements

Based on the upstream latency measurements benchmark

ISR back to interrupted thread

ISR back to interrupted thread (without 99.9th)

For better readability:

ISR to another thread

ISR to another thread (without 99.9th)

For better readability:

Other tests

unit in ns

unit in cycles

kernel Object Performance (sys_kernel)

Based on the upstream sys_kernel microbenchmark, split for better readability

Semaphore, LIFO, FIFO, Stacks

Memslab

[ycsin]

what if we set up TLS for dummy threads?

[andyross]

Dummy threads never run, so will never access TLS. The idea is just a trick for re-using the context switch code for initial startup, you need some "thread" to "save" to when you want to start running your first thread but are still on a boot stack somewhere.

[ycsin]

probably can just call arch_current_thread() here, and use the current implementation as the default when CONFIG_ARCH_HAS_CUSTOM_CURRENT_IMPL=n

[ycsin]

cc @srv-meta

[cfriedt]

The GP reg - makes a lot of sense for RISC-V. Thank you for the figures @ycsin.

The only thing that stuck out to me as odd was that suspending and resuming a thread seemed to take slightly longer in some graphs down below.

Please take a look at this @peter-mitsis, @andyross, @npitre @fkokosinski

[nashif]

how about Global Pointer (GP) register.
Makes it easier to read and understand what GP is...

[nashif]

probably also in the kconfig itself, spell it out.

[ycsin]

I updated the prompt and the help section to spell out the 'global pointer (GP)' but kept the Kconfig name to be consistent with CONFIG_RISCV_GP

[akabaev]

The only dummy thread that should matter here is the one that Zephyr uses during startup before switching to main thread, no? Should zephyr consider to possibly

• run on main thread from the start and eliminate the need for dummy thread entirely
• maybe allocate/reserve TLS for main thread early, use it from dummy thread and then inherit it from main during the initial switch?

Then TLS can be enabled for _current retrieval.

[npitre]

Did you look at the compiled code's assembly out put to see what could
make a difference? Especially with CONFIG_CURRENT_THREAD_USE_TLS=y it
should be about equivalent already. And CONFIG_RISCV_GP=y (which conflicts
with this PR) would itself add some additional performance gain.

[ycsin]

Did you look at the compiled code's assembly out put to see what could make a difference? Especially with CONFIG_CURRENT_THREAD_USE_TLS=y it should be about equivalent already. And CONFIG_RISCV_GP=y (which conflicts with this PR) would itself add some additional performance gain.

I do not have kernel benchmark that compares CONFIG_CURRENT_THREAD_USE_TLS & CONFIG_RISCV_GP (I can probably run them tomorrow (EDIT: benchmark is here)), they weren't a clear win in our internal application benchmarks.

The CONFIG_CURRENT_THREAD_USE_TLS only affects the k_current_get() calls and is per-thread, which is different from _current (per-cpu) and isn't used much in the kernel/scheduler

[ycsin]

Updated the release & migration notes to push this closer to the finish line.

Well... if you recompile using CONFIG_OUTPUT_DISASSEMBLY=y you'll notice that currently every _current reference is turned into an out-of-line call to z_impl_k_sched_current_thread_query. This is way suboptimal. Redefining _current to use arch_current() directly would be one way to get this feature's full potential, and most probably make the compiled code smaller too.

You are absolutely right, applied the suggestion, thanks.

And for test coverage purpose this should default to y.

Undecided about this yet, would this affect the CONFIG_RISCV_GP?

---

I renamed the API from arch_current_thread to arch_curr_thread to look more consistent with arch_curr_cpu

---

RFC: I wonder if I should name it as arch_curr_cpu_thread instead, to be more distinct from k_current_get?

k_current_get is a little tricky, but it should be fine as long as it is not used in the scheduler:

• When CONFIG_CURRENT_THREAD_USE_TLS=n, it is equivalent to _current / k_sched_current_thread_query(), which is implemented in a per-CPU global variable, and return thread scheduled on the current CPU, this variable can be modified in the scheduler
• Otherwise, it returns the z_tls_current, which depends on whatever thread that calls it, and cannot change after thread entry.

[andyross]

No reason not to +1, this all looks clean. One note on some better docs so our grandchildren understand what the deal is with USERSPACE and the GP register.

Aesthetically I'm a little mixed on the last patch, though. First because I'm not a big fan of giant refactoring, second because 13 bytes of extra typing starts to get beyond my annoyance threshold. But mostly it's just that on the uniprocessor systems that are Zephyr's ancestral homeland, "_current" really is just a global struct and it's helpful to remember that.

[andyross]

Style: while asm+register variables are a really useful trick for controlling marshalling behavior around inline assembly, I think here it might be more confusing than valuable and a more traditional asm block which reads the value directly might be clearer.

[andyross]

Should document somewhere (maybe just right here in the help string) why USERSPACE is disallowed, and maybe even open an issue demanding that proper entry/exit protection for GP be implemented.

[ycsin]

Created #81843

[cfriedt]

🚀

[npitre]

I just saw the deprecation part after being merged in main.

This PR didn't include that part initially. Or did it?

What is the rationale for that part?

Personally I'm rather against this. Unless there is an actual downside,
the core of Zephyr always used _current and it was fine. It is quite
prevalent as well and the alternative is proving rather verbose.
Furthermore, as a concept, the "current thread" is not something that is
necessarily architecture specific. Having a centrally defined abstraction
also helps with instrumentation when such need comes around, etc. etc.

Again, is there a rationale for this? Otherwise I propose reverting
commit b1def71.

[ycsin]

This PR didn't include that part initially. Or did it?

What is the rationale for that part?

Yeah, it was added later, since _current functions the same as arch_current_thread(), I thought it was good to clean that up.

If we revert b1def71, it would just mean that we are going to have 3 functions to get the current thread:

• _current - the thread scheduled on the current CPU
• arch_current_thread() - interchangeably with _current
• k_current_get() - the thread from where this function is called

[npitre]

• _current - the thread scheduled on the current CPU

... to be used in the kernel's internal code.

• arch_current_thread() - interchangeably with _current

Semantically they would be different. We traditionally used arch_* for
architecture specific implementations. In this case it doesn't necessarily
have to be an architecture specific implementation. Probably the fallback
case would be better not be called arch_current_thread() for that reason
and avoid possible confusion.

• k_current_get() - the thread from where this function is called

That exists mainly as a syscall for user threads. In fact, with the gp-based
implementation on RISC-V, the syscall could be dispensed with too. But in
theory, those 3 variants could potentially be different in some ways.