dbus: add hold/release cookie mechanism for scheduler state management#51
dbus: add hold/release cookie mechanism for scheduler state management#51e-tho wants to merge 4 commits into
Conversation
Clients that need to temporarily switch the active scheduler had no safe way to restore the previous state on cleanup, and concurrent clients would silently overwrite each other's scheduler choice. Introduce HoldScheduler/ReleaseScheduler methods modelled after power-profiles-daemon's HoldProfile/ReleaseProfile API. Acquiring a hold activates the requested scheduler and returns an opaque cookie; the pre-hold state is saved automatically. Releasing the cookie restores it. Multiple concurrent holds are supported with last-write-wins semantics — releasing the active hold falls back to the next most recent one, and releasing the last hold restores the original state. Active holds are exposed as a new ActiveHolds property for introspection. scxctl gains hold, release, and holds subcommands to exercise the new interface from the command line.
frap129
left a comment
frap129
left a comment
There was a problem hiding this comment.
Overall looks okay, but one issue I noticed. hold_scheduler and release_scheduler manage current_scx, current_mode, and current_args, but nothing prevents a client from calling switch_scheduler, stop_scheduler, restore_default, etc. while holds are active. If a switch happens while holds exist, the hold stack won't be accurate anymore. I think you need to either return an error for other scheduler change methods while !active_holds.is_empty(), or clear all holds (and the pre-hold snapshot) when a different method is called.
Calling switch, stop, restart, or restore while holds are active would leave the hold stack inconsistent with the running scheduler, making the pre-hold state unrestorable on release. All mutating methods now return an error if any holds are active.
|
I went for the conflict error approach since the other one would lead to clients calling |
| /// Opaque cookie returned to the caller. | ||
| pub cookie: u32, | ||
| /// Scheduler name requested by this hold. | ||
| pub scheduler: String, |
There was a problem hiding this comment.
| pub scheduler: String, | |
| pub scheduler: SupportedSched, |
|
|
||
| self.active_holds.push(HoldEntry { | ||
| cookie, | ||
| scheduler: <SupportedSched as Into<&str>>::into(scheduler.clone()).to_owned(), |
There was a problem hiding this comment.
why not just store SupportedSched in HoldEntry too? it's smaller and static typed
There was a problem hiding this comment.
It's a zvariant wire type, and zvariant can't derive Value/OwnedValue for SupportedSched.
3 participants
Clients that need to temporarily switch the active scheduler had no safe way to restore the previous state on cleanup, and concurrent clients would silently overwrite each other's scheduler choice.
Introduce HoldScheduler/ReleaseScheduler methods modelled after power-profiles-daemon's HoldProfile/ReleaseProfile API. Acquiring a hold activates the requested scheduler and returns an opaque cookie; the pre-hold state is saved automatically. Releasing the cookie restores it. Multiple concurrent holds are supported with last-write-wins semantics — releasing the active hold falls back to the next most recent one, and releasing the last hold restores the original state.
Active holds are exposed as a new ActiveHolds property for introspection.
scxctl gains hold, release, and holds subcommands to exercise the new interface from the command line.