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

hwloc_topology_set_pid seems to lack error checking on Linux #624

Open
HadrienG2 opened this issue Oct 10, 2023 · 1 comment
Open

hwloc_topology_set_pid seems to lack error checking on Linux #624

HadrienG2 opened this issue Oct 10, 2023 · 1 comment

Comments

@HadrienG2
Copy link

HadrienG2 commented Oct 10, 2023

As of hwloc v2.9.3, and at least on Linux, hwloc_topology_set_pid() seems to accept nonexistent PIDs as input without complaining (it keeps returning 0). Hopefully it does nothing in this scenario, or the error is reported later e.g. at topology loading time? But an error should probably be signaled early on to ease debugging if possible, because even errors at loading time are already ambiguous (it's not clear which configuration parameter caused the error).

@bgoglin
Copy link
Contributor

bgoglin commented Oct 10, 2023

Reporting an error early wouldn't help much since the PID could disappear between set_pid() and load(), or even be replaced by another process. There are lots of work going on in Linux to avoid this kind of issues (the overall idea is to "acquire a pidfd" so that the PID cannot be reused in the middle), it's a really complicated problem in general, lots of API and syscalls might get changed because of this.

In the end, our topology PID is only used for binding and for getting cgroup info. Binding will just fail. Cgroup won't be found, and it will fallback to no-restricting (default cgroup). I am not sure we want to report a late load() an error in that case.

bgoglin added a commit that referenced this issue Oct 10, 2023
bgoglin added a commit that referenced this issue Oct 10, 2023
Refs #624.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e381de8)
bgoglin added a commit that referenced this issue Oct 10, 2023
Refs #624.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e381de8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants