-
Notifications
You must be signed in to change notification settings - Fork 406
Add support for tid argument in sched_(get/set)affinity #4130
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
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.
RalfJung
left a comment
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. |
|
I'm going to close this PR due to inactivity. Thanks for trying to contribute, and feel free to reopen or make a new PR if you want to get back it it again! |
Linked to #4062
On Linux, values returned by
gettidcan be passed as thepidargument when callingsched_getaffinityorsched_setaffinity.