Skip to content
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

Experimental tracepoint support #1084

Open
wants to merge 3 commits into
base: rust-next
Choose a base branch
from

Conversation

Darksonn
Copy link
Collaborator

No description provided.

rust/kernel/static_call.rs Show resolved Hide resolved
rust/kernel/static_key.rs Show resolved Hide resolved
};

if should_trace {
// TODO: cpu_online(raw_smp_processor_id())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the ramifications of not having this? If the CPU is offline, will we hang, or just delay?

rust/kernel/tracepoint.rs Outdated Show resolved Hide resolved
rust/bindings/rwonce.rs Outdated Show resolved Hide resolved
rust/kernel/tracepoint.rs Outdated Show resolved Hide resolved
rust/kernel/static_call.rs Outdated Show resolved Hide resolved
.quad {0} + {1} - .
.popsection

2: mov {2:e}, 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may want to use att_syntax ;-)

Copy link
Collaborator Author

@Darksonn Darksonn May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you show me how to do that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options(att_syntax), here is an example:

unsafe fn i32_xadd(v: *mut i32, mut i: i32) -> i32 {
    // SAFETY: Per function safety requirement, the address of `v` is valid for "xadd".
    unsafe {
        asm!(
            lock_instr!("xaddl {i:e}, ({v})"),
            i = inout(reg) i,
            v = in(reg) v,
            options(att_syntax, preserves_flags),
        );
    }

    i
}

in https://lore.kernel.org/rust-for-linux/[email protected]/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kernel developers will prefer that, yeah... :)

Sadly, specifying every single time that may be painful. Perhaps we could have our own asm! that expands to the actual one plus that option, if that is possible, or otherwise it would be nice to have a rustc flag to always use that by default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to our wishlist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A asm! wrapper should be fairly straightforward. The asm macro deliberately allows multiple options to simplify cases where people want to always add options(att_syntax). See rust-lang/rust#73227.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Gary, then we should probably do it soon if we are going to start adding more asm! blocks. I would still like to have a flag (or similar) nevertheless, to avoid extra macros and to match the C compilers.

rust/kernel/static_key.rs Outdated Show resolved Hide resolved
@ojeda ojeda mentioned this pull request May 31, 2024
39 tasks
rust/bindings/rwonce.rs Outdated Show resolved Hide resolved
rust/helpers.c Outdated Show resolved Hide resolved
rust/kernel/static_call.rs Outdated Show resolved Hide resolved
@Darksonn Darksonn force-pushed the tracepoint branch 2 times, most recently from a528226 to 1062c5b Compare June 4, 2024 15:11
Darksonn added 3 commits June 6, 2024 11:06
Add static_call support by mirroring how C does. When the platform does
not support static calls (right now, that means that it is not x86),
then the function pointer is loaded from a global and called. Otherwise,
we generate a call to a trampoline function, and objtool is used to make
these calls patchable at runtime.

Signed-off-by: Alice Ryhl <[email protected]>
Add just enough support for static key so that we can use it from
tracepoints. Tracepoints rely on `static_key_false` even though it is
deprecated, so we add the same functionality to Rust.

Signed-off-by: Alice Ryhl <[email protected]>
Make it possible to have Rust code call into tracepoints defined by C
code. It is still required that the tracepoint is declared in a C
header, and that this header is included in the input to bindgen.

Signed-off-by: Alice Ryhl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants