Fix regression issue after r326218

[• hselasky]

Summary

The sched_add() function is not only used when the thread is initially started, but also by the turnstiles when a mutex is blocked. In r326218 some code was added which always resets the ts_cpu field which selects the current CPU a thread is executing on, disregarding prior sched_pin() and sched_bind() calls. This patch fixes the initialization of the ts_cpu field for SCHED_ULE to only happen once when the initial thread is constructed. Forking will then later on ensure that the ts_cpu value gets copied to all children.

Test Plan

Test using the RCU in the LinuxKPI quickly shows if sched_bind() and sched_pin() is working like expected.

[kib]

Hmm, after I looked over all uses of thread_alloc(), I do not see a single path where sched_add() is executed without accompanying sched_fork_thread() before it, except perhaps for the thread0. So wouldn't it be simpler to just remove the line from sched_add() for now ?

If there is some path where ts_cpu is not actually initialized before sched_add(), Nathan will point this out and we can look at either local solution or revive this approach.

For thread0, ts_cpu is set in sched_setup(). Is it too late ?

sys/kern/init_main.c

593 ↗

(On Diff #35984)

Just for the statistical reasons, I suggest to move the call right before line 496 (vm_domain_policy_init()).

sys/kern/kern_proc.c

216 ↗

(On Diff #35984)

Why do you call sched_thread_ctor() there ? The thread on the process list must be already fully initialized.

It either comes right after the thread_alloc() where the sched_thread_ctor() was already called, or is cached from the previous process instantiate, and then it must have some valid value in ts_cpu. Anyway, sched_fork() will be called shortly, which calls sched_fork_thread() which copies ts_cpu for the new thread from the current fork thread.

sys/kern/sched_ule.c

1417 ↗

(On Diff #35984)

There should be blank line after '{' if no locals.

[jhb]

sys/kern/sched_ule.c

1408 ↗

(On Diff #35984)

I wonder if you can just move this one line into schedinit(). That's where the rest of thread0's td_sched is setup. At the very least, that would replace the call to sched_thread_ctor() in init_main.c. It is also more consistent with existing code.

In fact, once that is in place, any new threads already inherit ts_cpu from the creating thread in sched_fork_thread(), so I don't think you will need anything else (so you can remove all of sched_thread_ctor()).

It's still not clear to me even in the original change why sched_add() ever needed changing as even this change is still early enough before any other kthreads are created (SI_SUB_CREATE_INIT is the first sysinit to create another thread and that is even after this code is run at SI_SUB_RUN_QUEUE). I still think it is clearer to initialize thread0's ts_cpu in schedinit() though since that is where thread0's td_sched is initialized.

[• hselasky]

Update as per suggestions from @kib and @jhb .

[jhb]

I'm fine with this if it boots ok for you. Will have to let Nathan test booting on a platform with a non-zero BSP when he is able to test.

[kib]

Note for the commit message: sched_add() actually marks the thread as runnable for all locks, for instance sleepqueues do setrunnable()->sched_wakeup()->sched_add().

[• hselasky]

@kib : got it. I'll fix the commit message before pushing the change. Thank you!