On 02.08.24 11:31, Alice Ryhl wrote:
> diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
> new file mode 100644
> index 000000000000..69dafdb8bfe8
> --- /dev/null
> +++ b/rust/kernel/tracepoint.rs
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Logic for tracepoints.
> +
> +/// Declare the Rust entry point for a tracepoint.
> +///
> +/// This macro generates an unsafe function that calls into C, and its safety requirements will be
> +/// whatever the relevant C code requires. To document these safety requirements, you may add
> +/// doc-comments when invoking the macro.
I think we should mandate safety documentation for the function.
> +#[macro_export]
> +macro_rules! declare_trace {
> + ($($(#[$attr:meta])* $pub:vis fn $name:ident($($argname:ident : $argtyp:ty),* $(,)?);)*) => {$(
Can you add an `unsafe` in front of `fn`, since this macro generates an
`unsafe` function? Otherwise I don't see how the SAFETY comment below is
correct.
---
Cheers,
Benno
> + $( #[$attr] )*
> + #[inline(always)]
> + $pub unsafe fn $name($($argname : $argtyp),*) {
> + #[cfg(CONFIG_TRACEPOINTS)]
> + {
> + // SAFETY: It's always okay to query the static key for a tracepoint.
> + let should_trace = unsafe {
> + $crate::macros::paste! {
> + $crate::static_key::static_key_false!(
> + $crate::bindings::[< __tracepoint_ $name >],
> + $crate::bindings::tracepoint,
> + key
> + )
> + }
> + };
> +
> + if should_trace {
> + $crate::macros::paste! {
> + // SAFETY: The caller guarantees that it is okay to call this tracepoint.
> + unsafe { $crate::bindings::[< rust_do_trace_ $name >]($($argname),*) };
> + }
> + }
> + }
> +
> + #[cfg(not(CONFIG_TRACEPOINTS))]
> + {
> + // If tracepoints are disabled, insert a trivial use of each argument
> + // to avoid unused argument warnings.
> + $( let _unused = $argname; )*
> + }
> + }
> + )*}
> +}
> +
> +pub use declare_trace;
>
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
>
