-
Notifications
You must be signed in to change notification settings - Fork 356
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
Add support for tid argument in sched_(get/set)affinity #4130
base: master
Are you sure you want to change the base?
Conversation
// TODO: when https://github.com/rust-lang/miri/issues/3730 is fixed this should use its notion of tid/pid | ||
let thread_id = match pid { | ||
0 => this.active_thread(), | ||
let thread_id = match (pid, &*this.tcx.sess.target.os) { |
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.
Seems a bit odd to match on the pair here. I think this is more clear as
if pid == 0 { ... }
else if this.tcx.sess.target.os == "linux" {
// On Linux we support `gettid`, which is guaranteed to support IDs matching `sched_getaffinity`.
...
} else { ... }
let thread_id = match (pid, &*this.tcx.sess.target.os) { | ||
(0, _) => this.active_thread(), | ||
(tid, "linux") => { | ||
if let Ok(tid) = this.machine.threads.thread_id_try_from(tid) { |
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.
This will not return the right ID. See the gettid implementation to see how ThreadId is converted to the user-visible TID; this here will have to do the opposite transformation. Please put both directions in some shared place to make sure they remain consistent in the future.
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.
Thanks for the PR! However, as-is this doesn't quite have the right behavior.
Please also add a test ensuring we can set an affinity via the TID and then get it via "0" and vice versa.
☔ The latest upstream changes (possibly 1c87fa0) made this pull request unmergeable. Please resolve the merge conflicts. |
Linked to #4062
On Linux, values returned by
gettid
can be passed as thepid
argument when callingsched_getaffinity
orsched_setaffinity
.