-
Notifications
You must be signed in to change notification settings - Fork 4
Add Linux support and fix potential unsoundness #4
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
base: main
Are you sure you want to change the base?
Conversation
46f59a2 to
e1b9544
Compare
|
Note: it would be nice to set up basic CI: Build on Linux and presumably on Illumos too. While I could do the Linux CI, I have no clue about how you at Oxide have your Illumos CI set up (as that isn't a thing that Github normally supports, custom runners I presume?) I did test |
e1b9544 to
c638570
Compare
|
Looks like oxidecomputer/usdt#340 was recently merged that should provide support on Linux using usdt. Some of the other improvements are still relevant. I'll split those out into a separate PR in the coming days. |
Before, if tokio started allowing for Ids that were 0, this code would be unsound. Now it should be fully sound. Also the check happens at compile time, which is preferable to doing it at runtime.
c638570 to
9acae3f
Compare
|
@hawkw I have now changed approach entirely, since Linux support was recently added to the usdt crate. I'm guessing you don't watch this repo, since you didn't give any feedback. Sorry if I'm pinging you when that wasn't needed. Once this is merged it would be good to have a new release as well, which will be semver major due to removing one of the error cases: ❯ cargo semver-checks
Checking tokio-dtrace v0.1.1 -> v0.1.1 (no change; assume minor)
Checked [ 0.007s] 140 checks: 137 pass, 3 fail, 0 warn, 38 skip
--- failure enum_variant_missing: pub enum variant removed or renamed ---
Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_variant_missing.ron
Failed in:
variant RegistrationError::InvalidCasts, previously in file /home/arvid/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-dtrace-0.1.1/src/lib.rs:228
--- failure function_missing: pub fn removed or renamed ---
Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/function_missing.ron
Failed in:
function tokio_dtrace::check_casts, previously in file /home/arvid/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-dtrace-0.1.1/src/lib.rs:266
--- failure struct_missing: pub struct removed or renamed ---
Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_missing.ron
Failed in:
struct tokio_dtrace::InvalidCasts, previously in file /home/arvid/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-dtrace-0.1.1/src/lib.rs:246
Summary semver requires new major version: 3 major and 0 minor checks failed
Finished [ 26.089s] tokio-dtrace |
|
Might be a skill issue, or maybe a problem with this/usdt but I'm observing: and when I try to attach the following program I see the error It's most likely an issue with my setup/kernel version, but posting just in case. Will update if I find out more - for now I am going to fork and remove the worker probes for the sake of testing, as that seems to be the main source of issues atm (why?) `bpftrace --info`I am probably fighting a losing battle by running bpftrace inside EKS :D |
|
@conradludgate Huh, when I tested this previously it worked. The only gotcha I could think of would be forgetting Here are some scattered thoughts and ideas:
❯ eu-readelf -n target/debug/examples/basic
Note section [ 2] '.note.ABI-tag' of 32 bytes at offset 0x2fc:
Owner Data size Type
GNU 16 GNU_ABI_TAG
OS: Linux, ABI: 4.4.0
Note section [ 3] '.note.gnu.build-id' of 36 bytes at offset 0x31c:
Owner Data size Type
GNU 20 GNU_BUILD_ID
Build ID: 4a893a3ee89360920be79c56d76c0a11c3fbe437
Note section [43] '.note.stapsdt' of 664 bytes at offset 0x1c1c634:
Owner Data size Type
stapsdt 69 Version: 3
PC: 0xbcfab, Base: 0x6236a, Semaphore: 0x1ba7d0
Provider: tokio, Name: task-spawn, Args: '8@%rdi 8@%rsi 4@%edx 4@%ecx'
stapsdt 74 Version: 3
PC: 0xbd23b, Base: 0x6236a, Semaphore: 0x1ba7d2
Provider: tokio, Name: task-poll-start, Args: '8@%rdi 8@%rsi 4@%edx 4@%ecx'
stapsdt 72 Version: 3
PC: 0xbd4cb, Base: 0x6236a, Semaphore: 0x1ba7d4
Provider: tokio, Name: task-poll-end, Args: '8@%rdi 8@%rsi 4@%edx 4@%ecx'
stapsdt 73 Version: 3
PC: 0xbd75b, Base: 0x6236a, Semaphore: 0x1ba7d6
Provider: tokio, Name: task-terminate, Args: '8@%rdi 8@%rsi 4@%edx 4@%ecx'
stapsdt 51 Version: 3
PC: 0xbd7cc, Base: 0x6236a, Semaphore: 0x1ba7d8
Provider: tokio, Name: worker-thread-start, Args: ''
stapsdt 50 Version: 3
PC: 0xbd7fc, Base: 0x6236a, Semaphore: 0x1ba7da
Provider: tokio, Name: worker-thread-stop, Args: ''
stapsdt 50 Version: 3
PC: 0xbd82c, Base: 0x6236a, Semaphore: 0x1ba7dc
Provider: tokio, Name: worker-thread-park, Args: ''
stapsdt 52 Version: 3
PC: 0xbd85c, Base: 0x6236a, Semaphore: 0x1ba7de
Provider: tokio, Name: worker-thread-unpark, Args: ''We here see reasonable semaphore addresses listed. If you see those, but bpftrace see weird addresses like For reference (I use Arch Linux, which is relatively bleeding edge): ❯ sudo bpftrace --info
System
OS: Linux 6.17.1-zen1-1-zen #1 ZEN SMP PREEMPT_DYNAMIC Mon, 06 Oct 2025 18:48:15 +0000
Arch: x86_64
Build
version: v0.24.0
LLVM: 21.1.4
bfd: yes
libdw (DWARF support): yes
libsystemd (systemd notify support): yes
blazesym (advanced symbolization): yes
Kernel helpers
dpath: yes get_tai_ns: yes
get_func_ip: yes lookup_percpu_elem: yes
Kernel features
Instruction limit: 1000000 btf: yes
module btf: yes map batch: yes
Probe types
kprobe_multi: yes uprobe_multi: yes
kprobe_session: yes iter: yes |
|
As for differences between worker-* and other tracepoints: The worker tracepoints don't take arguments. I have no idea why that would be relevant though. (I don't really see a point of having a sempahore for a tracepoint without arguments, but that is up to the underlying |
|
Thanks for the tip. Updating bpftools did indeed do the trick and now the probes can attach. Although there's still the 0x11 issue. `eu-readelf -n ...` |
|
That's odd, in your output (please use proper markdown syntax to make it easier to read next time) I see
|
Not sure what github is doing. The syntax looks correct to me |
|
Seems you figured out the formatting. Still I would like to know the other things I mentioned: 1. Does it happen on the examples? 2. What about the versions? These would help narrow down the cause of the issue you are seeing. I could start to try to reproduce it with that info. |
|
I was able to find the root cause of the issue I was facing. It's nothing to do with this crate in particular but it's a combination of the asm that usdt crate produces on linux and a weird interaction with monomorphisation, codegen-units, and linker --gc-sections. I'll open an issue on the usdt crate. |
u64. There are two aspects to this:u64instead ofNonZeroU64: I believe the old code was unsound as it would cause UB if tokio ever changed to allow the 0 value for tasks (unlikely but theoretically possible).The new code is however still unsound if tokio would change to something like
(u32, u16)which keeps the same size but introduces padding. I'm looking for a solution to this (if one exists). See https://users.rust-lang.org/t/unsafe-unions-bit-pattern-validity/133366This PR closes issue #3.