Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sleiderr
Copy link

@sleiderr sleiderr commented Jan 9, 2025

Linked to #4062

On Linux, values returned by gettid can be passed as the pid argument when calling sched_getaffinity or sched_setaffinity.

// 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) {
Copy link
Member

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) {
Copy link
Member

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.

Copy link
Member

@RalfJung RalfJung left a 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.

@rustbot
Copy link
Collaborator

rustbot commented Jan 12, 2025

☔ The latest upstream changes (possibly 1c87fa0) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: Waiting for the PR author to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants