sched_ule: Enable preemption of curthread in the load balancer
ClosedPublic
Actions

Authored by markj on Jul 7 2022, 6:25 PM.

Details

Reviewers

mav
jhb
kib
jeff

Commits

rG0927ff78147b: sched_ule: Enable preemption of curthread in the load balancer

Summary

The load balancer executes from statclock and periodically tries to move
threads among CPUs in order to balance load. It may move a thread to
the current CPU (the loader balancer always runs on CPU 0). When it
does so, it may need to schedule preemption of the interrupted thread.
Use sched_setpreempt() to do so, same as sched_add().

PR: 264867

Test Plan

Using schedgraph.d I saw a softclock thread sit on CPU 0's runqueue for 50ms while
a batch-priority thread was running. Some reading of the individual trace entries
showed that the softclock thread was enqueued while running the load balancer. I
haven't been able to reproduce the problem with this patch applied.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 46353
Build 43242: arc lint + arc unit

Event Timeline

markj created this revision.Jul 7 2022, 6:25 PM

Herald added a subscriber: imp. · View Herald TranscriptJul 7 2022, 6:25 PM

Harbormaster completed remote builds in B46306: Diff 107886.Jul 7 2022, 6:25 PM

markj requested review of this revision.Jul 7 2022, 6:25 PM

markj edited the summary of this revision. (Show Details)Jul 7 2022, 7:07 PM

markj edited the test plan for this revision. (Show Details)

Not to this change, but it would be good if the load balancer could somehow prefer to migrate long running threads, not some random interrupt threads it may spot by pure luck.

sys/kern/sched_ule.c
2666–2668	Just a cosmetics and not yours, but in this particular case variable initialization in declaration would make more sense to me to assert the thread lock more specifically.

This revision is now accepted and ready to land.Jul 7 2022, 7:48 PM

kib accepted this revision.Jul 8 2022, 11:43 AM

kib added inline comments.

sys/kern/sched_ule.c
971	Where is an AST is scheduled?

markj marked 2 inline comments as done.Jul 10 2022, 4:21 PM

markj added inline comments.

sys/kern/sched_ule.c
971	In sched_setpreempt(). Probably it should be "preemption" instead?

Address review feedback.

This revision now requires review to proceed.Jul 10 2022, 4:21 PM

Harbormaster completed remote builds in B46353: Diff 107983.Jul 10 2022, 4:22 PM

kib accepted this revision.Jul 10 2022, 6:49 PM

kib added inline comments.

sys/kern/sched_ule.c
971	You mean setting td_owepreempt, which is definitely not an ast. Might be, finish the sentence as `schedule a preemption by setting td_owepreempt for current thread`.

This revision is now accepted and ready to land.Jul 10 2022, 6:49 PM

markj added inline comments.Jul 10 2022, 7:11 PM

sys/kern/sched_ule.c
971	If the interrupted thread was in user mode, then preemption is triggered by setting TDF_NEEDRESCHED, which is handled in ast(). td_owepreempt is set as well, but this mechanism only affects kernel mode interruptions.