Add SO_PREFER_BUSY_POLL and SO_BUSY_POLL_BUDGET by tammela · Pull Request #3917 · rust-lang/libc

tammela · 2024-09-11T19:54:28Z

Remove the comment of these socket options.
Reference: https://elixir.bootlin.com/linux/latest/source/include/uapi/asm-generic/socket.h

rustbot · 2024-09-11T19:54:33Z

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

tammela · 2024-09-11T20:30:18Z

From the CI results (i686-unknown-linux-musl) it seems like the machine does not have a uapi header with the constant.
They were added in 5.11 and the sanitized headers are in 4.19: https://github.com/rust-lang/libc/blob/main/ci/install-musl.sh#L68

devnexen · 2024-09-11T20:53:53Z

for the time being, need to add those constants as exception (for CI), as if musl then ignore those two.

tammela · 2024-09-12T13:43:16Z

@devnexen Got it. I was wondering if someone tried to update the kernel headers for the musl CI.
Alpine uses musl as well and they don't seem to use sabotage-linux headers but rather roll their own (currently targeting 6.6 soon to be 6.10):
https://git.alpinelinux.org/aports/tree/main/linux-headers?h=master

tammela · 2024-09-12T18:16:05Z

I updated the CI to use the alpine headers here: tammela@17af7ba

Seems to be passing the CI scripts, let me know if it's worth the shot and I can open up a PR

devnexen · 2024-09-12T18:18:13Z

nice, it might be best to create a PR for this musl update alone tough. cc @tgross35

tgross35

Huh, no clue why those were commented out but looks like it has been that way since #2135. Anyway lgtm, thanks for including the link.

tammela · 2024-09-30T17:58:22Z

Needs this one to pass CI: #3921

tgross35

#3921 should be merged soon, would you mind rebasing to resolve conflicts?

Also, could you add these to the semver tests at libc-test/semver/linux.txt?

tgross35 · 2024-11-27T17:10:55Z

It looks like CI is passing - is this ready to be merged or is there something else to be done? (I've held off because of the labels)

tammela · 2024-11-27T17:21:52Z

It looks like CI is passing - is this ready to be merged or is there something else to be done? (I've held off because of the labels)

I gave in and gated the new constants for !musl. So it should be good from my side.
Musl made the decision to hardcode the SO_ constants instead of inherit them from the OS via #include <asm-generic/socket.h>.
Essentially updating the kernel headers is not enough, you have to hope a recent musl version kept up with the new constants...
The next musl version update will likely fill in the gap (till 70), but linux already added a bunch more :)

tgross35 · 2024-11-27T18:05:15Z

libc-test/semver/TODO-linux.txt

 SO_BINDTOIFINDEX
 SO_BPF_EXTENSIONS
 SO_BSDCOMPAT
+SO_BUSY_POLL_BUDGET


Could you instead add these to linux-gnu.txt or linux-gnu-x86_64.txt instead?

Since there is something left to do, maybe leave a // FIXME(musl) in the .rs file rather than using the TODO semver.

Good catch, will do!

I think it needs to go in one of the linux-gnu* files rather than linux-{arch} since these aren't yet available on musl

I added to the x86 one, but it should be fine for everything except musl, but it doesn't seem to have such a file

tgross35 · 2025-02-22T22:08:22Z

src/unix/linux_like/linux/arch/generic/mod.rs

+        pub const SO_PREFER_BUSY_POLL: c_int = 69;
+        pub const SO_BUSY_POLL_BUDGET: c_int = 70;
    }
 }
-// pub const SO_PREFER_BUSY_POLL: c_int = 69;
-// pub const SO_BUSY_POLL_BUDGET: c_int = 70;


Sorry, this slipped under the radar. I don't understand what the change is here; it looks like the net effect is changing SO_PREFER_BUSY_POLL and SO_BUSY_POLL_BUDGET from being available on all platforms to only being available on non-musl platforms. Why is this needed? Musl does seem to define them https://github.com/kraj/musl/blob/1880359b54ff7dd9f5016002bfdae4b136007dde/include/sys/socket.h#L292-L293.

At the time they didn't have this constants, let me push with them outside and see if it works now!

So I went a bit further in the upgrading musl rabbit hole.

Musl 1.2.x now defines time to be 64 bits on 32 bit systems. Unfortunately, that's the version that has the constants we are looking for.
Time in 64 bits changes a bunch of struct definitions and even syscalls so it's a big change [1].
I can open up a PR changing some low hanging fruits to kickstart the transition.
One deficiency we have is that two new types are being used to help the transition: __kernel_ulong_t and __kernel_long_t. These can be redefined on 32bits to be long long variants [2], so far x86 is the only arch expanding them.

The syscall change is more challenging, the tests fail with:

bad recvmmsg function pointer: rust: 139377735 (0x84ebc47) != c 139259307 (0x84cedab) bad gettimeofday function pointer: rust: 139376444 (0x84eb73c) != c 139362562 (0x84e8102) bad adjtimex function pointer: rust: 139375084 (0x84eb1ec) != c 139209963 (0x84c2ceb) bad clock_adjtime function pointer: rust: 139375189 (0x84eb255) != c 139210036 (0x84c2d34)

Not sure how to this fix those as SYS_recvmmsg works on 64-bit but on 32-bit you would need SYS_recvmmsg_time64 + something else?

[1] https://git.musl-libc.org/cgit/musl/tree/src/network/recvmmsg.c#n21
[2] https://elixir.bootlin.com/linux/v6.13.4/source/arch/x86/include/uapi/asm/posix_types_x32.h#L15

If the SO_ constants are useful then we can add them but just skip tests on musl in libc-test/build.rs, separate from the musl upgrade.

The musl upgrade is unfortunately tricky, but very much needs to happen. #3791 does most of the changes (and I left some review there), that PR is a cleaned up version of #3068. Cc #4148 which made linux uapi configurable - we'd probably want something similar for musl so we can unstably test before 1.0.

(If you're considering working on this, feel free to ask me questions on zulip)

Oh great! I didn't know about this PR.
I did a quick run with my patch applied on top of it and it works just fine!

To be clear, that PR cannot be merged in its current state. This doesn't need to block the change here though, you can just skip the constants on musl in libc-test/build.rs.

To be clear, that PR cannot be merged in its current state

Oh, what is missing?

This doesn't need to block the change here though, you can just skip the constants on musl in libc-test/build.rs.

Will do

Oh, what is missing?

Not that there is anything fundamentally wrong with the change, but it is pretty out of date and there are a lot of open review comments. I haven't heard from the author in a while so unfortunately it likely needs somebody else to pick it up.

tgross35 · 2025-03-04T00:54:52Z

libc-test/build.rs

            if loongarch64 && (name == "MFD_NOEXEC_SEAL" || name == "MFD_EXEC") {
                return true;
            }
+            // FIXME: Requires musl >= 1.2


Could you make this FIXME(musl)? Trying to keep all our fixmes greppable since we have so many of them

tgross35

One nit but lgtm. CI failures look spurious.

Remove the comment of these socket options. Reference: https://elixir.bootlin.com/linux/latest/source/include/uapi/asm-generic/socket.h Note, musl hardcodes 'SO_*' constants instead of inheriting them from the OS. Signed-off-by: Pedro Tammela <pctammela@gmail.com>

tgross35 · 2025-03-04T21:13:56Z

Thanks!

Remove the comment of these socket options. Reference: https://elixir.bootlin.com/linux/latest/source/include/uapi/asm-generic/socket.h Note, musl hardcodes 'SO_*' constants instead of inheriting them from the OS. Signed-off-by: Pedro Tammela <pctammela@gmail.com> (backport <rust-lang#3917>) (cherry picked from commit b3884fb)

rustbot assigned tgross35 Sep 11, 2024

rustbot added the S-waiting-on-review label Sep 11, 2024

tammela mentioned this pull request Sep 11, 2024

Uncommenting SO_PREFER_BUSY_POLL and SO_BUSY_POLL_BUDGET #3896

Closed

tgross35 approved these changes Sep 30, 2024

View reviewed changes

tgross35 force-pushed the patch-1 branch from c864b81 to 81f8510 Compare September 30, 2024 16:13

tgross35 enabled auto-merge September 30, 2024 16:13

tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Sep 30, 2024

tgross35 added S-blocked and removed S-waiting-on-review labels Oct 15, 2024

tgross35 force-pushed the patch-1 branch from 81f8510 to 3876b60 Compare November 6, 2024 19:45

tgross35 added S-waiting-on-ci S-blocked and removed S-blocked S-waiting-on-ci labels Nov 6, 2024

tgross35 requested changes Nov 19, 2024

View reviewed changes

rustbot added the S-waiting-on-author label Nov 19, 2024

auto-merge was automatically disabled November 21, 2024 16:26
Head branch was pushed to by a user without write access

tammela force-pushed the patch-1 branch from 3876b60 to 65a772f Compare November 21, 2024 16:26

rustbot added O-linux O-unix labels Nov 21, 2024

tgross35 removed the S-blocked label Nov 21, 2024

tgross35 force-pushed the patch-1 branch from 65a772f to 23732fe Compare November 21, 2024 19:00

tgross35 approved these changes Nov 21, 2024

View reviewed changes

tgross35 reviewed Nov 27, 2024

View reviewed changes

tammela force-pushed the patch-1 branch 3 times, most recently from 72bf054 to 8ddba26 Compare November 27, 2024 21:40

tgross35 reviewed Feb 22, 2025

View reviewed changes

tammela force-pushed the patch-1 branch 3 times, most recently from 473158f to 597ae13 Compare February 28, 2025 15:35

rustbot added O-musl O-mips O-sparc labels Feb 28, 2025

tammela force-pushed the patch-1 branch 2 times, most recently from ba7e8fd to dc5cb31 Compare March 3, 2025 15:46

tgross35 reviewed Mar 4, 2025

View reviewed changes

tgross35 approved these changes Mar 4, 2025

View reviewed changes

tammela force-pushed the patch-1 branch from dc5cb31 to b3884fb Compare March 4, 2025 18:36

tgross35 added this pull request to the merge queue Mar 4, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 4, 2025

tgross35 added this pull request to the merge queue Mar 4, 2025

Merged via the queue into rust-lang:main with commit 75542bc Mar 4, 2025
40 of 44 checks passed

tgross35 mentioned this pull request Mar 10, 2025

[0.2] Backports #4315

Merged

tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Mar 10, 2025

Conversation

tammela commented Sep 11, 2024

Uh oh!

rustbot commented Sep 11, 2024

Uh oh!

tammela commented Sep 11, 2024

Uh oh!

devnexen commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tammela commented Sep 12, 2024

Uh oh!

tammela commented Sep 12, 2024

Uh oh!

devnexen commented Sep 12, 2024

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

tammela commented Sep 30, 2024

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

tgross35 commented Nov 27, 2024

Uh oh!

tammela commented Nov 27, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammela Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammela Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammela Mar 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Mar 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammela Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 commented Mar 4, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

devnexen commented Sep 11, 2024 •

edited

Loading

tammela Feb 28, 2025 •

edited

Loading

tammela Feb 28, 2025 •

edited

Loading

tammela Mar 1, 2025 •

edited

Loading

tgross35 Mar 1, 2025 •

edited

Loading

tammela Mar 3, 2025 •

edited

Loading

tgross35 left a comment •

edited

Loading