unscaledcycleclock: use RDTIME instead of RDCYCLE on RISC-V #1631
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Starting with Linux 6.6 [1], RDCYCLE is a privileged instruction on RISC-V and can't be used directly from userland. There is a sysctl option to change that as a transition period, but it will eventually disappear. Use RDTIME instead, which while less accurate has the advantage of being synchronized between CPU (and thus monotonic) and of constant frequency. Therefore this change also fixes another issue caused by wrong time measurement when a process is rescheduled from one CPU to another [2]. Given RDTIME uses a timer independent from the CPU, this patche removes the ABSL_INTERNAL_UNSCALED_CYCLECLOCK_FREQUENCY_IS_CPU_FREQUENCY define accordingly. This fixes the testsuite on riscv64, tested on a VisionFive 2 board. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc4c07c89aada16229084eeb93895c95b7eabaa3 [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025221
derekmauro
requested changes
Mar 11, 2024
| @@ -53,7 +53,7 @@ | |||
| #if ABSL_USE_UNSCALED_CYCLECLOCK | |||
| // This macro can be used to test if UnscaledCycleClock::Frequency() | |||
| // is NominalCPUFrequency() on a particular platform. | |||
| #if (defined(__i386__) || defined(__x86_64__) || defined(__riscv) || \ | |||
| #if (defined(__i386__) || defined(__x86_64__) || \ | |||
| defined(_M_IX86) || defined(_M_X64)) | |||
| #define ABSL_INTERNAL_UNSCALED_CYCLECLOCK_FREQUENCY_IS_CPU_FREQUENCY | |||
There was a problem hiding this comment.
We need an implementation of UnscaledCycleClock::Frequency() now that these are not the same thing. The code in this repo may not call that method, but code exists that does.
https://riscv.org/wp-content/uploads/2016/06/riscv-spec-v2.1.pdf says
The RDTIME pseudo-instruction reads the low XLEN bits of the time CSR, which counts wall-clock
real time that has passed from an arbitrary start time in the past. RDTIMEH is an RV32I-only instruction that reads bits 63–32 of the same real-time counter. The underlying 64-bit counter should
never overflow in practice. The execution environment should provide a means of determining the
period of the real-time counter (seconds/tick). The period must be constant. The real-time clocks
of all hardware threads in a single user application should be synchronized to within one tick of the
real-time clock. The environment should provide a means to determine the accuracy of the clock.
I have no idea how to get that.
If you are just trying to get something to build, -DABSL_USE_UNSCALED_CYCLECLOCK=0 should work.
There was a problem hiding this comment.
The frequency is implementation dependent, and there is no easy way to get it. Could it be measured instead? Basically how it is done for the ABSL_INTERNAL_UNSCALED_CYCLECLOCK_FREQUENCY_IS_CPU_FREQUENCY?
aurel32
added a commit
to aurel32/abseil-cpp
that referenced
this pull request
Mar 19, 2024
Starting with Linux 6.6 [1], RDCYCLE is a privileged instruction on RISC-V and can't be used directly from userland. There is a sysctl option to change that as a transition period, but it will eventually disappear. The RDTIME instruction is another less accurate alternative, however its frequency varies from board to board, and there is currently now way to get its frequency from userland [2]. Therefore this patch just removes the code for unscaledcycleclock on RISC-V. Without processor specific implementation, abseil relies on std::chrono::steady_clock::now().time_since_epoch() which is basically a wrapper around clock_gettime (CLOCK_MONOTONIC), which in turns use __vdso_clock_gettime(). On RISC-V this VDSO is just a wrapper around RDTIME correctly scaled to use nanoseconds units. This fixes the testsuite on riscv64, tested on a VisionFive 2 board. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc4c07c89aada16229084eeb93895c95b7eabaa3 [2] abseil#1631
|
Closing in favor of #1644 |
copybara-service bot
pushed a commit
that referenced
this pull request
Mar 22, 2024
Imported from GitHub PR #1644 Starting with Linux 6.6 [1], RDCYCLE is a privileged instruction on RISC-V and can't be used directly from userland. There is a sysctl option to change that as a transition period, but it will eventually disappear. The RDTIME instruction is another less accurate alternative, however its frequency varies from board to board, and there is currently now way to get its frequency from userland [2]. Therefore this patch just removes the code for unscaledcycleclock on RISC-V. Without processor specific implementation, abseil relies on std::chrono::steady_clock::now().time_since_epoch() which is basically a wrapper around clock_gettime (CLOCK_MONOTONIC), which in turns use __vdso_clock_gettime(). On RISC-V this VDSO is just a wrapper around RDTIME correctly scaled to use nanoseconds units. This fixes the testsuite on riscv64, tested on a VisionFive 2 board. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc4c07c89aada16229084eeb93895c95b7eabaa3 [2] #1631 Merge 43356a2 into 76f8011 Merging this change closes #1644 COPYBARA_INTEGRATE_REVIEW=#1644 from aurel32:rv64-no-unscaledcycleclock 43356a2 PiperOrigin-RevId: 618286262 Change-Id: Ie4120a727e7d0bb185df6e06ea145c780ebe6652
netkex
pushed a commit
to netkex/abseil-cpp
that referenced
this pull request
Apr 3, 2024
Imported from GitHub PR abseil#1644 Starting with Linux 6.6 [1], RDCYCLE is a privileged instruction on RISC-V and can't be used directly from userland. There is a sysctl option to change that as a transition period, but it will eventually disappear. The RDTIME instruction is another less accurate alternative, however its frequency varies from board to board, and there is currently now way to get its frequency from userland [2]. Therefore this patch just removes the code for unscaledcycleclock on RISC-V. Without processor specific implementation, abseil relies on std::chrono::steady_clock::now().time_since_epoch() which is basically a wrapper around clock_gettime (CLOCK_MONOTONIC), which in turns use __vdso_clock_gettime(). On RISC-V this VDSO is just a wrapper around RDTIME correctly scaled to use nanoseconds units. This fixes the testsuite on riscv64, tested on a VisionFive 2 board. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc4c07c89aada16229084eeb93895c95b7eabaa3 [2] abseil#1631 Merge 43356a2 into 76f8011 Merging this change closes abseil#1644 COPYBARA_INTEGRATE_REVIEW=abseil#1644 from aurel32:rv64-no-unscaledcycleclock 43356a2 PiperOrigin-RevId: 618286262 Change-Id: Ie4120a727e7d0bb185df6e06ea145c780ebe6652
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Starting with Linux 6.6 [1], RDCYCLE is a privileged instruction on RISC-V and can't be used directly from userland. There is a sysctl option to change that as a transition period, but it will eventually disappear.
Use RDTIME instead, which while less accurate has the advantage of being synchronized between CPU (and thus monotonic) and of constant frequency. Therefore this change also fixes another issue caused by wrong time measurement when a process is rescheduled from one CPU to another [2].
Given RDTIME uses a timer independent from the CPU, this patche removes the ABSL_INTERNAL_UNSCALED_CYCLECLOCK_FREQUENCY_IS_CPU_FREQUENCY define accordingly.
This fixes the testsuite on riscv64, tested on a VisionFive 2 board.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cc4c07c89aada16229084eeb93895c95b7eabaa3
[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025221