unscaledcycleclock: remove RISC-V support by aurel32 · Pull Request #1644 · abseil/abseil-cpp

aurel32 · 2024-03-19T20:51:25Z

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

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

derekmauro · 2024-03-20T15:07:45Z

We have internal clients that are using this code. It can't simply be removed.

In the other pull request I asked if you could build with -DABSL_USE_UNSCALED_CYCLECLOCK=0. Does that solve your problem?

derekmauro · 2024-03-20T16:14:29Z

I'm also going to have to track down the callers of this and see what they want to do.

aurel32 · 2024-03-20T18:06:35Z

We have internal clients that are using this code. It can't simply be removed.

Ok. Note that in addition to the 6.6 kernel issue, the use of rdcycle means it can't be use on multicore CPUs, unless pinning the process on a single core, as the controrary to rdtime, the rdcycle counters are not synchronised between cores and just "jumps" if the process is rescheduled on another core.

In the other pull request I asked if you could build with -DABSL_USE_UNSCALED_CYCLECLOCK=0. Does that solve your problem?

I guess you mean at the cmake level. It doesn't work and cmake outputs:

CMake Warning:
  Manually-specified variables were not used by the project:

    ABSL_USE_UNSCALED_CYCLECLOCK

derekmauro · 2024-03-22T14:36:03Z

I've decided to accept this change and keep the RISC-V code as an internal-only patch. I've also filed an internal bug (b/330872512) for the team that is using it to figure out what they want to do with it. This should be merged soon.

Re: ABSL_USE_UNSCALED_CYCLECLOCK - I was talking about a preprocessor define. There is no corresponding CMake option.

aurel32 · 2024-03-23T15:53:49Z

Thanks a lot @derekmauro

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

jrtc27 · 2024-09-13T15:48:05Z

RDCYCLE works just fine on FreeBSD; if you want to avoid RDCYCLE, and don't want to use RDTIME, on Linux then at least please instead change the #ifdefs to include non-Linux rather than penalise every OS out there.

jrtc27 · 2024-09-13T15:49:31Z

That or leave it in there and add defined(__riscv) && defined(__linux__) to ABSL_USE_UNSCALED_CYCLECLOCK_DEFAULT, which seems to exist entirely to support this kind of situation?

aurel32 · 2024-09-13T16:35:55Z

RDCYCLE works just fine on FreeBSD; if you want to avoid RDCYCLE, and don't want to use RDTIME, on Linux then at least please instead change the #ifdefs to include non-Linux rather than penalise every OS out there.

Do you mean that RDCYCLE is accessible from userland on FreeBSD? Or do you mean it works on FreeBSD the way it is used in abseil-cpp?

The other reason not to use RDCYCLE is that it is not monotonic if the task is moved between cores, causing the testsuite to fail: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1025221

jrtc27 · 2024-09-13T16:44:41Z

It is accessible from userland on FreeBSD. With the caveat of course that you be careful about core migration. For RDTIME there's an (ugly) sysctl available to get the frequency, too. Of course we do have a vDSO implementation of clock_gettime too that uses RDTIME under the hood so it's not awful to lack a faster implementation, I just don't like the idea of deleting code just because it doesn't work on only Linux.

Imported from GitHub PR abseil/abseil-cpp#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/abseil-cpp#1631 Merge 43356a2548cfde76e164d446cb69004b488c6a71 into 76f8011beabdaee872b5fde7546e02407b220cb1 Merging this change closes #1644 COPYBARA_INTEGRATE_REVIEW=abseil/abseil-cpp#1644 from aurel32:rv64-no-unscaledcycleclock 43356a2548cfde76e164d446cb69004b488c6a71 PiperOrigin-RevId: 618286262 Change-Id: Ie4120a727e7d0bb185df6e06ea145c780ebe6652 Link: https://github.com/riscv-forks/electron/raw/4eff53436a1a86ac548a107e21ca5078518833c0/patches/chromium/unscaledcycleclock-remove-riscv-support.patch Signed-off-by: Mingcong Bai <[email protected]>

This patch brings enough support for bootstraping bazel itself on riscv64 linux platform. Changes: * Bump rules_java for bazelbuild/rules_java#282 bazelbuild/rules_java#198 * Bump stardoc to 0.7.1 and rules_jvm_external to 6.1 to fix CI failure * Use portable `uname -m` in mininize_jdk.sh * Add prebuilt jdk for riscv64 * Bump abseil-cpp for abseil/abseil-cpp#1644 Tested on riscv 64 linux by `bazel build //src:bazel --compilation_mode=opt`

- Bump abseil-cpp for abseil/abseil-cpp#1644 , which fixes RDCYCLE SIGILL on linux >= 6.6. - Make the patch less likely to rot as we might keep it for a while.

This patch brings enough support for bootstraping bazel itself on riscv64 linux platform. Changes: * Bump rules_java for bazelbuild/rules_java#282 bazelbuild/rules_java#198 * Bump stardoc to 0.7.1 and rules_jvm_external to 6.2 to fix CI failure * Use portable `uname -m` in mininize_jdk.sh * Add prebuilt jdk for riscv64 * Bump abseil-cpp for abseil/abseil-cpp#1644 Tested on riscv 64 linux by `bazel build //src:bazel --compilation_mode=opt`

- Bump abseil-cpp for abseil/abseil-cpp#1644 , which fixes RDCYCLE SIGILL on linux >= 6.6. - Make the patch less likely to rot as we might keep it for a while.

This patch brings enough support for bootstraping bazel itself on riscv64 linux platform. Changes: * Bump rules_java for bazelbuild/rules_java#282 bazelbuild/rules_java#198 * Bump stardoc to 0.7.1 and rules_jvm_external to 6.2 to fix CI failure * Use portable `uname -m` in mininize_jdk.sh * Add prebuilt jdk for riscv64 * Bump abseil-cpp for abseil/abseil-cpp#1644 Tested on riscv 64 linux by `bazel build //src:bazel --compilation_mode=opt`

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

This patch brings enough support for bootstraping bazel itself on riscv64 linux platform. Changes: * Bump rules_java for bazelbuild/rules_java#282 bazelbuild/rules_java#198 * Bump stardoc to 0.7.1 and rules_jvm_external to 6.1 to fix CI failure * Use portable `uname -m` in mininize_jdk.sh * Add prebuilt jdk for riscv64 * Bump abseil-cpp for abseil/abseil-cpp#1644 Tested on riscv 64 linux by `bazel build //src:bazel --compilation_mode=opt`

aurel32 mentioned this pull request Mar 19, 2024

unscaledcycleclock: use RDTIME instead of RDCYCLE on RISC-V #1631

Closed

copybara-service bot closed this in 7335a36 Mar 22, 2024

derekmauro mentioned this pull request Apr 8, 2024

Replace rdcycle instruction with rdtime #1550

Closed

kxxt mentioned this pull request Jul 17, 2024

SIGILL on rdcycle, which becomes a privileged instruction since linux 6.6 riscv-forks/electron#2

Closed

3 tasks

nanonyme mentioned this pull request Sep 24, 2024

grpc hits SIGILL on riscv64 grpc/grpc#37791

Closed

kxxt mentioned this pull request Mar 28, 2025

[7.7.0] Add support for riscv64 linux bazelbuild/bazel#25699

Closed

kxxt added a commit to kxxt/archriscv-packages that referenced this pull request Mar 28, 2025

updpatch: bazel7 7.5.0-1

d9907f2

- Bump abseil-cpp for abseil/abseil-cpp#1644 , which fixes RDCYCLE SIGILL on linux >= 6.6. - Make the patch less likely to rot as we might keep it for a while.

kxxt mentioned this pull request Mar 29, 2025

updpatch: bazel7 7.5.0-1 felixonmars/archriscv-packages#4631

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unscaledcycleclock: remove RISC-V support#1644

unscaledcycleclock: remove RISC-V support#1644
aurel32 wants to merge 1 commit intoabseil:masterfrom
aurel32:rv64-no-unscaledcycleclock

aurel32 commented Mar 19, 2024

Uh oh!

derekmauro commented Mar 20, 2024

Uh oh!

derekmauro commented Mar 20, 2024

Uh oh!

aurel32 commented Mar 20, 2024

Uh oh!

derekmauro commented Mar 22, 2024

Uh oh!

aurel32 commented Mar 23, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

aurel32 commented Sep 13, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

aurel32 commented Mar 19, 2024

Uh oh!

derekmauro commented Mar 20, 2024

Uh oh!

derekmauro commented Mar 20, 2024

Uh oh!

aurel32 commented Mar 20, 2024

Uh oh!

derekmauro commented Mar 22, 2024

Uh oh!

aurel32 commented Mar 23, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

aurel32 commented Sep 13, 2024

Uh oh!

jrtc27 commented Sep 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants