-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Use clock BOOTTIME for thread::sleep on linux #113513
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
Conversation
Thread::sleep uses nanosleep. Nanosleep uses the MONOTONIC clock. That clock does not take suspend time into account. Since kernel commit d6ed449 the MONOTONIC clock behaves the same as the BOOTTIME clock. The BOOTTIME clock does count time spend suspended. Commit d6ed449 was merged somewhere between v4.14 and v4.19. Rust supports kernels before and after that. This commit changes sleep to use the BOOTTIME clock on linux targes. This makes sleep behave consistently on linux regardless of kernel version.
r? @cuviper (rustbot has picked a reviewer for you, use r? to override) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
That's wrong. It was reverted shortly after in torvalds/linux@a3ed0e4 |
I missed that, thanks a lot! No need for this then. |
Thread::sleep
uses nanosleep. Nanosleep uses theMONOTONIC
clock. That clock does not take suspend time into account. Since kernel commit d6ed449 the MONOTONIC clock behaves the same as theBOOTTIME
clock. TheBOOTTIME
clock does count time spend suspended.Kernel commit d6ed449 was merged somewhere between v4.14 and v4.19. Rust supports kernels before and after that.
This commit changes sleep to use the
BOOTTIME
clock on linux targes. This makes sleep behave consistently on linux regardless of kernel version.I discovered this while preparing the implementation for rust-lang/libs-team#237. Fixing it changes behavior of rust code on older kernel versions and could therefore be seen as breaking. Since the kernel also broke behavior the same way I think it might be okay.
A proposal (#88714) to switch to the
BOOTTIME
clock forInstant
ran into trouble as the minimally supported linux kernel did no supportBOOTTIME
. Since then the minimal kernel version has been increased and this is no longer an issue.It might be useful to re-evaluate that given the change made in kernel commit d6ed449. Lets however first look at
sleep
before we open that can of worms.I think this change is small and simple enough for a PR if its not let me know and I'll move it to a libs team RFC.
Sources: