-
-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix unix changing from real-time to normal policy #39
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -596,19 +596,29 @@ pub fn set_thread_priority_and_policy( | |
e => Err(Error::OS(e)), | ||
} | ||
} else { | ||
// If this is a normal-scheduled thread, the priority is | ||
// set via niceness. | ||
set_errno(0); | ||
// Normal priority threads must be set with static priority 0. | ||
let params = ScheduleParams { sched_priority: 0 }.into_posix(); | ||
|
||
let ret = unsafe { libc::setpriority(libc::PRIO_PROCESS, 0, fixed_priority) }; | ||
if ret == 0 { | ||
return Ok(()); | ||
let ret = unsafe { | ||
libc::pthread_setschedparam( | ||
native, | ||
policy.to_posix(), | ||
¶ms as *const libc::sched_param, | ||
) | ||
}; | ||
|
||
if ret != 0 { | ||
return Err(Error::OS(ret)); | ||
} | ||
|
||
match errno() { | ||
0 => Ok(()), | ||
e => Err(Error::OS(e)), | ||
// Normal priority threads adjust relative priority through niceness. | ||
set_errno(0); | ||
let ret = unsafe { libc::setpriority(libc::PRIO_PROCESS, 0, fixed_priority) }; | ||
if ret != 0 { | ||
return Err(Error::OS(errno())); | ||
} | ||
|
||
Ok(()) | ||
} | ||
} | ||
} | ||
|
@@ -820,6 +830,44 @@ mod tests { | |
assert!(thread_schedule_policy_param(thread_id).is_ok()); | ||
} | ||
|
||
// Running this test requires CAP_SYS_NICE. | ||
#[test] | ||
fn change_between_realtime_and_normal_policies_requires_capabilities() { | ||
use crate::ThreadPriorityOsValue; | ||
|
||
const TEST_PRIORITY: u8 = 15; | ||
|
||
let realtime_policy = ThreadSchedulePolicy::Realtime(RealtimeThreadSchedulePolicy::Fifo); | ||
let normal_policy = ThreadSchedulePolicy::Normal(NormalThreadSchedulePolicy::Other); | ||
|
||
// While we may desire an OS-specific priority, the reported value is always crossplatform. | ||
let desired_priority = ThreadPriority::Os(ThreadPriorityOsValue(TEST_PRIORITY as _)); | ||
let expected_priority = ThreadPriority::Crossplatform(ThreadPriorityValue(TEST_PRIORITY)); | ||
|
||
let thread = std::thread::current(); | ||
thread | ||
.set_priority_and_policy(realtime_policy, desired_priority) | ||
.expect("to set realtime fifo policy"); | ||
|
||
assert_eq!(thread.get_schedule_policy(), Ok(realtime_policy)); | ||
assert_eq!(thread.get_priority(), Ok(expected_priority)); | ||
|
||
thread | ||
.set_priority_and_policy(normal_policy, desired_priority) | ||
.expect("to set normal other policy"); | ||
|
||
assert_eq!(thread.get_schedule_policy(), Ok(normal_policy)); | ||
|
||
// On linux, normal priority threads always have static priority 0. Instead the "nice" value is used. | ||
#[cfg(not(target_os = "linux"))] | ||
assert_eq!(thread.get_priority(), Ok(expected_priority)); | ||
#[cfg(target_os = "linux")] | ||
{ | ||
let nice = unsafe { libc::getpriority(0, 0) }; | ||
assert_eq!(nice, TEST_PRIORITY as i32); | ||
} | ||
} | ||
iddm marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+834
to
+869
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Expand the test coverage in The current test validates the basic functionality of changing policies and priorities. However, it could be beneficial to include more edge cases, such as invalid priorities or unsupported policies, to ensure robustness. Would you like me to help by adding these test cases? |
||
|
||
#[test] | ||
#[cfg(target_os = "linux")] | ||
fn set_deadline_policy() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider consolidating error handling for clarity and maintainability.
The current implementation handles errors from
pthread_setschedparam
andsetpriority
in separate blocks. This could be streamlined by consolidating the error handling into a single block, improving readability and maintainability. Here's a proposed refactor: