Running the sched_preempt handler in a critical section causes us to acquire the scheduler lock twice in a row. It is not necessary since interrupts aredisabled already in this path.
ClosedPublic
Actions

Authored by jeff on Dec 1 2019, 8:35 PM.

Details

Reviewers

jhb
kib
mav
markj
mjg

Commits

rS355311: Don't run sched_preempt() inside of an extra critical section. This disables

Summary

I am working on reducing the diffs in my scheduler locking change branch. https://github.com/freebsd/freebsd/compare/master...jwroberson:schedlock

This is a small change that allows the direct switch optimization in sched_preempt() to work as intended. Otherwise we grab the sched lock, set owepreempt, drop the sched lock, critical_exit, acquire the sched lock, see owepreempt, then switch.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27873
Build 26044: arc lint + arc unit

Event Timeline

jeff created this revision.Dec 1 2019, 8:35 PM

Harbormaster completed remote builds in B27873: Diff 65102.Dec 1 2019, 8:35 PM

jeff retitled this revision from Running the sched_preempt handler in a critical section causes us to acquire the scheduler lock twice in a row. It is not necessary since interrupts are disabled already in this path. to Running the sched_preempt handler in a critical section causes us to acquire the scheduler lock twice in a row. It is not necessary since interrupts aredisabled already in this path..Dec 1 2019, 8:37 PM

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

jeff added reviewers: jhb, kib, mav, markj.

jeff set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptDec 1 2019, 8:37 PM

jeff added a reviewer: mjg.Dec 1 2019, 8:37 PM

kib accepted this revision.Dec 1 2019, 8:58 PM

kib added inline comments.

sys/x86/x86/mp_x86.c
1275	I must note that having oldframe and td_intr_frame correct in larger region is sometimes useful. I do not mind much about intr_nesting_level.

This revision is now accepted and ready to land.Dec 1 2019, 8:58 PM

jeff added inline comments.Dec 1 2019, 9:01 PM

sys/x86/x86/mp_x86.c
1275	I can move it back out. I just wanted to be precise about what the different handlers required.

mav accepted this revision.Dec 1 2019, 10:52 PM

markj accepted this revision.Dec 2 2019, 4:35 PM

markj added inline comments.

sys/x86/x86/mp_x86.c
1275	I've found td_intr_frame from preempted threads useful as well, FWIW.

jeff added inline comments.Dec 2 2019, 8:13 PM

sys/x86/x86/mp_x86.c
1275	I will move this code back in the committed patch after testing.

I suspect the original critical section was only for hardclock() (wanted all of hardclock() to finish before preempting to run softclock()) and that sched_preempt ended up under it due to inertia.

jhb accepted this revision.Dec 2 2019, 8:59 PM

Closed by commit rS355311: Don't run sched_preempt() inside of an extra critical section. This disables (authored by jeff). · Explain WhyDec 2 2019, 10:34 PM

This revision was automatically updated to reflect the committed changes.

jeff added a commit: rS355311: Don't run sched_preempt() inside of an extra critical section. This disables.