sched: Flag to force scheduler to always choose idle thread

[obiwac]

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.

[olce]

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

1502–1506

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.

2667–2668

Seems much simpler to continue not returning anything, and unlock as needed in the function.

2680

See previous inline comment.

2685

See previous inline comment.

2697–2698

2710–2711

2865–2868

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().

[obiwac]

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

[obiwac]

thanks for the review!

and pending D54410 then

sys/kern/sched_ule.c

1502–1506

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 :)

2667–2668

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.

2865–2868

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?