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

unix::init: Also fall back from poll on ENOSYS #113358

Closed
wants to merge 1 commit into from

Conversation

mkroening
Copy link
Contributor

Operating systems like Unikraft may return ENOSYS (Function not implemented (POSIX.1-2001)) on certain syscalls.

Falling back from using poll at runtime initialization allows starting Rust applications on such systems.

Closes unikraft/lib-musl#58.

Operating systems like [Unikraft] may return [`ENOSYS`] (Function not
implemented (POSIX.1-2001)) on certain syscalls.

Falling back from using `poll` at runtime initialization allows starting
Rust applications on such systems.

[Unikraft]: https://unikraft.org/
[`ENOSYS`]: https://www.man7.org/linux/man-pages/man3/errno.3.html
@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 5, 2023
@the8472
Copy link
Member

the8472 commented Jul 5, 2023

Is unikraft a separate out of tree target or does it pretend to be linux-compatible? If it wants to be linux then imo it should support poll, since that's a pretty essential system call.
If it's a separate target then ideally it wouldn't be hitting that code-path in the first place because the polling code is behind platform-specific cfg's (albeit a blacklist).

@mkroening
Copy link
Contributor Author

Is unikraft a separate out of tree target?

Unikraft is currently out of tree, but I'll start a PR to properly add it soon.

Does Unikraft pretend to be linux-compatible? If it wants to be linux then imo it should support poll, since that's a pretty essential system call.

At the same time, Unikraft pretends to be exactly Linux with musl. As a modular libOS, components of Unikraft can be enabled at will and a module which provides the whole poll infrastructure does exist.

Nevertheless, it would be preferable if a minimal Unikraft configuration can at least start a minimal Rust application. I think falling back from using poll on ENOSYS would be very sensible, especially since the fallback path already exists for EINVAL, EAGAIN, and ENOMEM.

@the8472
Copy link
Member

the8472 commented Jul 5, 2023

Nevertheless, it would be preferable if a minimal Unikraft configuration can at least start a minimal Rust application.

It can still do that by opting out of the poll-based code path and going directly for the fcntl one.

The goal of deciding at compile-time is to minimize the amount of syscalls made at program startup.

@workingjubilee
Copy link
Member

I agree. Either you support the syscalls we expect or we should not have to pretend you qualify as target_os = "linux", because the syscalls are effectively the only part of Linux that we care about unequivocally.

@workingjubilee
Copy link
Member

workingjubilee commented Jul 5, 2023

Note that POSIX says the expected errors for poll.h are EAGAIN, EINTR, and EINVAL, and while ENOSYS is implied for any functionality not supported by the OS, poll.h is now part of the Base as of Issue 7 (IEEE Std 1003.1™-2017). We have a specific Linux kernel minimum that we support, too, so invoking POSIX here doesn't necessarily suggest we should support all versions of POSIX equally, either, nevermind on a path optimized for certain OS which meet certain expectations.

@mkroening
Copy link
Contributor Author

Thank you so much for your replies! :)

I had thought, this was an easy fix that doesn't hurt anybody. This is the only Rust-sided incompatibility for starting applications on Unikraft.

The Unikraft target is very configurable and may or may not support poll for specific builds, so reacting to “not implemented” seemed to be very sensible to me.

If you insist, I'll include the suggested cfg!-based fix when opening a PR for the Unikraft target. We were hoping to avoid cfg!s in the start, though, and were planning to only eventually start using them for optimizations.

@workingjubilee
Copy link
Member

Note that other "operating system frameworks" and the like also use cfgs to similar "handle that this may not be enabled"... Maybe we should have some sort of cfg that is easier to opt-in to when defining a target's capabilities in its target-spec.json, so it doesn't have to be special-cased for every OS like that?

@mkroening
Copy link
Contributor Author

Maybe we should have some sort of cfg that is easier to opt-in to when defining a target's capabilities in its target-spec.json, so it doesn't have to be special-cased for every OS like that?

Interesting idea, though that is probably out of scope for my work on Unikraft.

Closing this, then, since there appears to be no one in favor of this PR's approach.

@mkroening mkroening closed this Jul 5, 2023
@workingjubilee
Copy link
Member

If this wasn't the code path that Actual-Factual-Linux uses, it would be much more sensible. Please feel free to open an issue regarding this as a problem, not necessarily specific to Unikraft, because "problems with how std::sys is structured" have been a concern for some time. I don't expect you to work on it, but I would appreciate a more complete report as to what you would hope for from the internal structure of std.

Ask for @rustbot label: +A-technical-debt

@rustbot rustbot added the A-technical-debt Area: Internal cleanup work label Jul 5, 2023
@mkroening mkroening deleted the no-poll branch July 6, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-technical-debt Area: Internal cleanup work S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poll returns ENOSYS
5 participants