sched: Flag to force scheduler to always choose idle thread
Needs ReviewPublic
Actions

Authored by obiwac on Dec 29 2025, 8:54 PM.

Details

Reviewers

olce
markj

Summary

For s2idle, we need a reliable way to force the scheduler into an idle loop.

This is a bit heavy-handed and not necessarily the right approach - revision opened mostly so @olce can have a look :) Also this would need to be done with the old scheduler if we go ahead with this design.

The small change at line 1502 is because my language server is complaining about it being a misleading statement.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 69511
Build 66394: arc lint + arc unit

Event Timeline

obiwac created this revision.Dec 29 2025, 8:54 PM

Herald added a subscriber: imp. · View Herald TranscriptDec 29 2025, 8:54 PM

obiwac requested review of this revision.Dec 29 2025, 8:54 PM

Harbormaster completed remote builds in B69511: Diff 168740.Dec 29 2025, 8:54 PM

obiwac edited the summary of this revision. (Show Details)Dec 29 2025, 8:54 PM

obiwac edited the summary of this revision. (Show Details)

obiwac edited the summary of this revision. (Show Details)Dec 29 2025, 8:58 PM

obiwac added a child revision: D54409: x86: New (un)idle IPIs for setting idle flag in scheduler.

emaste added a subscriber: emaste.Jan 6 2026, 4:53 PM

obiwac planned changes to this revision.Jan 14 2026, 4:21 PM

I have a couple of comments, but really we should first determine what to do with D54410, please see there.

sys/kern/sched_ule.c
1503–1505	Strange that a language server complains about that... I don't see what is misleading with the original form, except for indentation, where one can argue it doesn't matter here. The new form is not `style(9)` compliant, and that I find really misleading :-) (a very quick glance does not see that the `if` has an `else` branch). A way out which is imperfect but that I often use: Put the herald comment inside the block guarded by the `if` instead of before, possible modify the comment's content due to the new location, and format as prescribed by `style(9)`. In the end, keeping the original code is probably the best option. If doing some change here, it should anyway be a separate commit.
2666	Seems much simpler to continue not returning anything, and unlock as needed in the function.
2679	See previous inline comment.
2685	See previous inline comment.
2697–2698
2710–2711
2867–2869	I'd move that at the end of `tdq_choose()` instead of here, even if this function currently has the only call to `tdq_choose()`.

Make sched_preempt_locked entirely responsible for releasing thread lock and style change.

Harbormaster completed remote builds in B70020: Diff 170129.Tue, Jan 20, 3:34 PM

thanks for the review!

and pending D54410 then

sys/kern/sched_ule.c
1503–1505	It's actually complaining about the next if over (the global search for least loaded CPU) being indented too deeply, which I don't quite understand either. Possibly a bug in the LS. (a very quick glance does not see that the if has an else branch). Counterpoint is that if you read the subsequent "if" its not immediately obvious that its an else if and looks like it stands on its own. A way out which is imperfect but that I often use: Put the herald comment inside the block guarded by the if instead of before, possible modify the comment's content due to the new location, and format as prescribed by style(9). I do like this option. What would you like to change in the comment's content though? I'm not claiming to understand this code entirely but it seems like the body of the `if (ccg != NULL)` is doing exactly what the comment describes, i.e. looking for the least loaded CPU in the LLC CPU group. If doing some change here, it should anyway be a separate commit. yup :)
2666	Feels kind of weird to mix responsibilities like this (i.e. having the caller lock and the callee unlock), but i suppose there's no real better way so long as `mi_switch()` can unlock anyways.
2867–2869	I guess that makes things a little more complicated as `tdq_choose()` doesn't have a single path where it returns the thread. I guess I could just do this check on the return after `runq_choose_idle()` though?