Page MenuHomeFreeBSD

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

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

Details

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

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jeff created this revision.Sun, Dec 1, 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..Sun, Dec 1, 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.
jeff added a reviewer: mjg.Sun, Dec 1, 8:37 PM
kib accepted this revision.Sun, Dec 1, 8:58 PM
kib added inline comments.
sys/x86/x86/mp_x86.c
1275 ↗(On Diff #65102)

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.Sun, Dec 1, 8:58 PM
jeff added inline comments.Sun, Dec 1, 9:01 PM
sys/x86/x86/mp_x86.c
1275 ↗(On Diff #65102)

I can move it back out. I just wanted to be precise about what the different handlers required.

mav accepted this revision.Sun, Dec 1, 10:52 PM
markj accepted this revision.Mon, Dec 2, 4:35 PM
markj added inline comments.
sys/x86/x86/mp_x86.c
1275 ↗(On Diff #65102)

I've found td_intr_frame from preempted threads useful as well, FWIW.

jeff added inline comments.Mon, Dec 2, 8:13 PM
sys/x86/x86/mp_x86.c
1275 ↗(On Diff #65102)

I will move this code back in the committed patch after testing.

jhb added a comment.Mon, Dec 2, 8:59 PM

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.Mon, Dec 2, 8:59 PM