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

define struct clone_args for linux-lts versions that don't have it #741

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

copy
Copy link
Contributor

@copy copy commented Jun 17, 2024

I've chosen to define this struct unconditionally, and rename it:

  • Renaming is necessary because, if we redefine struct clone_args, we'll get a compilation error if sched.h contains an older version of this struct
  • I believe it's not possible to detect if a struct is defined using just preprocessor macros, so a configure script would be needed
  • CLONE_PIDFD is not defined in the linux-lts headers, but it is defined in musl, so it can't be used to determine if struct clone_args is defined
  • The Linux syscall interface is backwards-compatible anyway

But I'm open to suggestions for other fixes.

Fixes #737

@copy
Copy link
Contributor Author

copy commented Jun 18, 2024

I realised that the original version caused a regression: Using the latest version of the struct unconditionally would break older kernels (at runtime). I've instead changed it to use the first version of the struct. The current code doesn't use any of the new fields (set_tid, set_tid_size and cgroup). If you'd like to check, sizeof caml_eio_clone_args now matches CLONE_ARGS_SIZE_VER0 from linux/sched.h.

Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - thanks!

@talex5 talex5 merged commit 642bdbe into ocaml-multicore:main Jun 19, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

Fails to build with musl-gcc w/ linux-lts headers (no struct clone_args)
2 participants