2024-06-10 14:03:17

by Alice Ryhl

[permalink] [raw]
Subject: [PATCH v2 0/2] Tracepoints and static branch in Rust

An important part of a production ready Linux kernel driver is
tracepoints. So to write production ready Linux kernel drivers in Rust,
we must be able to call tracepoints from Rust code. This patch series
adds support for calling tracepoints declared in C from Rust.

To use the tracepoint support, you must:

1. Declare the tracepoint in a C header file as usual.

2. Add #define CREATE_RUST_TRACE_POINTS next to your
#define CREATE_TRACE_POINTS.

2. Make sure that the header file is visible to bindgen.

3. Use the declare_trace! macro in your Rust code to generate Rust
functions that call into the tracepoint.

For example, the kernel has a tracepoint called `sched_kthread_stop`. It
is declared like this:

TRACE_EVENT(sched_kthread_stop,
TP_PROTO(struct task_struct *t),
TP_ARGS(t),
TP_STRUCT__entry(
__array( char, comm, TASK_COMM_LEN )
__field( pid_t, pid )
),
TP_fast_assign(
memcpy(__entry->comm, t->comm, TASK_COMM_LEN);
__entry->pid = t->pid;
),
TP_printk("comm=%s pid=%d", __entry->comm, __entry->pid)
);

To call the above tracepoint from Rust code, you must first ensure that
the Rust helper for the tracepoint is generated. To do this, you would
modify kernel/sched/core.c by adding #define CREATE_RUST_TRACE_POINTS.

Next, you would include include/trace/events/sched.h in
rust/bindings/bindings_helper.h so that the exported C functions are
visible to Rust, and then you would declare the tracepoint in Rust:

declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}

This will define an inline Rust function that checks the static key,
calling into rust_do_trace_##name if the tracepoint is active. Since
these tracepoints often take raw pointers as arguments, it may be
convenient to wrap it in a safe wrapper:

mod raw {
declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}
}

#[inline]
pub fn trace_sched_kthread_stop(task: &Task) {
// SAFETY: The pointer to `task` is valid.
unsafe { raw::sched_kthread_stop(task.as_raw()) }
}

A future expansion of the tracepoint support could generate these safe
versions automatically, but that is left as future work for now.

This is intended for use in the Rust Binder driver, which was originally
sent as an RFC [1]. The RFC did not include tracepoint support, but you
can see how it will be used in Rust Binder at [2]. The author has
verified that the tracepoint support works on Android devices.

This implementation implements support for static keys in Rust so that
the actual static branch happens in the Rust object file. However, the
__DO_TRACE body remains in C code. See v1 for an implementation where
__DO_TRACE is also implemented in Rust.

Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl <[email protected]>
---
Changes in v2:
- Call into C code for __DO_TRACE.
- Drop static_call patch, as it is no longer needed.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Alice Ryhl (2):
rust: add static_key_false
rust: add tracepoint support

include/linux/tracepoint.h | 18 ++++++++-
include/trace/define_trace.h | 7 ++++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 2 +
rust/kernel/static_key.rs | 87 +++++++++++++++++++++++++++++++++++++++++
rust/kernel/tracepoint.rs | 47 ++++++++++++++++++++++
scripts/Makefile.build | 2 +-
7 files changed, 162 insertions(+), 2 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471

Best regards,
--
Alice Ryhl <[email protected]>



2024-06-10 14:14:20

by Alice Ryhl

[permalink] [raw]
Subject: [PATCH v2 1/2] rust: add static_key_false

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.

It is not possible to use the existing C implementation of
arch_static_branch because it passes the argument `key` to inline
assembly as an 'i' parameter, so any attempt to add a C helper for this
function will fail to compile because the value of `key` must be known
at compile-time.

Signed-off-by: Alice Ryhl <[email protected]>
---
rust/kernel/lib.rs | 1 +
rust/kernel/static_key.rs | 87 +++++++++++++++++++++++++++++++++++++++++++++++
scripts/Makefile.build | 2 +-
3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fbd91a48ff8b..a0be9544996d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@
pub mod prelude;
pub mod print;
mod static_assert;
+pub mod static_key;
#[doc(hidden)]
pub mod std_vendor;
pub mod str;
diff --git a/rust/kernel/static_key.rs b/rust/kernel/static_key.rs
new file mode 100644
index 000000000000..6c3dbe14c98a
--- /dev/null
+++ b/rust/kernel/static_key.rs
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+
+use crate::bindings::*;
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "x86_64")]
+macro_rules! _static_key_false {
+ ($key:path, $keytyp:ty, $field:ident) => {'my_label: {
+ core::arch::asm!(
+ r#"
+ 1: .byte 0x0f,0x1f,0x44,0x00,0x00
+
+ .pushsection __jump_table, "aw"
+ .balign 8
+ .long 1b - .
+ .long {0} - .
+ .quad {1} + {2} - .
+ .popsection
+ "#,
+ label {
+ break 'my_label true;
+ },
+ sym $key,
+ const ::core::mem::offset_of!($keytyp, $field),
+ );
+
+ break 'my_label false;
+ }};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "aarch64")]
+macro_rules! _static_key_false {
+ ($key:path, $keytyp:ty, $field:ident) => {'my_label: {
+ core::arch::asm!(
+ r#"
+ 1: nop
+
+ .pushsection __jump_table, "aw"
+ .align 3
+ .long 1b - ., {0} - .
+ .quad {1} + {2} - .
+ .popsection
+ "#,
+ label {
+ break 'my_label true;
+ },
+ sym $key,
+ const ::core::mem::offset_of!($keytyp, $field),
+ );
+
+ break 'my_label false;
+ }};
+}
+
+/// Branch based on a static key.
+///
+/// Takes three arguments:
+///
+/// * `key` - the path to the static variable containing the `static_key`.
+/// * `keytyp` - the type of `key`.
+/// * `field` - the name of the field of `key` that contains the `static_key`.
+#[macro_export]
+macro_rules! static_key_false {
+ // Forward to the real implementation. Separated like this so that we don't have to duplicate
+ // the documentation.
+ ($key:path, $keytyp:ty, $field:ident) => {{
+ // Assert that `$key` has type `$keytyp` and that `$key.$field` has type `static_key`.
+ //
+ // SAFETY: We know that `$key` is a static because otherwise the inline assembly will not
+ // compile. The raw pointers created in this block are in-bounds of `$key`.
+ static _TY_ASSERT: () = unsafe {
+ let key: *const $keytyp = ::core::ptr::addr_of!($key);
+ let _: *const $crate::bindings::static_key = ::core::ptr::addr_of!((*key).$field);
+ };
+
+ $crate::static_key::_static_key_false! { $key, $keytyp, $field }
+ }};
+}
+
+pub use {_static_key_false, static_key_false};
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index efacca63c897..60197c1c063f 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -263,7 +263,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
# Compile Rust sources (.rs)
# ---------------------------------------------------------------------------

-rust_allowed_features := new_uninit
+rust_allowed_features := asm_const,asm_goto,new_uninit

# `--out-dir` is required to avoid temporaries being created by `rustc` in the
# current working directory, which may be not accessible in the out-of-tree

--
2.45.2.505.gda0bf45e8d-goog


2024-06-10 14:29:05

by Alice Ryhl

[permalink] [raw]
Subject: [PATCH v2 2/2] rust: add tracepoint support

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]>
---
include/linux/tracepoint.h | 18 +++++++++++++++-
include/trace/define_trace.h | 7 ++++++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 47 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 689b6d71590e..d82af4d77c9f 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -238,6 +238,20 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
#define __DECLARE_TRACE_RCU(name, proto, args, cond)
#endif

+/*
+ * Declare an exported function that Rust code can call to trigger this
+ * tracepoint. This function does not include the static branch; that is done
+ * in Rust to avoid a function call when the tracepoint is disabled.
+ */
+#define DEFINE_RUST_DO_TRACE(name, proto, args)
+#define DEFINE_RUST_DO_TRACE_REAL(name, proto, args) \
+ notrace void rust_do_trace_##name(proto) \
+ { \
+ __DO_TRACE(name, \
+ TP_ARGS(args), \
+ cpu_online(raw_smp_processor_id()), 0); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -253,6 +267,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
extern int __traceiter_##name(data_proto); \
DECLARE_STATIC_CALL(tp_func_##name, __traceiter_##name); \
extern struct tracepoint __tracepoint_##name; \
+ extern void rust_do_trace_##name(proto); \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -337,7 +352,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
void __probestub_##_name(void *__data, proto) \
{ \
} \
- DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
+ DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name); \
+ DEFINE_RUST_DO_TRACE(_name, TP_PROTO(proto), TP_ARGS(args))

#define DEFINE_TRACE(name, proto, args) \
DEFINE_TRACE_FN(name, NULL, NULL, PARAMS(proto), PARAMS(args));
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index 00723935dcc7..b47cc036acba 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -72,6 +72,13 @@
#define DECLARE_TRACE(name, proto, args) \
DEFINE_TRACE(name, PARAMS(proto), PARAMS(args))

+/* If requested, create helpers for calling these tracepoints from Rust. */
+#ifdef CREATE_RUST_TRACE_POINTS
+#undef DEFINE_RUST_DO_TRACE
+#define DEFINE_RUST_DO_TRACE(name, proto, args) \
+ DEFINE_RUST_DO_TRACE_REAL(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ddb5644d4fd9..d442f9ccfc2c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -15,6 +15,7 @@
#include <linux/refcount.h>
#include <linux/sched.h>
#include <linux/slab.h>
+#include <linux/tracepoint.h>
#include <linux/wait.h>
#include <linux/workqueue.h>

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index a0be9544996d..96f8f11c51f2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -45,6 +45,7 @@
pub mod sync;
pub mod task;
pub mod time;
+pub mod tracepoint;
pub mod types;
pub mod workqueue;

diff --git a/rust/kernel/tracepoint.rs b/rust/kernel/tracepoint.rs
new file mode 100644
index 000000000000..1005f09e0330
--- /dev/null
+++ b/rust/kernel/tracepoint.rs
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for tracepoints.
+
+/// Declare the Rust entry point for a tracepoint.
+#[macro_export]
+macro_rules! declare_trace {
+ ($($(#[$attr:meta])* $pub:vis fn $name:ident($($argname:ident : $argtyp:ty),* $(,)?);)*) => {$(
+ $( #[$attr] )*
+ #[inline(always)]
+ $pub unsafe fn $name($($argname : $argtyp),*) {
+ #[cfg(CONFIG_TRACEPOINTS)]
+ {
+ use $crate::bindings::*;
+
+ // 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!(
+ [< __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 { [< 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.45.2.505.gda0bf45e8d-goog


2024-06-12 15:04:27

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] rust: add static_key_false

On Mon, Jun 10, 2024 at 02:01:04PM +0000, Alice Ryhl wrote:
> 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.
>
> It is not possible to use the existing C implementation of
> arch_static_branch because it passes the argument `key` to inline
> assembly as an 'i' parameter, so any attempt to add a C helper for this
> function will fail to compile because the value of `key` must be known
> at compile-time.
>
> Signed-off-by: Alice Ryhl <[email protected]>

> +#[cfg(target_arch = "x86_64")]

> +#[cfg(target_arch = "aarch64")]

This patch breaks the build on riscv (and I assume loongarch):

error[E0432]: unresolved import `_static_key_false`
--> /stuff/linux/rust/kernel/static_key.rs:87:10
|
87 | pub use {_static_key_false, static_key_false};
| ^^^^^^^^^^^^^^^^^ no external crate `_static_key_false`

Cheers,
Conor.


Attachments:
(No filename) (0.99 kB)
signature.asc (235.00 B)
Download all attachments