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

selectors_epoll: use pidfd for registerProcess #23047

Draft
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

shirleyquirk
Copy link
Contributor

fixes #22758
requires linux > 5.3

previous signalfd method only worked on child processes.
this works on any process, and raises if the pid is invalid

the previous logic in selectInto where it checked if the read signalfdinfo.ssi_pid matched; we are guaranteed a separate fd for each pid, so its not necessary.

fixes nim-lang#22758
requires linux > 5.3
previous signalfd method only worked on child processes
this works on any process, and raises if the pid is invalid

the previous logic in selectInto where it checked if the read signalfdinfo.ssi_pid matched; we are guaranteed a separate fd for each pid, so its not necessary.
@shirleyquirk
Copy link
Contributor Author

err. maybe you dont need to use syscall.
sorry. man pages are outdated. this could be
importc pidfd_open header:"sys/pidfd.h" from glibc2.36

@shirleyquirk
Copy link
Contributor Author

TODO add test for #19152

@shirleyquirk
Copy link
Contributor Author

shirleyquirk commented Dec 9, 2023

#12354
maybe this would be an opportunity to introduce a new, non-discardable version
but that's pretty out of scope

@Araq
Copy link
Member

Araq commented Dec 9, 2023

How common is linux > 5.3? Seems rather dangerous to me.

@shirleyquirk
Copy link
Contributor Author

shirleyquirk commented Dec 11, 2023

it's crazy new, and should have a fallback, absolutely.

i wasn't sure where to put the check:

  • can we check at compiletime for the target linux version?
  • or for the presence of pidfd_open? (glibc > 2.36 musl > ?

failing one of those, if the syscall returns ENOSYS, could fall back to the signalfd method. that would require hardcoding SYS_pidfd_open so it can always compile.

@Araq
Copy link
Member

Araq commented Dec 12, 2023

It must be a runtime check, I think.

@shirleyquirk shirleyquirk marked this pull request as draft December 13, 2023 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants