[PATCH 0/3] Tracepoints and static branch/call 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. Make sure that the header file is visible to bindgen so that Rust
bindings are generated for the symbols that the tracepoint macro
emits.
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 would add the relevant
header file to rust/bindings/bindings_helper.h and add the following
invocation somewhere in your Rust code:
declare_trace! {
fn sched_kthread_stop(task: *mut task_struct);
}
This will define a Rust function of the given name that you can call
like any other Rust function. 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 will end up in the Rust object file. However,
it would also be possible to just wrap the trace_##name generated by
__DECLARE_TRACE in an extern C function and then call that from Rust.
This will simplify the Rust code by removing the need for static
branches and calls, but it places the static branch behind an external
call, which has performance implications.
A possible middle ground would be to place just the __DO_TRACE body in
an extern C function and to implement the Rust wrapper by doing the
static branch in Rust, and then calling into C the code that contains
__DO_TRACE when the tracepoint is active. However, this would need some
changes to include/linux/tracepoint.h to generate and export a function
containing the body of __DO_TRACE when the tracepoint should be callable
from Rust.
So in general, there is a tradeoff between placing parts of the
tracepoint (which is perf sensitive) behind an external call, and having
code duplicated in both C and Rust (which must be kept in sync when
changes are made). This is an important point that I would like feedback
on from the C maintainers.
Link:
https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3110088 [2]
Signed-off-by: Alice Ryhl
---
Alice Ryhl (3):
rust: add static_call support
rust: add static_key_false
rust: add tracepoint support
rust/bindings/bindings_helper.h | 1 +
rust/bindings/lib.rs| 15 +++
rust/helpers.c | 24 +++
rust/kernel/lib.rs | 3 ++
rust/kernel/static_call.rs | 92 +
rust/kernel/static_key.rs | 87 ++
rust/kernel/tracepoint.rs | 92 +
scripts/Makefile.build | 2 +-
8 files changed, 315 insertions(+), 1 deletion(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471
Best regards,
--
Alice Ryhl
[PATCH 3/3] 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
---
rust/bindings/bindings_helper.h | 1 +
rust/bindings/lib.rs| 15 +++
rust/helpers.c | 24 +++
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 92 +
5 files changed, 133 insertions(+)
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
#include
#include
+#include
#include
#include
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 40ddaee50d8b..48856761d682 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -48,3 +48,18 @@ mod bindings_helper {
}
pub use bindings_raw::*;
+
+/// Rust version of the C macro `rcu_dereference_raw`.
+///
+/// The rust helper only works with void pointers, but this wrapper method
makes it work with any
+/// pointer type using pointer casts.
+///
+/// # Safety
+///
+/// This method has the same safety requirements as the C macro of the same
name.
+#[inline(always)]
+pub unsafe fn rcu_dereference_raw(p: *const *mut T) -> *mut T {
+// SAFETY: This helper calls into the C macro, so the caller promises to
uphold the safety
+// requirements.
+unsafe { __rcu_dereference_raw(p as *mut *mut _) as *mut T }
+}
diff --git a/rust/helpers.c b/rust/helpers.c
index 2c37a0f5d7a8..0560cc2a512a 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -165,6 +165,30 @@ rust_helper_krealloc(const void *objp, size_t new_size,
gfp_t flags)
}
EXPORT_SYMBOL_GPL(rust_helper_krealloc);
+void rust_helper_preempt_enable_notrace(void)
+{
+ preempt_enable_notrace();
+}
+EXPORT_SYMBOL_GPL(rust_helper_preempt_enable_notrace);
+
+void rust_helper_preempt_disable_notrace(void)
+{
+ preempt_disable_notrace();
+}
+EXPORT_SYMBOL_GPL(rust_helper_preempt_disable_notrace);
+
+bool rust_helper_current_cpu_online(void)
+{
+ return cpu_online(raw_smp_processor_id());
+}
+EXPORT_SYMBOL_GPL(rust_helper_current_cpu_online);
+
+void *rust_helper___rcu_dereference_raw(void **p)
+{
+ return rcu_dereference_raw(p);
+}
+EXPORT_SYMBOL_GPL(rust_helper___rcu_dereference_raw);
+
/*
* `bindgen` binds the C `size_t` type as the Rust `usize` type, so we can
* use it in contexts where Rust expects a `usize` like slice (array) indices.
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 22e1fedd0774..3f3b280bb437 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,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 ..d628ae71fc58
--- /dev/null
+++ b/rust/kernel/tracepoint.rs
@@ -0,0 +1,92 @@
+// 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: This macro only compiles if $name is a real
tracepoint, and if it is a
+// real tracepoint, then it is okay to query the static key.
+let should_trace = unsafe {
+$crate::macros::paste! {
+$crate::static_key::static_key_false!(
+[< __tracepoint_ $name >],
+$crate::bindings::tracepoint,
+key
+)
+}
+};
+
+if should_trace {
+$crate::tracepoint::do_trace!($name($($argname :
$argtyp),*), cond);
+}
+}
+
+#[cfg(not(CONFIG_TRACEPOINTS))]
+{
+// If tracepoints are disabled, insert a trivial use of each
argument
+// to avoid unused argument warnings.
+$( let _unused = $argname; )*
+}
+}
+)*}
+}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! do_trace {
+($name:ident($($argname:ident : $argtyp:ty),* $(,)?), $cond:expr) => {{
+if !$crate::bindings::current_cpu_online() {
+return;
+}
+
+// SAFETY: This call is balanced with the call below.
+unsafe { $crate::bindings::preempt_disable_notrace() };
+
+// SAFETY:
[PATCH 1/3] rust: add static_call support
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
---
rust/kernel/lib.rs | 1 +
rust/kernel/static_call.rs | 92 ++
2 files changed, 93 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fbd91a48ff8b..d534b1178955 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_call;
#[doc(hidden)]
pub mod std_vendor;
pub mod str;
diff --git a/rust/kernel/static_call.rs b/rust/kernel/static_call.rs
new file mode 100644
index ..f7b8ba7bf1fb
--- /dev/null
+++ b/rust/kernel/static_call.rs
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static calls.
+
+#[macro_export]
+#[doc(hidden)]
+macro_rules! ty_underscore_for {
+($arg:expr) => {
+_
+};
+}
+
+#[doc(hidden)]
+#[repr(transparent)]
+pub struct AddressableStaticCallKey {
+_ptr: *const bindings::static_call_key,
+}
+unsafe impl Sync for AddressableStaticCallKey {}
+impl AddressableStaticCallKey {
+pub const fn new(ptr: *const bindings::static_call_key) -> Self {
+Self { _ptr: ptr }
+}
+}
+
+#[cfg(CONFIG_HAVE_STATIC_CALL)]
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _static_call {
+($name:ident($($args:expr),* $(,)?)) => {{
+// Symbol mangling will give this symbol a unique name.
+#[cfg(CONFIG_HAVE_STATIC_CALL_INLINE)]
+#[link_section = ".discard.addressable"]
+#[used]
+static __ADDRESSABLE: $crate::static_call::AddressableStaticCallKey =
unsafe {
+
$crate::static_call::AddressableStaticCallKey::new(::core::ptr::addr_of!(
+$crate::macros::paste! { $crate::bindings:: [<__SCK__ $name
>]; }
+))
+};
+
+let fn_ptr: unsafe extern "C"
fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
+$crate::macros::paste! { $crate::bindings:: [<__SCT__ $name >]; };
+(fn_ptr)($($args),*)
+}};
+}
+
+#[cfg(not(CONFIG_HAVE_STATIC_CALL))]
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _static_call {
+($name:ident($($args:expr),* $(,)?)) => {{
+let void_ptr_fn: *mut ::core::ffi::c_void =
+$crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >];
}.func;
+
+let fn_ptr: unsafe extern "C"
fn($($crate::static_call::ty_underscore_for!($args)),*) -> _ =
+if true {
+::core::mem::transmute(void_ptr_fn)
+} else {
+// This is dead code, but it influences type inference on
`fn_ptr` so that we
+// transmute the function pointer to the right type.
+$crate::macros::paste! { $crate::bindings:: [<__SCT__ $name
>]; }
+};
+
+(fn_ptr)($($args),*)
+}};
+}
+
+/// Statically call a global function.
+///
+/// # Safety
+///
+/// This macro will call the provided function. It is up to the caller to
uphold the safety
+/// guarantees of the function.
+///
+/// # Examples
+///
+/// ```ignore
+/// fn call_static() {
+/// unsafe {
+/// static_call! { your_static_call() };
+/// }
+/// }
+/// ```
+#[macro_export]
+macro_rules! static_call {
+// Forward to the real implementation. Separated like this so that we
don't have to duplicate
+// the documentation.
+($($args:tt)*) => { $crate::static_call::_static_call! { $($args)* } };
+}
+
+pub use {_static_call, static_call, ty_underscore_for};
--
2.45.2.505.gda0bf45e8d-goog
[PATCH 2/3] 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.
Signed-off-by: Alice Ryhl
---
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 d534b1178955..22e1fedd0774 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -39,6 +39,7 @@
pub mod print;
mod static_assert;
pub mod static_call;
+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 ..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
Re: [PATCH 0/3] Tracepoints and static branch/call in Rust
On Thu, Jun 6, 2024 at 5:25 PM Mathieu Desnoyers wrote: > > On 2024-06-06 11:05, Alice Ryhl wrote: > > This implementation implements support for static keys in Rust so that > > the actual static branch will end up in the Rust object file. However, > > it would also be possible to just wrap the trace_##name generated by > > __DECLARE_TRACE in an extern C function and then call that from Rust. > > This will simplify the Rust code by removing the need for static > > branches and calls, but it places the static branch behind an external > > call, which has performance implications. > > The tracepoints try very hard to minimize overhead of dormant tracepoints > so it is not frowned-upon to have them built into production binaries. > This is needed to make sure distribution vendors keep those tracepoints > in the kernel binaries that reach end-users. > > Adding a function call before evaluation of the static branch goes against > this major goal. > > > > > A possible middle ground would be to place just the __DO_TRACE body in > > an extern C function and to implement the Rust wrapper by doing the > > static branch in Rust, and then calling into C the code that contains > > __DO_TRACE when the tracepoint is active. However, this would need some > > changes to include/linux/tracepoint.h to generate and export a function > > containing the body of __DO_TRACE when the tracepoint should be callable > > from Rust. > > This tradeoff is more acceptable than having a function call before > evaluation of the static branch, but I wonder what is the upside of > this tradeoff compared to inlining the whole __DO_TRACE in Rust ? > > > So in general, there is a tradeoff between placing parts of the > > tracepoint (which is perf sensitive) behind an external call, and having > > code duplicated in both C and Rust (which must be kept in sync when > > changes are made). This is an important point that I would like feedback > > on from the C maintainers. > > I don't see how the duplication happens there: __DO_TRACE is meant to be > inlined into each C tracepoint caller site, so the code is already meant > to be duplicated. Having an explicit function wrapping the tracepoint > for Rust would just create an extra instance of __DO_TRACE if it happens > to be also inlined into C code. > > Or do you meant you would like to prevent having to duplicate the > implementation of __DO_TRACE in both C and Rust ? > > I'm not sure if you mean to prevent source code duplication between > C and Rust or duplication of binary code (instructions). It's a question of maintenance burden. If you change how __DO_TRACE is implemented, then those changes must also be reflected in the Rust version. There's no issue in the binary code. Alice
Re: [PATCH 3/3] rust: add tracepoint support
On Thu, Jun 6, 2024 at 5:29 PM Mathieu Desnoyers
wrote:
>
> On 2024-06-06 11:05, Alice Ryhl wrote:
> > 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
>
> [...]
>
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index 2c37a0f5d7a8..0560cc2a512a 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -165,6 +165,30 @@ rust_helper_krealloc(const void *objp, size_t
> > new_size, gfp_t flags)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_krealloc);
> >
> > +void rust_helper_preempt_enable_notrace(void)
> > +{
> > + preempt_enable_notrace();
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_preempt_enable_notrace);
> > +
> > +void rust_helper_preempt_disable_notrace(void)
> > +{
> > + preempt_disable_notrace();
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_preempt_disable_notrace);
> > +
> > +bool rust_helper_current_cpu_online(void)
> > +{
> > + return cpu_online(raw_smp_processor_id());
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_current_cpu_online);
> > +
> > +void *rust_helper___rcu_dereference_raw(void **p)
> > +{
> > + return rcu_dereference_raw(p);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper___rcu_dereference_raw);
>
> Ouch. Doing a function call for each of those small operations will
> have a rather large performance impact when tracing is active. If it is
> not possible to inline those in Rust, then implementing __DO_TRACE in
> a C function would at least allow Rust to only do a single call to C
> rather than go back and forth between Rust and C.
>
> What prevents inlining those helpers in Rust ?
There's nothing fundamental that prevents it. But they contain a large
amount of architecture specific code, which makes them a significant
amount of work to reimplement in Rust.
For example, rcu_dereference_raw calls into READ_ONCE. READ_ONCE is
usually a volatile load, but under arm64+LTO, you get a bunch of
inline assembly that relies on ALTERNATIVE for feature detection:
https://elixir.bootlin.com/linux/v6.9/source/arch/arm64/include/asm/rwonce.h#L36
And preempt_enable_notrace has a similar story.
The solution that Boqun mentions is nice, but it relies on rustc and
clang using the same version of LLVM. You are unlikely to have
compilers with matching LLVMs unless you intentionally take steps to
make that happen.
But yes, perhaps these helpers are an argument to have a single call
for __DO_TRACE instead.
Alice
Re: [PATCH 2/3] rust: add static_key_false
On Thu, Jun 6, 2024 at 5:37 PM Mathieu Desnoyers wrote: > > On 2024-06-06 11:05, 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. > > > > Signed-off-by: Alice Ryhl > > --- > > 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 d534b1178955..22e1fedd0774 100644 > > --- a/rust/kernel/lib.rs > > +++ b/rust/kernel/lib.rs > > @@ -39,6 +39,7 @@ > > pub mod print; > > mod static_assert; > > pub mod static_call; > > +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 ..6c3dbe14c98a > > --- /dev/null > > +++ b/rust/kernel/static_key.rs > > @@ -0,0 +1,87 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > This static key code is something that can be generally useful > both in kernel and user-space. Is there anything that would prevent > licensing this under MIT right away instead so it could eventually be > re-used more broadly in userspace as well ? I would not mind. Alice
Re: [PATCH 1/3] rust: add static_call support
Peter Zijlstra wrote:
> On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote:
> > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra wrote:
> > >
> > > This is absolutely unreadable gibberish -- how am I supposed to keep
> > > this in sync with the rest of the static_call infrastructure?
> >
> > Yeah, they are macros, which look different from "normal" Rust code.
>
> Macros like CPP ?
Yes, this patch series uses declarative macros, which are the closest
that Rust has to the C preprocessor. They are powerful, but just like
CPP, they can become pretty complicated and hard to read if you are
doing something non-trivial.
The macro_rules! block is how you define a new declarative macro.
The ($name:ident($($args:expr),* $(,)?)) part defines the arguments to
the declarative macro. This syntax means:
1. The input starts with any identifier, which we call $name.
2. Then there must be a ( token.
3. Then the $(),* part defines a repetition group. The contents inside
the parenthesises are what is being repeated. The comma means that
repetitions are separated by commas. The asterisk means that the
contents may be repeated any number of times, including zero times.
(Alternatives are + which means repeated at least once, and ? which
means that the contents may be repeated zero or one time.)
4. The contents of the repetition group will be an expression, which we
call $args.
5. After the last expression, we have $(,)? which means that you can
have zero or one commas after the last expression. Rust usually
allows trailing commas in lists, which is why I included it.
6. And finally, you must close the input with a ) token.
So for example, you might invoke the macro like this:
static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct));
Here, $name will be tp_func_my_tracepoint, and the repetition group is
repeated twice, with $args first corresponding to `__data` and then to
`&mut my_task_struct` when the macro is expanded. The $(,)? group is
repeated zero times.
Inside the macro, you will see things such as:
$crate::macros::paste! { $crate::bindings:: [<__SCK__ $name >]; }
The Rust syntax for invoking a macro has an exclamation mark after the
name, so you know that $crate::macros::paste is a macro. The `paste`
macro just emits its input unchanged, except that any identifiers
between [< and >] are concatenated into a single identifier. So if $name
is my_static_key, then the above invocation of paste! emits:
$crate::bindings::__SCK__my_static_key;
The $crate::bindings module is where the output of bindgen goes, so this
should correspond to the C symbol called __SCK__my_static_key.
> > Is there something we could do to help here? I think Alice and others
> > would be happy to explain how it works and/or help maintain it in the
> > future if you prefer.
>
> Write a new language that looks more like C -- pretty please ? :-)
>
> Mostly I would just really like you to just use arm/jump_label.h,
> they're inline functions and should be doable with IR, no weirdo CPP
> involved in this case.
I assume that you're referring to static_key_false here? I don't think
that function can be exposed using IR because it passes the function
argument called key as an "i" argument to an inline assembly block. Any
attempt to compile static_key_false without knowing the value of key at
compile time will surely fail to compile with the
invalid operand for inline asm constraint 'i'
error.
Alice
[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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
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/20240606-tracepoint-v1-0-6551627bf...@google.com
---
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
[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
---
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 ..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
[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
---
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
#include
#include
+#include
#include
#include
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 ..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 stat
[PATCH v3 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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
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/20240606-tracepoint-v1-0-6551627bf...@google.com
---
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 | 143
rust/kernel/tracepoint.rs | 47 +
scripts/Makefile.build | 2 +-
7 files changed, 218 insertions(+), 2 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471
Best regards,
--
Alice Ryhl
[PATCH v3 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
---
rust/kernel/lib.rs| 1 +
rust/kernel/static_key.rs | 143 ++
scripts/Makefile.build| 2 +-
3 files changed, 145 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 ..9c844fe3e3a3
--- /dev/null
+++ b/rust/kernel/static_key.rs
@@ -0,0 +1,143 @@
+// 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;
+}};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "loongarch64")]
+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;
+}};
+}
+
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "riscv64")]
+macro_rules! _static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {'my_label: {
+core::arch::asm!(
+r#"
+.align 2
+.option push
+.option norelax
+.option norvc
+1: nop
+.option pop
+.pushsection __jump_table, "aw"
+.align 3
+.long 1b - ., {0} - .
+.dword {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
`$
[PATCH v3 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
---
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
#include
#include
+#include
#include
#include
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 ..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 stat
Re: [PATCH v2 1/2] rust: add static_key_false
On Wed, Jun 12, 2024 at 5:03 PM Conor Dooley wrote:
>
> On Mon, Jun 10, 2024 at 02:01:04PM +, 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
>
> > +#[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.
Thank you. Fixed in v3.
https://lore.kernel.org/all/20240621-tracepoint-v3-0-9e44eeea2...@google.com/
Alice
Re: [PATCH v3 2/2] rust: add tracepoint support
On Fri, Jun 21, 2024 at 12:35 PM Alice Ryhl wrote:
>
> 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
> ---
> 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
Hmm, I tried using the support where I have both events and hooks:
#define CREATE_TRACE_POINTS
#define CREATE_RUST_TRACE_POINTS
#include
#include
But it's not really working. Initially I thought that it's because I
need to undef DEFINE_RUST_DO_TRACE at the end of this file, but even
when I added that, I still get this error:
error: redefinition of 'str__rust_binder__trace_system_name'
Is the Rust support missing something, or is the answer just that you
can't have two files of the same name like this? Or am I doing
something else wrong?
Alice
Re: [PATCH v3 2/2] rust: add tracepoint support
On Tue, Jun 25, 2024 at 8:11 PM Boqun Feng wrote: > > On Fri, Jun 21, 2024 at 02:52:10PM +0200, Alice Ryhl wrote: > [...] > > > > Hmm, I tried using the support where I have both events and hooks: > > > > #define CREATE_TRACE_POINTS > > #define CREATE_RUST_TRACE_POINTS > > #include > > #include > > > > But it's not really working. Initially I thought that it's because I > > need to undef DEFINE_RUST_DO_TRACE at the end of this file, but even > > when I added that, I still get this error: > > > > error: redefinition of 'str__rust_binder__trace_system_name' > > > > Is the Rust support missing something, or is the answer just that you > > can't have two files of the same name like this? Or am I doing > > something else wrong? > > > > Because your hooks/rust_binder.h and events/rust_binder.h use the same > TRACE_SYSTEM name? Could you try something like: > > #define TRACE_SYSTEM rust_binder_hook > > in your hooks/rust_binder.h? I was able to get it to work by moving the includes into two different .c files. I don't think changing TRACE_SYSTEM works because it must match the filename. Alice
Re: [PATCH v3 1/2] rust: add static_key_false
On Tue, Jun 25, 2024 at 6:18 PM Boqun Feng wrote: > > Hi Alice, > > On Fri, Jun 21, 2024 at 10:35:26AM +, 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 > > [Add linux-arch, and related arch maintainers Cced] > > Since inline asms are touched here, please consider copying linux-arch > and arch maintainers next time ;-) Will do. > For x86_64 and arm64 bits: > > Acked-by: Boqun Feng > > One thing though, we should split the arch-specific impls into different > files, for example: rust/kernel/arch/arm64.rs or rust/arch/arm64.rs. > That'll be easier for arch maintainers to watch the Rust changes related > to a particular architecture. Is that how you would prefer to name these files? You don't want static_key somewhere in the filename? > Another thought is that, could you implement an arch_static_branch!() > (instead of _static_key_false!()) and use it for static_key_false!() > similar to what we have in C? The benefit is that at least for myself > it'll be easier to compare the implementation between C and Rust. I can try to include that. Alice
[PATCH v4 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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v4:
- Move arch-specific code into rust/kernel/arch.
- Restore DEFINE_RUST_DO_TRACE at end of define_trace.h
- Link to v3:
https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
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/20240606-tracepoint-v1-0-6551627bf...@google.com
---
Alice Ryhl (2):
rust: add static_key_false
rust: add tracepoint support
include/linux/tracepoint.h | 18 +++-
include/trace/define_trace.h | 12
rust/bindings/bindings_helper.h | 1 +
rust/kernel/arch/arm64/jump_label.rs | 34 +++
rust/kernel/arch/loongarch/jump_label.rs | 35
rust/kernel/arch/mod.rs | 24
rust/kernel/arch/riscv/jump_label.rs | 38 ++
rust/kernel/arch/x86/jump_label.rs | 35
rust/kernel/lib.rs | 3 ++
rust/kernel/static_key.rs| 32 ++
rust/kernel/tracepoint.rs| 47
scripts/Makefile.build | 2 +-
12 files changed, 279 insertions(+), 2 deletions(-)
---
base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
change-id: 20240606-tracepoint-31e15b90e471
Best regards,
--
Alice Ryhl
[PATCH v4 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
---
rust/kernel/arch/arm64/jump_label.rs | 34
rust/kernel/arch/loongarch/jump_label.rs | 35 +
rust/kernel/arch/mod.rs | 24
rust/kernel/arch/riscv/jump_label.rs | 38
rust/kernel/arch/x86/jump_label.rs | 35 +
rust/kernel/lib.rs | 2 ++
rust/kernel/static_key.rs| 32 +++
scripts/Makefile.build | 2 +-
8 files changed, 201 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/arch/arm64/jump_label.rs
b/rust/kernel/arch/arm64/jump_label.rs
new file mode 100644
index ..5eede2245718
--- /dev/null
+++ b/rust/kernel/arch/arm64/jump_label.rs
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Arm64 Rust implementation of jump_label.h
+
+/// arm64 implementation of arch_static_branch
+#[macro_export]
+#[cfg(target_arch = "aarch64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+.pushsection __jump_table, "aw"
+.align 3
+.long 1b - ., {0} - .
+.quad {1} + {2} + {3} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+const $crate::arch::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+pub use arch_static_branch;
diff --git a/rust/kernel/arch/loongarch/jump_label.rs
b/rust/kernel/arch/loongarch/jump_label.rs
new file mode 100644
index ..8d31318aeb11
--- /dev/null
+++ b/rust/kernel/arch/loongarch/jump_label.rs
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Loongarch Rust implementation of jump_label.h
+
+/// loongarch implementation of arch_static_branch
+#[doc(hidden)]
+#[macro_export]
+#[cfg(target_arch = "loongarch64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+.pushsection __jump_table, "aw"
+.align 3
+.long 1b - ., {0} - .
+.quad {1} + {2} + {3} - .
+.popsection
+"#,
+label {
+break 'my_label true;
+},
+sym $key,
+const ::core::mem::offset_of!($keytyp, $field),
+const $crate::arch::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+pub use arch_static_branch;
diff --git a/rust/kernel/arch/mod.rs b/rust/kernel/arch/mod.rs
new file mode 100644
index ..14271d2530e9
--- /dev/null
+++ b/rust/kernel/arch/mod.rs
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Architecture specific code.
+
+#[cfg_attr(target_arch = "aarch64", path = "arm64")]
+#[cfg_attr(target_arch = "x86_64", path = "x86")]
+#[cfg_attr(target_arch = "loongarch64", path = "loongarch")]
+#[cfg_attr(target_arch = "riscv64", path = "riscv")]
+mod inner {
+pub mod jump_label;
+}
+
+pub use self::inner::*;
+
+/// A helper used by inline assembly to pass a boolean to as a `const`
parameter.
+///
+/// Using this function instead of a cast lets you assert that the input is a
boolean, rather than
+/// some other type that can be cast to an integer.
+#[doc(hidden)]
+pub const fn bool_to_int(b: bool) -> i32 {
+b as i32
+}
diff --git a/rust/kernel/arch/riscv/jump_label.rs
b/rust/kernel/arch/riscv/jump_label.rs
new file mode 100644
index ..2672e0c6f033
--- /dev/null
+++ b/rust/kernel/arch/riscv/jump_label.rs
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! RiscV Rust implementation of jump_label.h
+
+/// riscv implementation of arch_static_branch
+#[macro_export]
+#[cfg(target_arch = "riscv64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident,
[PATCH v4 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
---
include/linux/tracepoint.h | 18 +++-
include/trace/define_trace.h| 12 +++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 47 +
5 files changed, 78 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..08ed5ce63a96 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
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
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
#include
#include
+#include
#include
#include
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fffd4e1dd1c1..9ae90eb69020 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,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 ..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 :
$argty
Re: [PATCH v3 2/2] rust: add tracepoint support
On Wed, Jun 26, 2024 at 8:43 PM Steven Rostedt wrote: > > On Wed, 26 Jun 2024 10:48:23 +0200 > Alice Ryhl wrote: > > > > > > > Because your hooks/rust_binder.h and events/rust_binder.h use the same > > > TRACE_SYSTEM name? Could you try something like: > > > > > > #define TRACE_SYSTEM rust_binder_hook > > > > > > in your hooks/rust_binder.h? > > > > I was able to get it to work by moving the includes into two different > > .c files. I don't think changing TRACE_SYSTEM works because it must > > match the filename. > > Try to use: > > #define TRACE_SYSTEM_VAR rust_binder_hook_other_name > > in one. Then that is used as the variable for that file. Thanks. I also made a change to restore the value of DEFINE_RUST_DO_TRACE after define_trace.h Alice
Re: [PATCH v4 1/2] rust: add static_key_false
On Wed, Jul 31, 2024 at 7:05 PM Peter Zijlstra wrote:
>
> On Fri, Jun 28, 2024 at 01:23:31PM +, Alice Ryhl wrote:
>
> > rust/kernel/arch/arm64/jump_label.rs | 34
> > rust/kernel/arch/loongarch/jump_label.rs | 35 +
> > rust/kernel/arch/mod.rs | 24
> > rust/kernel/arch/riscv/jump_label.rs | 38
> >
> > rust/kernel/arch/x86/jump_label.rs | 35 +
> > rust/kernel/lib.rs | 2 ++
> > rust/kernel/static_key.rs| 32 +++
> > scripts/Makefile.build | 2 +-
> > 8 files changed, 201 insertions(+), 1 deletion(-)
>
> So I really find the amount of duplicated asm offensive. Is is far too
> easy for any of this to get out of sync.
>
> > diff --git a/rust/kernel/arch/x86/jump_label.rs
> > b/rust/kernel/arch/x86/jump_label.rs
> > new file mode 100644
> > index ..383bed273c50
> > --- /dev/null
> > +++ b/rust/kernel/arch/x86/jump_label.rs
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2024 Google LLC.
> > +
> > +//! X86 Rust implementation of jump_label.h
> > +
> > +/// x86 implementation of arch_static_branch
> > +#[macro_export]
> > +#[cfg(target_arch = "x86_64")]
> > +macro_rules! arch_static_branch {
> > +($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'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} + {3} - .
> > +.popsection
> > +"#,
> > +label {
> > +break 'my_label true;
> > +},
> > +sym $key,
> > +const ::core::mem::offset_of!($keytyp, $field),
> > +const $crate::arch::bool_to_int($branch),
> > +);
> > +
> > +break 'my_label false;
> > +}};
> > +}
>
> Note that this uses the forced 5 byte version, and not the dynamic sized
> one. On top of that it hard-codes the nop5 string :/
>
> Please work harder to not have to duplicate stuff like this.
I really didn't want to duplicate it, but it's very hard to find a
performant alternative. Is there any way we could accept duplication
only in the cases where an 'i' parameter is used? I don't have the
choice of using a Rust helper for 'i' parameters.
Perhaps one option could be to put the Rust code inside jump_label.h
and have the header file evaluate to either C or Rust depending on the
value of some #ifdefs?
#ifndef RUST_ASM
/* existing C code goes here */
#endif
#ifdef RUST_ASM
// rust code goes here
#endif
That way the duplication is all in a single file. It would also avoid
the need for duplicating the nop5 string, as the Rust case is still
going through the C preprocessor and can use the existing #define.
I'm also open to other alternatives. But I don't have infinite
resources to drive major language changes.
Alice
Re: [PATCH v4 1/2] rust: add static_key_false
On Thu, Aug 1, 2024 at 12:28 PM Peter Zijlstra wrote:
>
> On Wed, Jul 31, 2024 at 11:34:13PM +0200, Alice Ryhl wrote:
>
> > > Please work harder to not have to duplicate stuff like this.
> >
> > I really didn't want to duplicate it, but it's very hard to find a
> > performant alternative. Is there any way we could accept duplication
> > only in the cases where an 'i' parameter is used? I don't have the
> > choice of using a Rust helper for 'i' parameters.
> >
> > Perhaps one option could be to put the Rust code inside jump_label.h
> > and have the header file evaluate to either C or Rust depending on the
> > value of some #ifdefs?
> >
> > #ifndef RUST_ASM
> > /* existing C code goes here */
> > #endif
> > #ifdef RUST_ASM
> > // rust code goes here
> > #endif
> >
> > That way the duplication is all in a single file. It would also avoid
> > the need for duplicating the nop5 string, as the Rust case is still
> > going through the C preprocessor and can use the existing #define.
>
> I suppose that is slightly better, but ideally you generate the whole of
> the Rust thing from the C version. After all, Clang can already parse
> this.
>
> That said, with the below patch, I think you should be able to reuse the
> JUMP_TABLE_ENTRY macro like:
>
> JUMP_TABLE_ENTRY({0}, {1}, {2} + {3})
Yeah, I think this can work. I will submit a follow-up patch that
removes the duplication soon.
Alice
[PATCH v5 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! {
/// # Safety
/// `task` must point at a valid task for the duration
/// of this call.
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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v5:
- Update first patch regarding inline asm duplication.
- Add __rust_do_trace helper to support conditions.
- Rename DEFINE_RUST_DO_TRACE_REAL to __DEFINE_RUST_DO_TRACE.
- Get rid of glob-import in tracepoint macro.
- Address safety requirements on tracepoints in docs.
- Link to v4:
https://lore.kernel.org/rust-for-linux/20240628-tracepoint-v4-0-353d523a9...@google.com
Changes in v4:
- Move arch-specific code into rust/kernel/arch.
- Restore DEFINE_RUST_DO_TRACE at end of define_trace.h
- Link to v3:
https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
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/20240606-tracepoint-v1-0-6551627bf...@google.com
---
Alice Ryhl (2):
rust: add static_key_false
rust: add tracepoint support
arch/arm64/include/asm/jump_label.h | 1 +
arch/loongarch/include/asm/jump_label.h | 1 +
arch/riscv/include/asm/jump_label.h | 1 +
arch/x86/include/asm/jump_label.h| 1 +
include/linux/tracepoint.h | 22 +-
include/trace/define_trace.h | 12
rust/bindings/bindings_helper.h | 1 +
rust/kernel/arch/arm64/jump_label.rs | 34
[PATCH v5 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.
One disadvantage of this patch is that it introduces a fair amount of
duplicated inline assembly. However, this is a limited and temporary
situation:
1. Most inline assembly has no reason to be duplicated like this. It is
only needed here due to the use of 'i' parameters.
2. Alice will submit a patch along the lines of [1] that removes the
duplication. This will happen as a follow-up to this patch series.
Comments are added to the C side as a reminder to change the asm on the
Rust side in case the C side is changed. For simplicity, the x86 asm
uses the forced 5 byte version. This will be fixed together with the
upcoming removal of the duplication.
Reviewed-by: Gary Guo
Tested-by: WANG Rui
Link:
https://lore.kernel.org/rust-for-linux/20240801102804.gq33...@noisy.programming.kicks-ass.net/
[1]
Signed-off-by: Alice Ryhl
---
arch/arm64/include/asm/jump_label.h | 1 +
arch/loongarch/include/asm/jump_label.h | 1 +
arch/riscv/include/asm/jump_label.h | 1 +
arch/x86/include/asm/jump_label.h| 1 +
rust/kernel/arch/arm64/jump_label.rs | 34
rust/kernel/arch/loongarch/jump_label.rs | 35 +
rust/kernel/arch/mod.rs | 24
rust/kernel/arch/riscv/jump_label.rs | 38
rust/kernel/arch/x86/jump_label.rs | 35 +
rust/kernel/lib.rs | 2 ++
rust/kernel/static_key.rs| 32 +++
scripts/Makefile.build | 2 +-
12 files changed, 205 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index 4e753908b801..56c106669f99 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -15,6 +15,7 @@
#define JUMP_LABEL_NOP_SIZEAARCH64_INSN_SIZE
+/* Changes to this asm must be reflected in
`rust/kernel/arch/arm64/jump_label.rs` */
#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
".align 3\n\t" \
diff --git a/arch/loongarch/include/asm/jump_label.h
b/arch/loongarch/include/asm/jump_label.h
index 29acfe3de3fa..3bdf0d834e8c 100644
--- a/arch/loongarch/include/asm/jump_label.h
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -13,6 +13,7 @@
#define JUMP_LABEL_NOP_SIZE4
+/* Changes to this asm must be reflected in
`rust/kernel/arch/loongarch/jump_label.rs` */
#define JUMP_TABLE_ENTRY \
".pushsection __jump_table, \"aw\"\n\t" \
".align3 \n\t" \
diff --git a/arch/riscv/include/asm/jump_label.h
b/arch/riscv/include/asm/jump_label.h
index 1c768d02bd0c..8ed63f588243 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -16,6 +16,7 @@
#define JUMP_LABEL_NOP_SIZE 4
+/* Changes to this asm must be reflected in
`rust/kernel/arch/riscv/jump_label.rs` */
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
{
diff --git a/arch/x86/include/asm/jump_label.h
b/arch/x86/include/asm/jump_label.h
index cbbef32517f0..687fff131afe 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -12,6 +12,7 @@
#include
#include
+/* Changes to this asm must be reflected in
`rust/kernel/arch/x86/jump_label.rs` */
#define JUMP_TABLE_ENTRY \
".pushsection __jump_table, \"aw\" \n\t" \
_ASM_ALIGN "\n\t" \
diff --git a/rust/kernel/arch/arm64/jump_label.rs
b/rust/kernel/arch/arm64/jump_label.rs
new file mode 100644
index ..5eede2245718
--- /dev/null
+++ b/rust/kernel/arch/arm64/jump_label.rs
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Arm64 Rust implementation of jump_label.h
+
+/// arm64 implementation of arch_static_branch
+#[macro_export]
+#[cfg(target_arch = "aarch64")]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+core::arch::asm!(
+r#"
+1: nop
+
+
[PATCH v5 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.
Instead of calling __DO_TRACE directly, the exported rust_do_trace_
function calls an inline helper function. This is because the `cond`
argument does not exist at the callsite of DEFINE_RUST_DO_TRACE.
__DECLARE_TRACE always emits an inline static and an extern declaration
that is only used when CREATE_RUST_TRACE_POINTS is set. These should not
end up in the final binary so it is not a problem that they sometimes
are emitted without a user.
Reviewed-by: Carlos Llamas
Signed-off-by: Alice Ryhl
---
include/linux/tracepoint.h | 22 +-
include/trace/define_trace.h| 12 ++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 49 +
5 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6be396bb4297..5042ca588e41 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -237,6 +237,18 @@ 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(name, proto, args) \
+ notrace void rust_do_trace_##name(proto)\
+ { \
+ __rust_do_trace_##name(args); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -252,6 +264,13 @@ 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 __rust_do_trace_##name(proto)\
+ { \
+ __DO_TRACE(name,\
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 0); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -336,7 +355,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..8159294c2041 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(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index b940a5777330..142ee71ae7c4 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
Re: [PATCH v5 1/2] rust: add static_key_false
On Fri, Aug 2, 2024 at 11:40 AM Peter Zijlstra wrote: > > On Fri, Aug 02, 2024 at 09:31:27AM +, 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. > > > > One disadvantage of this patch is that it introduces a fair amount of > > duplicated inline assembly. However, this is a limited and temporary > > situation: > > > > 1. Most inline assembly has no reason to be duplicated like this. It is > >only needed here due to the use of 'i' parameters. > > > > 2. Alice will submit a patch along the lines of [1] that removes the > >duplication. This will happen as a follow-up to this patch series. > > You're talking about yourself in the 3rd person? I'm not sure if commit messages are supposed to be a personal message from me, or an impersonal description of the patch. I'm happy to reword. > What's the rush, why not do it right first? Well for one, this version of the series mostly just makes changes to the second patch. But also, maybe I'm being too aggressive about it, but I have large amounts of out-of-tree code that I've been working on upstreaming, and it's a lot of work to keep it all up-to-date and rebased. I want to reduce it as much as I can. I was hoping that I could get the changes to include/linux/tracepoint.h off my plate, even if more work is needed on the static_key side of things. Alice
[PATCH v6 0/5] 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! {
/// # Safety
/// `task` must point at a valid task for the duration
/// of this call.
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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v6:
- Add support for !CONFIG_JUMP_LABEL.
- Add tracepoint to rust_print sample.
- Deduplicate inline asm.
- Require unsafe inside `declare_trace!`.
- Fix bug on x86 due to use of intel syntax.
- Link to v5:
https://lore.kernel.org/r/20240802-tracepoint-v5-0-faa164494...@google.com
Changes in v5:
- Update first patch regarding inline asm duplication.
- Add __rust_do_trace helper to support conditions.
- Rename DEFINE_RUST_DO_TRACE_REAL to __DEFINE_RUST_DO_TRACE.
- Get rid of glob-import in tracepoint macro.
- Address safety requirements on tracepoints in docs.
- Link to v4:
https://lore.kernel.org/rust-for-linux/20240628-tracepoint-v4-0-353d523a9...@google.com
Changes in v4:
- Move arch-specific code into rust/kernel/arch.
- Restore DEFINE_RUST_DO_TRACE at end of define_trace.h
- Link to v3:
https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
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/20240606-tracepoint-v1-0-6551627bf...@google.com
---
Alice Ryhl (5):
rust: add generic static_key_false
rust: add tracepoint support
rust: samples: add tracepoint to Rust sample
jump_label: adjust inline asm to be consistent
rust: add
[PATCH v6 1/5] rust: add generic 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.
This patch only provides a generic implementation without code patching
(matching the one used when CONFIG_JUMP_LABEL is disabled). Later
patches add support for inline asm implementations that use runtime
patching.
When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
function, so a Rust helper is defined for `static_key_count` in this
case. If Rust is compiled with LTO, this call should get inlined. The
helper can be eliminated once we have the necessary inline asm to make
atomic operations from Rust.
Signed-off-by: Alice Ryhl
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers.c| 9 +
rust/kernel/arch_static_branch_asm.rs | 1 +
rust/kernel/jump_label.rs | 29 +
rust/kernel/lib.rs| 1 +
5 files changed, 41 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index b940a5777330..8fd092e1b809 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/rust/helpers.c b/rust/helpers.c
index 92d3c03ae1bd..5a9bf5209cd8 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -133,6 +134,14 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
}
EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
+#ifndef CONFIG_JUMP_LABEL
+int rust_helper_static_key_count(struct static_key *key)
+{
+ return static_key_count(key);
+}
+EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
+#endif
+
__force void *rust_helper_ERR_PTR(long err)
{
return ERR_PTR(err);
diff --git a/rust/kernel/arch_static_branch_asm.rs
b/rust/kernel/arch_static_branch_asm.rs
new file mode 100644
index ..958f1f130455
--- /dev/null
+++ b/rust/kernel/arch_static_branch_asm.rs
@@ -0,0 +1 @@
+::kernel::concat_literals!("1: jmp " "{l_yes}" " # objtool NOPs this \n\t"
".pushsection __jump_table, \"aw\" \n\t" " " ".balign 8" " " "\n\t" ".long 1b
- . \n\t" ".long " "{l_yes}" "- . \n\t" " " ".quad" " " " " "{symb} + {off} +
{branch}" " - . \n\t" ".popsection \n\t")
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
new file mode 100644
index ..011e1fc1d19a
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+//!
+//! C header:
[`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
+
+/// 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`.
+///
+/// # Safety
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {{
+let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
+
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}};
+}
+pub use static_key_false;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 274bdc1b0a82..91af9f75d121 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
pub mod firmware;
pub mod init;
pub mod ioctl;
+pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
#[cfg(CONFIG_NET)]
--
2.46.0.76.ge559c4bf1a-goog
[PATCH v6 2/5] 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.
Instead of calling __DO_TRACE directly, the exported rust_do_trace_
function calls an inline helper function. This is because the `cond`
argument does not exist at the callsite of DEFINE_RUST_DO_TRACE.
__DECLARE_TRACE always emits an inline static and an extern declaration
that is only used when CREATE_RUST_TRACE_POINTS is set. These should not
end up in the final binary so it is not a problem that they sometimes
are emitted without a user.
Reviewed-by: Carlos Llamas
Signed-off-by: Alice Ryhl
---
include/linux/tracepoint.h | 22 +-
include/trace/define_trace.h| 12 ++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 49 +
5 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6be396bb4297..5042ca588e41 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -237,6 +237,18 @@ 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(name, proto, args) \
+ notrace void rust_do_trace_##name(proto)\
+ { \
+ __rust_do_trace_##name(args); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -252,6 +264,13 @@ 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 __rust_do_trace_##name(proto)\
+ { \
+ __DO_TRACE(name,\
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 0); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -336,7 +355,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..8159294c2041 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(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8fd092e1b809..fc6f94729789 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
[PATCH v6 3/5] rust: samples: add tracepoint to Rust sample
This updates the Rust printing sample to invoke a tracepoint. This
ensures that we have a user in-tree from the get-go even though the
patch is being merged before its real user.
Signed-off-by: Alice Ryhl
---
MAINTAINERS| 1 +
include/trace/events/rust_sample.h | 31 +++
rust/bindings/bindings_helper.h| 1 +
samples/rust/Makefile | 3 ++-
samples/rust/rust_print.rs | 18 ++
samples/rust/rust_print_events.c | 8
6 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8766f3e5e87e..465ca809ced4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19920,6 +19920,7 @@ C: zulip://rust-for-linux.zulipchat.com
P: https://rust-for-linux.com/contributing
T: git https://github.com/Rust-for-Linux/linux.git rust-next
F: Documentation/rust/
+F: include/trace/events/rust_sample.h
F: rust/
F: samples/rust/
F: scripts/*rust*
diff --git a/include/trace/events/rust_sample.h
b/include/trace/events/rust_sample.h
new file mode 100644
index ..dbc80ca2e465
--- /dev/null
+++ b/include/trace/events/rust_sample.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tracepoints for `samples/rust/rust_print.rs`.
+ *
+ * Copyright (C) 2024 Google, Inc.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rust_sample
+
+#if !defined(_RUST_SAMPLE_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _RUST_SAMPLE_TRACE_H
+
+#include
+
+TRACE_EVENT(rust_sample_loaded,
+ TP_PROTO(int magic_number),
+ TP_ARGS(magic_number),
+ TP_STRUCT__entry(
+ __field(int, magic_number)
+ ),
+ TP_fast_assign(
+ __entry->magic_number = magic_number;
+ ),
+ TP_printk("magic=%d", __entry->magic_number)
+);
+
+#endif /* _RUST_SAMPLE_TRACE_H */
+
+/* This part must be outside protection */
+#include
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fc6f94729789..fe97256afe65 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
/* `bindgen` gets confused at certain things. */
const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..f29280ec4820 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+ccflags-y += -I$(src) # needed for trace events
obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o
-obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
rust_print_events.o
subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 6eabb0d79ea3..6d14b08cac1c 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -69,6 +69,8 @@ fn init(_module: &'static ThisModule) -> Result {
arc_print()?;
+trace::trace_rust_sample_loaded(42);
+
Ok(RustPrint)
}
}
@@ -78,3 +80,19 @@ fn drop(&mut self) {
pr_info!("Rust printing macros sample (exit)\n");
}
}
+
+mod trace {
+use core::ffi::c_int;
+
+kernel::declare_trace! {
+/// # Safety
+///
+/// Always safe to call.
+unsafe fn rust_sample_loaded(magic: c_int);
+}
+
+pub(crate) fn trace_rust_sample_loaded(magic: i32) {
+// SAFETY: Always safe to call.
+unsafe { rust_sample_loaded(magic as c_int) }
+}
+}
diff --git a/samples/rust/rust_print_events.c b/samples/rust/rust_print_events.c
new file mode 100644
index ..a9169ff0edf1
--- /dev/null
+++ b/samples/rust/rust_print_events.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ */
+
+#define CREATE_TRACE_POINTS
+#define CREATE_RUST_TRACE_POINTS
+#include
--
2.46.0.76.ge559c4bf1a-goog
[PATCH v6 4/5] jump_label: adjust inline asm to be consistent
To avoid duplication of inline asm between C and Rust, we need to
import the inline asm from the relevant `jump_label.h` header into Rust.
To make that easier, this patch updates the header files to expose the
inline asm via a new ARCH_STATIC_BRANCH_ASM macro.
The header files are all updated to define a ARCH_STATIC_BRANCH_ASM that
takes the same arguments in a consistent order so that Rust can use the
same logic for every architecture.
Link: https://lore.kernel.org/r/20240725183325.122827-7-oj...@kernel.org [1]
Signed-off-by: Alice Ryhl
---
arch/arm/include/asm/jump_label.h | 14 +
arch/arm64/include/asm/jump_label.h | 20 -
arch/loongarch/include/asm/jump_label.h | 16 +++
arch/riscv/include/asm/jump_label.h | 50 ++---
arch/x86/include/asm/jump_label.h | 38 ++---
5 files changed, 75 insertions(+), 63 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h
b/arch/arm/include/asm/jump_label.h
index e4eb54f6cd9f..a35aba7f548c 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -9,13 +9,17 @@
#define JUMP_LABEL_NOP_SIZE 4
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1:\n\t"\
+ WASM(nop) "\n\t"\
+ ".pushsection __jump_table, \"aw\"\n\t"\
+ ".word 1b, " label ", " key "\n\t" \
+ ".popsection\n\t" \
+
static __always_inline bool arch_static_branch(struct static_key *key, bool
branch)
{
- asm goto("1:\n\t"
-WASM(nop) "\n\t"
-".pushsection __jump_table, \"aw\"\n\t"
-".word 1b, %l[l_yes], %c0\n\t"
-".popsection\n\t"
+ asm goto(ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
: : "i" (&((char *)key)[branch]) : : l_yes);
return false;
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index a0a5bbae7229..424ed421cd97 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -19,10 +19,14 @@
#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
".align 3\n\t" \
- ".long 1b - ., %l["#label"] - .\n\t" \
- ".quad %c0 - .\n\t"\
- ".popsection\n\t" \
- : : "i"(key) : : label
+ ".long 1b - ., " label " - .\n\t" \
+ ".quad " key " - .\n\t"\
+ ".popsection\n\t"
+
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: nop\n\t"\
+ JUMP_TABLE_ENTRY(key, label)
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
@@ -30,8 +34,8 @@ static __always_inline bool arch_static_branch(struct
static_key * const key,
char *k = &((char *)key)[branch];
asm goto(
- "1: nop \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
@@ -43,9 +47,11 @@ static __always_inline bool arch_static_branch_jump(struct
static_key * const ke
const bool branch)
{
char *k = &((char *)key)[branch];
+
asm goto(
"1: b %l[l_yes] \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ JUMP_TABLE_ENTRY("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
l_yes:
diff --git a/arch/loongarch/include/asm/jump_label.h
b/arch/loongarch/include/asm/jump_label.h
index 29acfe3de3fa..8a924bd69d19 100644
--- a/arch/loongarch/include/asm/jump_label.h
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -13,18 +13,22 @@
#define JUMP_LABEL_NOP_SIZE4
-#define JUMP_TABLE_ENTRY \
+/* This macro is also expanded on the Rust side. */
+#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
[PATCH v6 5/5] rust: add arch_static_branch
To allow the Rust implementation of static_key_false to use runtime code
patching instead of the generic implementation, pull in the relevant
inline assembly from the jump_label.h header by running the C
preprocessor on a .rs.S file. Build rules are added for .rs.S files.
Since the relevant inline asm has been adjusted to export the inline asm
via the ARCH_STATIC_BRANCH_ASM macro in a consistent way, the Rust side
does not need architecture specific code to pull in the asm.
It is not possible to use the existing C implementation of
arch_static_branch via a Rust helper because it passes the argument
`key` to inline assembly as an 'i' parameter. 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.
Suggested-by: Peter Zijlstra
Co-developed-by: Miguel Ojeda
Signed-off-by: Miguel Ojeda
Signed-off-by: Alice Ryhl
---
rust/Makefile | 5 ++-
rust/kernel/.gitignore | 3 ++
rust/kernel/arch_static_branch_asm.rs.S | 7
rust/kernel/jump_label.rs | 64 -
rust/kernel/lib.rs | 30
scripts/Makefile.build | 9 -
6 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 199e0db67962..277fcef656b8 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -14,6 +14,8 @@ CFLAGS_REMOVE_helpers.o = -Wmissing-prototypes
-Wmissing-declarations
always-$(CONFIG_RUST) += libmacros.so
no-clean-files += libmacros.so
+always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) +=
kernel/arch_static_branch_asm.rs
+
always-$(CONFIG_RUST) += bindings/bindings_generated.rs
bindings/bindings_helpers_generated.rs
obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
always-$(CONFIG_RUST) += exports_alloc_generated.h
exports_bindings_generated.h \
@@ -409,7 +411,8 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
-$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
+ $(obj)/kernel/arch_static_branch_asm.rs FORCE
+$(call if_changed_rule,rustc_library)
endif # CONFIG_RUST
diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
new file mode 100644
index ..d082731007c6
--- /dev/null
+++ b/rust/kernel/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+/arch_static_branch_asm.rs
diff --git a/rust/kernel/arch_static_branch_asm.rs.S
b/rust/kernel/arch_static_branch_asm.rs.S
new file mode 100644
index ..9e373d4f7567
--- /dev/null
+++ b/rust/kernel/arch_static_branch_asm.rs.S
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}",
"{l_yes}"))
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
index 011e1fc1d19a..7757e4f8e85e 100644
--- a/rust/kernel/jump_label.rs
+++ b/rust/kernel/jump_label.rs
@@ -23,7 +23,69 @@ macro_rules! static_key_false {
let _key: *const $keytyp = ::core::ptr::addr_of!($key);
let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
-$crate::bindings::static_key_count(_key.cast_mut()) > 0
+#[cfg(not(CONFIG_JUMP_LABEL))]
+{
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}
+
+#[cfg(CONFIG_JUMP_LABEL)]
+$crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false
}
}};
}
pub use static_key_false;
+
+/// Assert that the assembly block evaluates to a string literal.
+#[cfg(CONFIG_JUMP_LABEL)]
+const _: &str = include!("arch_static_branch_asm.rs");
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("SRCTREE"),
"/rust/kernel/arch_static_branch_asm.rs"));
+l_yes = label {
+break 'my_label true;
+},
+symb = sym $key,
+off = const ::core::mem::offset_of!($keytyp, $field),
+branch = const $crate::jump_label::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("SRCTREE"),
&quo
Re: [PATCH v6 1/5] rust: add generic static_key_false
On Fri, Aug 9, 2024 at 8:16 PM Gary Guo wrote:
>
> On Thu, 08 Aug 2024 17:23:37 +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.
> >
> > This patch only provides a generic implementation without code patching
> > (matching the one used when CONFIG_JUMP_LABEL is disabled). Later
> > patches add support for inline asm implementations that use runtime
> > patching.
> >
> > When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
> > function, so a Rust helper is defined for `static_key_count` in this
> > case. If Rust is compiled with LTO, this call should get inlined. The
> > helper can be eliminated once we have the necessary inline asm to make
> > atomic operations from Rust.
> >
> > Signed-off-by: Alice Ryhl
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/helpers.c| 9 +
> > rust/kernel/arch_static_branch_asm.rs | 1 +
> > rust/kernel/jump_label.rs | 29 +
> > rust/kernel/lib.rs| 1 +
> > 5 files changed, 41 insertions(+)
> >
> > diff --git a/rust/kernel/arch_static_branch_asm.rs
> > b/rust/kernel/arch_static_branch_asm.rs
> > new file mode 100644
> > index ..958f1f130455
> > --- /dev/null
> > +++ b/rust/kernel/arch_static_branch_asm.rs
> > @@ -0,0 +1 @@
> > +::kernel::concat_literals!("1: jmp " "{l_yes}" " # objtool NOPs this \n\t"
> > ".pushsection __jump_table, \"aw\" \n\t" " " ".balign 8" " " "\n\t" ".long
> > 1b - . \n\t" ".long " "{l_yes}" "- . \n\t" " " ".quad" " " " " "{symb} +
> > {off} + {branch}" " - . \n\t" ".popsection \n\t")
>
>
> I believe this file is included by mistake, given it's added to
> gitignore in patch 5.
We may have to rethink the file extension used here. You have no idea
how many times I've deleted that file from this commit.
Alice
Re: [PATCH v6 5/5] rust: add arch_static_branch
On Sat, Aug 10, 2024 at 11:04 PM Peter Zijlstra wrote:
>
> On Thu, Aug 08, 2024 at 05:23:41PM +, Alice Ryhl wrote:
>
> > +/// Wrapper around `asm!` that uses at&t syntax on x86.
> > +// Uses a semicolon to avoid parsing ambiguities, even though this does
> > not match native `asm!`
> > +// syntax.
> > +#[cfg(target_arch = "x86_64")]
> > +#[macro_export]
> > +macro_rules! asm {
> > +($($asm:expr),* ; $($rest:tt)*) => {
> > +::core::arch::asm!( $($asm)*, options(att_syntax), $($rest)* )
> > +};
> > +}
> > +
> > +/// Wrapper around `asm!` that uses at&t syntax on x86.
>
> ^ the above line seems out of place given the 'not' below.
Comments with three slashes are what gets rendered in the html version
of the docs [1]. This way, the rendered docs will say that the `asm!`
macro is a wrapper around the built-in `asm!` that uses at&t syntax on
x86 regardless of which platform you build the docs on.
[1]: https://rust.docs.kernel.org/kernel/
> > +// For non-x86 arches we just pass through to `asm!`.
> > +#[cfg(not(target_arch = "x86_64"))]
> ^^^
> > +#[macro_export]
> > +macro_rules! asm {
> > +($($asm:expr),* ; $($rest:tt)*) => {
> > +::core::arch::asm!( $($asm)*, $($rest)* )
> > +};
> > +}
Re: [PATCH v6 4/5] jump_label: adjust inline asm to be consistent
On Thu, Aug 8, 2024 at 7:23 PM Alice Ryhl wrote: > > To avoid duplication of inline asm between C and Rust, we need to > import the inline asm from the relevant `jump_label.h` header into Rust. > To make that easier, this patch updates the header files to expose the > inline asm via a new ARCH_STATIC_BRANCH_ASM macro. > > The header files are all updated to define a ARCH_STATIC_BRANCH_ASM that > takes the same arguments in a consistent order so that Rust can use the > same logic for every architecture. > > Link: https://lore.kernel.org/r/20240725183325.122827-7-oj...@kernel.org [1] This link is in the wrong place. It's supposed to be mentioned as a dependency for this series. Also, I intended to have the same tags here as I did on the last patch. Alice
[PATCH v7 0/5] 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.
3. Make sure that the header file is visible to bindgen.
4. 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! {
/// # Safety
/// `task` must point at a valid task for the duration
/// of this call.
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/20231101-rust-binder-v1-0-08ba9197f...@google.com/
[1]
Link: https://r.android.com/3119993 [2]
Signed-off-by: Alice Ryhl
---
Changes in v7:
- Fix spurious file included in first patch.
- Fix issue with riscv asm.
- Fix tags on fourth patch to match fifth patch.
- Add Reviewed-by/Acked-by tags where appropriate.
- Link to v6:
https://lore.kernel.org/r/20240808-tracepoint-v6-0-a23f800f1...@google.com
Changes in v6:
- Add support for !CONFIG_JUMP_LABEL.
- Add tracepoint to rust_print sample.
- Deduplicate inline asm.
- Require unsafe inside `declare_trace!`.
- Fix bug on x86 due to use of intel syntax.
- Link to v5:
https://lore.kernel.org/r/20240802-tracepoint-v5-0-faa164494...@google.com
Changes in v5:
- Update first patch regarding inline asm duplication.
- Add __rust_do_trace helper to support conditions.
- Rename DEFINE_RUST_DO_TRACE_REAL to __DEFINE_RUST_DO_TRACE.
- Get rid of glob-import in tracepoint macro.
- Address safety requirements on tracepoints in docs.
- Link to v4:
https://lore.kernel.org/rust-for-linux/20240628-tracepoint-v4-0-353d523a9...@google.com
Changes in v4:
- Move arch-specific code into rust/kernel/arch.
- Restore DEFINE_RUST_DO_TRACE at end of define_trace.h
- Link to v3:
https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com
Changes in v3:
- Support for Rust static_key on loongarch64 and riscv64.
- Avoid failing compilation on architectures that are missing Rust
static_key support when the archtectures does not actually use it.
- Link to v2:
https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com
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.
[PATCH v7 1/5] rust: add generic 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.
This patch only provides a generic implementation without code patching
(matching the one used when CONFIG_JUMP_LABEL is disabled). Later
patches add support for inline asm implementations that use runtime
patching.
When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
function, so a Rust helper is defined for `static_key_count` in this
case. If Rust is compiled with LTO, this call should get inlined. The
helper can be eliminated once we have the necessary inline asm to make
atomic operations from Rust.
Signed-off-by: Alice Ryhl
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers.c | 9 +
rust/kernel/jump_label.rs | 29 +
rust/kernel/lib.rs | 1 +
4 files changed, 40 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index b940a5777330..8fd092e1b809 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/rust/helpers.c b/rust/helpers.c
index 92d3c03ae1bd..5a9bf5209cd8 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -133,6 +134,14 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
}
EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
+#ifndef CONFIG_JUMP_LABEL
+int rust_helper_static_key_count(struct static_key *key)
+{
+ return static_key_count(key);
+}
+EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
+#endif
+
__force void *rust_helper_ERR_PTR(long err)
{
return ERR_PTR(err);
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
new file mode 100644
index ..011e1fc1d19a
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+//!
+//! C header:
[`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
+
+/// 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`.
+///
+/// # Safety
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {{
+let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
+
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}};
+}
+pub use static_key_false;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 274bdc1b0a82..91af9f75d121 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
pub mod firmware;
pub mod init;
pub mod ioctl;
+pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
#[cfg(CONFIG_NET)]
--
2.46.0.184.g6999bdac58-goog
[PATCH v7 2/5] 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.
Instead of calling __DO_TRACE directly, the exported rust_do_trace_
function calls an inline helper function. This is because the `cond`
argument does not exist at the callsite of DEFINE_RUST_DO_TRACE.
__DECLARE_TRACE always emits an inline static and an extern declaration
that is only used when CREATE_RUST_TRACE_POINTS is set. These should not
end up in the final binary so it is not a problem that they sometimes
are emitted without a user.
Reviewed-by: Carlos Llamas
Reviewed-by: Gary Guo
Signed-off-by: Alice Ryhl
---
include/linux/tracepoint.h | 22 +-
include/trace/define_trace.h| 12 ++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 49 +
5 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6be396bb4297..5042ca588e41 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -237,6 +237,18 @@ 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(name, proto, args) \
+ notrace void rust_do_trace_##name(proto)\
+ { \
+ __rust_do_trace_##name(args); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -252,6 +264,13 @@ 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 __rust_do_trace_##name(proto)\
+ { \
+ __DO_TRACE(name,\
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 0); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -336,7 +355,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..8159294c2041 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(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8fd092e1b809..fc6f94729789 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#inc
[PATCH v7 3/5] rust: samples: add tracepoint to Rust sample
This updates the Rust printing sample to invoke a tracepoint. This
ensures that we have a user in-tree from the get-go even though the
patch is being merged before its real user.
Signed-off-by: Alice Ryhl
---
MAINTAINERS| 1 +
include/trace/events/rust_sample.h | 31 +++
rust/bindings/bindings_helper.h| 1 +
samples/rust/Makefile | 3 ++-
samples/rust/rust_print.rs | 18 ++
samples/rust/rust_print_events.c | 8
6 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index f328373463b0..1acf5bfddfc4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19922,6 +19922,7 @@ C: zulip://rust-for-linux.zulipchat.com
P: https://rust-for-linux.com/contributing
T: git https://github.com/Rust-for-Linux/linux.git rust-next
F: Documentation/rust/
+F: include/trace/events/rust_sample.h
F: rust/
F: samples/rust/
F: scripts/*rust*
diff --git a/include/trace/events/rust_sample.h
b/include/trace/events/rust_sample.h
new file mode 100644
index ..dbc80ca2e465
--- /dev/null
+++ b/include/trace/events/rust_sample.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tracepoints for `samples/rust/rust_print.rs`.
+ *
+ * Copyright (C) 2024 Google, Inc.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rust_sample
+
+#if !defined(_RUST_SAMPLE_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _RUST_SAMPLE_TRACE_H
+
+#include
+
+TRACE_EVENT(rust_sample_loaded,
+ TP_PROTO(int magic_number),
+ TP_ARGS(magic_number),
+ TP_STRUCT__entry(
+ __field(int, magic_number)
+ ),
+ TP_fast_assign(
+ __entry->magic_number = magic_number;
+ ),
+ TP_printk("magic=%d", __entry->magic_number)
+);
+
+#endif /* _RUST_SAMPLE_TRACE_H */
+
+/* This part must be outside protection */
+#include
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fc6f94729789..fe97256afe65 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
/* `bindgen` gets confused at certain things. */
const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..f29280ec4820 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+ccflags-y += -I$(src) # needed for trace events
obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o
-obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
rust_print_events.o
subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 6eabb0d79ea3..6d14b08cac1c 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -69,6 +69,8 @@ fn init(_module: &'static ThisModule) -> Result {
arc_print()?;
+trace::trace_rust_sample_loaded(42);
+
Ok(RustPrint)
}
}
@@ -78,3 +80,19 @@ fn drop(&mut self) {
pr_info!("Rust printing macros sample (exit)\n");
}
}
+
+mod trace {
+use core::ffi::c_int;
+
+kernel::declare_trace! {
+/// # Safety
+///
+/// Always safe to call.
+unsafe fn rust_sample_loaded(magic: c_int);
+}
+
+pub(crate) fn trace_rust_sample_loaded(magic: i32) {
+// SAFETY: Always safe to call.
+unsafe { rust_sample_loaded(magic as c_int) }
+}
+}
diff --git a/samples/rust/rust_print_events.c b/samples/rust/rust_print_events.c
new file mode 100644
index ..a9169ff0edf1
--- /dev/null
+++ b/samples/rust/rust_print_events.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ */
+
+#define CREATE_TRACE_POINTS
+#define CREATE_RUST_TRACE_POINTS
+#include
--
2.46.0.184.g6999bdac58-goog
[PATCH v7 4/5] jump_label: adjust inline asm to be consistent
To avoid duplication of inline asm between C and Rust, we need to
import the inline asm from the relevant `jump_label.h` header into Rust.
To make that easier, this patch updates the header files to expose the
inline asm via a new ARCH_STATIC_BRANCH_ASM macro.
The header files are all updated to define a ARCH_STATIC_BRANCH_ASM that
takes the same arguments in a consistent order so that Rust can use the
same logic for every architecture.
Suggested-by: Peter Zijlstra (Intel)
Acked-by: Peter Zijlstra (Intel)
Co-developed-by: Miguel Ojeda
Signed-off-by: Miguel Ojeda
Signed-off-by: Alice Ryhl
---
arch/arm/include/asm/jump_label.h | 14 +
arch/arm64/include/asm/jump_label.h | 20 -
arch/loongarch/include/asm/jump_label.h | 16 +++
arch/riscv/include/asm/jump_label.h | 50 ++---
arch/x86/include/asm/jump_label.h | 38 ++---
5 files changed, 75 insertions(+), 63 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h
b/arch/arm/include/asm/jump_label.h
index e4eb54f6cd9f..a35aba7f548c 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -9,13 +9,17 @@
#define JUMP_LABEL_NOP_SIZE 4
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1:\n\t"\
+ WASM(nop) "\n\t"\
+ ".pushsection __jump_table, \"aw\"\n\t"\
+ ".word 1b, " label ", " key "\n\t" \
+ ".popsection\n\t" \
+
static __always_inline bool arch_static_branch(struct static_key *key, bool
branch)
{
- asm goto("1:\n\t"
-WASM(nop) "\n\t"
-".pushsection __jump_table, \"aw\"\n\t"
-".word 1b, %l[l_yes], %c0\n\t"
-".popsection\n\t"
+ asm goto(ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
: : "i" (&((char *)key)[branch]) : : l_yes);
return false;
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index a0a5bbae7229..424ed421cd97 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -19,10 +19,14 @@
#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
".align 3\n\t" \
- ".long 1b - ., %l["#label"] - .\n\t" \
- ".quad %c0 - .\n\t"\
- ".popsection\n\t" \
- : : "i"(key) : : label
+ ".long 1b - ., " label " - .\n\t" \
+ ".quad " key " - .\n\t"\
+ ".popsection\n\t"
+
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: nop\n\t"\
+ JUMP_TABLE_ENTRY(key, label)
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
@@ -30,8 +34,8 @@ static __always_inline bool arch_static_branch(struct
static_key * const key,
char *k = &((char *)key)[branch];
asm goto(
- "1: nop \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
@@ -43,9 +47,11 @@ static __always_inline bool arch_static_branch_jump(struct
static_key * const ke
const bool branch)
{
char *k = &((char *)key)[branch];
+
asm goto(
"1: b %l[l_yes] \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ JUMP_TABLE_ENTRY("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
l_yes:
diff --git a/arch/loongarch/include/asm/jump_label.h
b/arch/loongarch/include/asm/jump_label.h
index 29acfe3de3fa..8a924bd69d19 100644
--- a/arch/loongarch/include/asm/jump_label.h
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -13,18 +13,22 @@
#define JUMP_LABEL_NOP_SIZE4
-#define JUMP_TABLE_ENTRY \
+/* This macro is also expanded on the Rust side. */
+#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_t
[PATCH v7 5/5] rust: add arch_static_branch
To allow the Rust implementation of static_key_false to use runtime code
patching instead of the generic implementation, pull in the relevant
inline assembly from the jump_label.h header by running the C
preprocessor on a .rs.S file. Build rules are added for .rs.S files.
Since the relevant inline asm has been adjusted to export the inline asm
via the ARCH_STATIC_BRANCH_ASM macro in a consistent way, the Rust side
does not need architecture specific code to pull in the asm.
It is not possible to use the existing C implementation of
arch_static_branch via a Rust helper because it passes the argument
`key` to inline assembly as an 'i' parameter. 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.
Suggested-by: Peter Zijlstra (Intel)
Co-developed-by: Miguel Ojeda
Signed-off-by: Miguel Ojeda
Signed-off-by: Alice Ryhl
---
rust/Makefile | 5 ++-
rust/kernel/.gitignore | 3 ++
rust/kernel/arch_static_branch_asm.rs.S | 7
rust/kernel/jump_label.rs | 64 -
rust/kernel/lib.rs | 30
scripts/Makefile.build | 9 -
6 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 199e0db67962..277fcef656b8 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -14,6 +14,8 @@ CFLAGS_REMOVE_helpers.o = -Wmissing-prototypes
-Wmissing-declarations
always-$(CONFIG_RUST) += libmacros.so
no-clean-files += libmacros.so
+always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) +=
kernel/arch_static_branch_asm.rs
+
always-$(CONFIG_RUST) += bindings/bindings_generated.rs
bindings/bindings_helpers_generated.rs
obj-$(CONFIG_RUST) += alloc.o bindings.o kernel.o
always-$(CONFIG_RUST) += exports_alloc_generated.h
exports_bindings_generated.h \
@@ -409,7 +411,8 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
-$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
+ $(obj)/kernel/arch_static_branch_asm.rs FORCE
+$(call if_changed_rule,rustc_library)
endif # CONFIG_RUST
diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
new file mode 100644
index ..d082731007c6
--- /dev/null
+++ b/rust/kernel/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+/arch_static_branch_asm.rs
diff --git a/rust/kernel/arch_static_branch_asm.rs.S
b/rust/kernel/arch_static_branch_asm.rs.S
new file mode 100644
index ..9e373d4f7567
--- /dev/null
+++ b/rust/kernel/arch_static_branch_asm.rs.S
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}",
"{l_yes}"))
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
index 011e1fc1d19a..7757e4f8e85e 100644
--- a/rust/kernel/jump_label.rs
+++ b/rust/kernel/jump_label.rs
@@ -23,7 +23,69 @@ macro_rules! static_key_false {
let _key: *const $keytyp = ::core::ptr::addr_of!($key);
let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
-$crate::bindings::static_key_count(_key.cast_mut()) > 0
+#[cfg(not(CONFIG_JUMP_LABEL))]
+{
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}
+
+#[cfg(CONFIG_JUMP_LABEL)]
+$crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false
}
}};
}
pub use static_key_false;
+
+/// Assert that the assembly block evaluates to a string literal.
+#[cfg(CONFIG_JUMP_LABEL)]
+const _: &str = include!("arch_static_branch_asm.rs");
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("SRCTREE"),
"/rust/kernel/arch_static_branch_asm.rs"));
+l_yes = label {
+break 'my_label true;
+},
+symb = sym $key,
+off = const ::core::mem::offset_of!($keytyp, $field),
+branch = const $crate::jump_label::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("SRCTREE"),
&quo
[PATCH v8 0/5] 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. This series includes a patch that adds a user of tracepoits to the rust_print sample. Please see that sample for details on what is needed to use this feature in Rust code. 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. When compiling for x86, this patchset has a dependency on [3] as we need objtool to convert jmp instructions to nop instructions. This patchset is based on top of the series containing [3]. There is also a conflict with splitting up the C helpers [4]. I've included an alternate version of the first patch that shows how to resolve the conflict. When using the alternate version of the first patch, this series applies cleanly on top of rust-next. Both [3] and [4] are already in rust-next. Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-0-08ba9197f...@google.com/ [1] Link: https://r.android.com/3119993 [2] Link: https://lore.kernel.org/all/20240725183325.122827-7-oj...@kernel.org/ [3] Link: https://lore.kernel.org/all/20240815103016.2771842-1-...@metaspace.dk/ [4] Signed-off-by: Alice Ryhl --- Changes in v8: - Use OBJTREE instead of SRCTREE for temporary asm file. - Adjust comments on `asm!` wrapper to be less confusing. - Include resolution of conflict with helpers splitting. - Link to v7: https://lore.kernel.org/r/20240816-tracepoint-v7-0-d609b916b...@google.com Changes in v7: - Fix spurious file included in first patch. - Fix issue with riscv asm. - Fix tags on fourth patch to match fifth patch. - Add Reviewed-by/Acked-by tags where appropriate. - Link to v6: https://lore.kernel.org/r/20240808-tracepoint-v6-0-a23f800f1...@google.com Changes in v6: - Add support for !CONFIG_JUMP_LABEL. - Add tracepoint to rust_print sample. - Deduplicate inline asm. - Require unsafe inside `declare_trace!`. - Fix bug on x86 due to use of intel syntax. - Link to v5: https://lore.kernel.org/r/20240802-tracepoint-v5-0-faa164494...@google.com Changes in v5: - Update first patch regarding inline asm duplication. - Add __rust_do_trace helper to support conditions. - Rename DEFINE_RUST_DO_TRACE_REAL to __DEFINE_RUST_DO_TRACE. - Get rid of glob-import in tracepoint macro. - Address safety requirements on tracepoints in docs. - Link to v4: https://lore.kernel.org/rust-for-linux/20240628-tracepoint-v4-0-353d523a9...@google.com Changes in v4: - Move arch-specific code into rust/kernel/arch. - Restore DEFINE_RUST_DO_TRACE at end of define_trace.h - Link to v3: https://lore.kernel.org/r/20240621-tracepoint-v3-0-9e44eeea2...@google.com Changes in v3: - Support for Rust static_key on loongarch64 and riscv64. - Avoid failing compilation on architectures that are missing Rust static_key support when the archtectures does not actually use it. - Link to v2: https://lore.kernel.org/r/20240610-tracepoint-v2-0-faebad81b...@google.com 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/20240606-tracepoint-v1-0-6551627bf...@google.com --- Alice Ryhl (5): rust: add generic static_key_false rust: add tracepoint support rust: samples: add tracepoint to Rust sample jump_label: adjust inline asm to be consistent rust: add arch_static_branch MAINTAINERS | 1 + arch/arm/include/asm/jump_label.h | 14 +++-- arch/arm64/include/asm/jump_label.h | 20 +--- arch/loongarch/include/asm/jump_label.h | 16 +++--- arch/riscv/include/asm/jump_label.h | 50 ++ arch/x86/include/asm/jump_label.h | 38 ++ include/linux/tracepoint.h | 22 +++- include/trace/define_trace.h| 12 + include/trace/events/rust_sample.h | 31 +++ rust/Makefile | 5 +- rust/bindings/bindings_helper.h | 3 ++ rust/helpers.c | 9 rust/kernel/.gitignore | 3 ++ rust/kernel/arch_static_branch_asm.rs.S | 7 +++ rust/kernel/jump_label.rs | 91 + rust/kernel/lib.rs | 37 ++ rust/kernel/tracepoint.rs | 49 ++ samples/rust/Makefile | 3 +- samples
[PATCH v8 1/5] rust: add generic 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.
This patch only provides a generic implementation without code patching
(matching the one used when CONFIG_JUMP_LABEL is disabled). Later
patches add support for inline asm implementations that use runtime
patching.
When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
function, so a Rust helper is defined for `static_key_count` in this
case. If Rust is compiled with LTO, this call should get inlined. The
helper can be eliminated once we have the necessary inline asm to make
atomic operations from Rust.
Signed-off-by: Alice Ryhl
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers.c | 9 +
rust/kernel/jump_label.rs | 29 +
rust/kernel/lib.rs | 1 +
4 files changed, 40 insertions(+)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index b940a5777330..8fd092e1b809 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/rust/helpers.c b/rust/helpers.c
index 92d3c03ae1bd..5a9bf5209cd8 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -133,6 +134,14 @@ bool rust_helper_refcount_dec_and_test(refcount_t *r)
}
EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
+#ifndef CONFIG_JUMP_LABEL
+int rust_helper_static_key_count(struct static_key *key)
+{
+ return static_key_count(key);
+}
+EXPORT_SYMBOL_GPL(rust_helper_static_key_count);
+#endif
+
__force void *rust_helper_ERR_PTR(long err)
{
return ERR_PTR(err);
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
new file mode 100644
index ..011e1fc1d19a
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+//!
+//! C header:
[`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
+
+/// 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`.
+///
+/// # Safety
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {{
+let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
+
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}};
+}
+pub use static_key_false;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 274bdc1b0a82..91af9f75d121 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
pub mod firmware;
pub mod init;
pub mod ioctl;
+pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
#[cfg(CONFIG_NET)]
--
2.46.0.184.g6999bdac58-goog
[PATCH v8 2/5] 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.
Instead of calling __DO_TRACE directly, the exported rust_do_trace_
function calls an inline helper function. This is because the `cond`
argument does not exist at the callsite of DEFINE_RUST_DO_TRACE.
__DECLARE_TRACE always emits an inline static and an extern declaration
that is only used when CREATE_RUST_TRACE_POINTS is set. These should not
end up in the final binary so it is not a problem that they sometimes
are emitted without a user.
Reviewed-by: Carlos Llamas
Reviewed-by: Gary Guo
Signed-off-by: Alice Ryhl
---
include/linux/tracepoint.h | 22 +-
include/trace/define_trace.h| 12 ++
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/tracepoint.rs | 49 +
5 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 6be396bb4297..5042ca588e41 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -237,6 +237,18 @@ 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(name, proto, args) \
+ notrace void rust_do_trace_##name(proto)\
+ { \
+ __rust_do_trace_##name(args); \
+ }
+
/*
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
@@ -252,6 +264,13 @@ 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 __rust_do_trace_##name(proto)\
+ { \
+ __DO_TRACE(name,\
+ TP_ARGS(args), \
+ TP_CONDITION(cond), 0); \
+ } \
static inline void trace_##name(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -336,7 +355,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..8159294c2041 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(name, PARAMS(proto), PARAMS(args))
+#endif
+
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
@@ -129,6 +136,11 @@
# undef UNDEF_TRACE_INCLUDE_PATH
#endif
+#ifdef CREATE_RUST_TRACE_POINTS
+# undef DEFINE_RUST_DO_TRACE
+# define DEFINE_RUST_DO_TRACE(name, proto, args)
+#endif
+
/* We may be processing more files */
#define CREATE_TRACE_POINTS
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 8fd092e1b809..fc6f94729789 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -20,6 +20,7 @@
#include
#include
#include
+#inc
[PATCH v8 3/5] rust: samples: add tracepoint to Rust sample
This updates the Rust printing sample to invoke a tracepoint. This
ensures that we have a user in-tree from the get-go even though the
patch is being merged before its real user.
Signed-off-by: Alice Ryhl
---
MAINTAINERS| 1 +
include/trace/events/rust_sample.h | 31 +++
rust/bindings/bindings_helper.h| 1 +
samples/rust/Makefile | 3 ++-
samples/rust/rust_print.rs | 18 ++
samples/rust/rust_print_events.c | 8
6 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index f328373463b0..1acf5bfddfc4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19922,6 +19922,7 @@ C: zulip://rust-for-linux.zulipchat.com
P: https://rust-for-linux.com/contributing
T: git https://github.com/Rust-for-Linux/linux.git rust-next
F: Documentation/rust/
+F: include/trace/events/rust_sample.h
F: rust/
F: samples/rust/
F: scripts/*rust*
diff --git a/include/trace/events/rust_sample.h
b/include/trace/events/rust_sample.h
new file mode 100644
index ..dbc80ca2e465
--- /dev/null
+++ b/include/trace/events/rust_sample.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tracepoints for `samples/rust/rust_print.rs`.
+ *
+ * Copyright (C) 2024 Google, Inc.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM rust_sample
+
+#if !defined(_RUST_SAMPLE_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _RUST_SAMPLE_TRACE_H
+
+#include
+
+TRACE_EVENT(rust_sample_loaded,
+ TP_PROTO(int magic_number),
+ TP_ARGS(magic_number),
+ TP_STRUCT__entry(
+ __field(int, magic_number)
+ ),
+ TP_fast_assign(
+ __entry->magic_number = magic_number;
+ ),
+ TP_printk("magic=%d", __entry->magic_number)
+);
+
+#endif /* _RUST_SAMPLE_TRACE_H */
+
+/* This part must be outside protection */
+#include
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index fc6f94729789..fe97256afe65 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
/* `bindgen` gets confused at certain things. */
const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..f29280ec4820 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+ccflags-y += -I$(src) # needed for trace events
obj-$(CONFIG_SAMPLE_RUST_MINIMAL) += rust_minimal.o
-obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_PRINT)+= rust_print.o
rust_print_events.o
subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS) += hostprogs
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 6eabb0d79ea3..6d14b08cac1c 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -69,6 +69,8 @@ fn init(_module: &'static ThisModule) -> Result {
arc_print()?;
+trace::trace_rust_sample_loaded(42);
+
Ok(RustPrint)
}
}
@@ -78,3 +80,19 @@ fn drop(&mut self) {
pr_info!("Rust printing macros sample (exit)\n");
}
}
+
+mod trace {
+use core::ffi::c_int;
+
+kernel::declare_trace! {
+/// # Safety
+///
+/// Always safe to call.
+unsafe fn rust_sample_loaded(magic: c_int);
+}
+
+pub(crate) fn trace_rust_sample_loaded(magic: i32) {
+// SAFETY: Always safe to call.
+unsafe { rust_sample_loaded(magic as c_int) }
+}
+}
diff --git a/samples/rust/rust_print_events.c b/samples/rust/rust_print_events.c
new file mode 100644
index ..a9169ff0edf1
--- /dev/null
+++ b/samples/rust/rust_print_events.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ */
+
+#define CREATE_TRACE_POINTS
+#define CREATE_RUST_TRACE_POINTS
+#include
--
2.46.0.184.g6999bdac58-goog
[PATCH v8 4/5] jump_label: adjust inline asm to be consistent
To avoid duplication of inline asm between C and Rust, we need to
import the inline asm from the relevant `jump_label.h` header into Rust.
To make that easier, this patch updates the header files to expose the
inline asm via a new ARCH_STATIC_BRANCH_ASM macro.
The header files are all updated to define a ARCH_STATIC_BRANCH_ASM that
takes the same arguments in a consistent order so that Rust can use the
same logic for every architecture.
Suggested-by: Peter Zijlstra (Intel)
Acked-by: Peter Zijlstra (Intel)
Co-developed-by: Miguel Ojeda
Signed-off-by: Miguel Ojeda
Signed-off-by: Alice Ryhl
---
arch/arm/include/asm/jump_label.h | 14 +
arch/arm64/include/asm/jump_label.h | 20 -
arch/loongarch/include/asm/jump_label.h | 16 +++
arch/riscv/include/asm/jump_label.h | 50 ++---
arch/x86/include/asm/jump_label.h | 38 ++---
5 files changed, 75 insertions(+), 63 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h
b/arch/arm/include/asm/jump_label.h
index e4eb54f6cd9f..a35aba7f548c 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -9,13 +9,17 @@
#define JUMP_LABEL_NOP_SIZE 4
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1:\n\t"\
+ WASM(nop) "\n\t"\
+ ".pushsection __jump_table, \"aw\"\n\t"\
+ ".word 1b, " label ", " key "\n\t" \
+ ".popsection\n\t" \
+
static __always_inline bool arch_static_branch(struct static_key *key, bool
branch)
{
- asm goto("1:\n\t"
-WASM(nop) "\n\t"
-".pushsection __jump_table, \"aw\"\n\t"
-".word 1b, %l[l_yes], %c0\n\t"
-".popsection\n\t"
+ asm goto(ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
: : "i" (&((char *)key)[branch]) : : l_yes);
return false;
diff --git a/arch/arm64/include/asm/jump_label.h
b/arch/arm64/include/asm/jump_label.h
index a0a5bbae7229..424ed421cd97 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -19,10 +19,14 @@
#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_table, \"aw\"\n\t" \
".align 3\n\t" \
- ".long 1b - ., %l["#label"] - .\n\t" \
- ".quad %c0 - .\n\t"\
- ".popsection\n\t" \
- : : "i"(key) : : label
+ ".long 1b - ., " label " - .\n\t" \
+ ".quad " key " - .\n\t"\
+ ".popsection\n\t"
+
+/* This macro is also expanded on the Rust side. */
+#define ARCH_STATIC_BRANCH_ASM(key, label) \
+ "1: nop\n\t"\
+ JUMP_TABLE_ENTRY(key, label)
static __always_inline bool arch_static_branch(struct static_key * const key,
const bool branch)
@@ -30,8 +34,8 @@ static __always_inline bool arch_static_branch(struct
static_key * const key,
char *k = &((char *)key)[branch];
asm goto(
- "1: nop \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ ARCH_STATIC_BRANCH_ASM("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
@@ -43,9 +47,11 @@ static __always_inline bool arch_static_branch_jump(struct
static_key * const ke
const bool branch)
{
char *k = &((char *)key)[branch];
+
asm goto(
"1: b %l[l_yes] \n\t"
- JUMP_TABLE_ENTRY(k, l_yes)
+ JUMP_TABLE_ENTRY("%c0", "%l[l_yes]")
+ : : "i"(k) : : l_yes
);
return false;
l_yes:
diff --git a/arch/loongarch/include/asm/jump_label.h
b/arch/loongarch/include/asm/jump_label.h
index 29acfe3de3fa..8a924bd69d19 100644
--- a/arch/loongarch/include/asm/jump_label.h
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -13,18 +13,22 @@
#define JUMP_LABEL_NOP_SIZE4
-#define JUMP_TABLE_ENTRY \
+/* This macro is also expanded on the Rust side. */
+#define JUMP_TABLE_ENTRY(key, label) \
".pushsection __jump_t
[PATCH v8 5/5] rust: add arch_static_branch
To allow the Rust implementation of static_key_false to use runtime code
patching instead of the generic implementation, pull in the relevant
inline assembly from the jump_label.h header by running the C
preprocessor on a .rs.S file. Build rules are added for .rs.S files.
Since the relevant inline asm has been adjusted to export the inline asm
via the ARCH_STATIC_BRANCH_ASM macro in a consistent way, the Rust side
does not need architecture specific code to pull in the asm.
It is not possible to use the existing C implementation of
arch_static_branch via a Rust helper because it passes the argument
`key` to inline assembly as an 'i' parameter. 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.
Suggested-by: Peter Zijlstra (Intel)
Co-developed-by: Miguel Ojeda
Signed-off-by: Miguel Ojeda
Signed-off-by: Alice Ryhl
---
rust/Makefile | 5 ++-
rust/kernel/.gitignore | 3 ++
rust/kernel/arch_static_branch_asm.rs.S | 7
rust/kernel/jump_label.rs | 64 -
rust/kernel/lib.rs | 35 ++
scripts/Makefile.build | 9 -
6 files changed, 120 insertions(+), 3 deletions(-)
diff --git a/rust/Makefile b/rust/Makefile
index 043d8737b430..27da24d90b0c 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -36,6 +36,8 @@ always-$(CONFIG_RUST_KERNEL_DOCTESTS) +=
doctests_kernel_generated_kunit.c
obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
+always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) +=
kernel/arch_static_branch_asm.rs
+
# Avoids running `$(RUSTC)` for the sysroot when it may not be available.
ifdef CONFIG_RUST
@@ -409,7 +411,8 @@ $(obj)/uapi.o: $(src)/uapi/lib.rs \
$(obj)/kernel.o: private rustc_target_flags = --extern alloc \
--extern build_error --extern macros --extern bindings --extern uapi
$(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
-$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
+$(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o \
+ $(obj)/kernel/arch_static_branch_asm.rs FORCE
+$(call if_changed_rule,rustc_library)
endif # CONFIG_RUST
diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
new file mode 100644
index ..d082731007c6
--- /dev/null
+++ b/rust/kernel/.gitignore
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+/arch_static_branch_asm.rs
diff --git a/rust/kernel/arch_static_branch_asm.rs.S
b/rust/kernel/arch_static_branch_asm.rs.S
new file mode 100644
index ..9e373d4f7567
--- /dev/null
+++ b/rust/kernel/arch_static_branch_asm.rs.S
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}",
"{l_yes}"))
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
index 011e1fc1d19a..ccfd20589c21 100644
--- a/rust/kernel/jump_label.rs
+++ b/rust/kernel/jump_label.rs
@@ -23,7 +23,69 @@ macro_rules! static_key_false {
let _key: *const $keytyp = ::core::ptr::addr_of!($key);
let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
-$crate::bindings::static_key_count(_key.cast_mut()) > 0
+#[cfg(not(CONFIG_JUMP_LABEL))]
+{
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}
+
+#[cfg(CONFIG_JUMP_LABEL)]
+$crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false
}
}};
}
pub use static_key_false;
+
+/// Assert that the assembly block evaluates to a string literal.
+#[cfg(CONFIG_JUMP_LABEL)]
+const _: &str = include!(concat!(env!("OBJTREE"),
"/rust/kernel/arch_static_branch_asm.rs"));
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(not(CONFIG_HAVE_JUMP_LABEL_HACK))]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("OBJTREE"),
"/rust/kernel/arch_static_branch_asm.rs"));
+l_yes = label {
+break 'my_label true;
+},
+symb = sym $key,
+off = const ::core::mem::offset_of!($keytyp, $field),
+branch = const $crate::jump_label::bool_to_int($branch),
+);
+
+break 'my_label false;
+}};
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(CONFIG_JUMP_LABEL)]
+#[cfg(CONFIG_HAVE_JUMP_LABEL_HACK)]
+macro_rules! arch_static_branch {
+($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+$crate::asm!(
+include!(concat!(env!("OBJTREE"),
"/rust/kernel/arch_stati
[PATCH v8 1/5 alt] rust: add generic 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.
This patch only provides a generic implementation without code patching
(matching the one used when CONFIG_JUMP_LABEL is disabled). Later
patches add support for inline asm implementations that use runtime
patching.
When CONFIG_JUMP_LABEL is unset, `static_key_count` is a static inline
function, so a Rust helper is defined for `static_key_count` in this
case. If Rust is compiled with LTO, this call should get inlined. The
helper can be eliminated once we have the necessary inline asm to make
atomic operations from Rust.
Signed-off-by: Alice Ryhl
---
This is an alternate version of patch 1 that resolves the conflict with
https://lore.kernel.org/all/20240725183325.122827-7-oj...@kernel.org/
rust/bindings/bindings_helper.h | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/tracepoint.c | 18 ++
rust/kernel/jump_label.rs | 29 +
rust/kernel/lib.rs | 1 +
5 files changed, 50 insertions(+)
create mode 100644 rust/helpers/tracepoint.c
create mode 100644 rust/kernel/jump_label.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ae82e9c941af..e0846e7e93e6 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 173533616c91..5b17839de43a 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -20,6 +20,7 @@
#include "slab.c"
#include "spinlock.c"
#include "task.c"
+#include "tracepoint.c"
#include "uaccess.c"
#include "wait.c"
#include "workqueue.c"
diff --git a/rust/helpers/tracepoint.c b/rust/helpers/tracepoint.c
new file mode 100644
index ..02aafb2b226f
--- /dev/null
+++ b/rust/helpers/tracepoint.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Helpers for tracepoints. At the moment, helpers are only needed when
+ * CONFIG_JUMP_LABEL is disabled, as `static_key_count` is only marked inline
+ * in that case.
+ *
+ * Copyright (C) 2024 Google LLC.
+ */
+
+#include
+
+#ifndef CONFIG_JUMP_LABEL
+int rust_helper_static_key_count(struct static_key *key)
+{
+ return static_key_count(key);
+}
+#endif
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
new file mode 100644
index ..011e1fc1d19a
--- /dev/null
+++ b/rust/kernel/jump_label.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Logic for static keys.
+//!
+//! C header:
[`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
+
+/// 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`.
+///
+/// # Safety
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_key_false {
+($key:path, $keytyp:ty, $field:ident) => {{
+let _key: *const $keytyp = ::core::ptr::addr_of!($key);
+let _key: *const $crate::bindings::static_key =
::core::ptr::addr_of!((*_key).$field);
+
+$crate::bindings::static_key_count(_key.cast_mut()) > 0
+}};
+}
+pub use static_key_false;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 274bdc1b0a82..91af9f75d121 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -36,6 +36,7 @@
pub mod firmware;
pub mod init;
pub mod ioctl;
+pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
#[cfg(CONFIG_NET)]
--
2.46.0.184.g6999bdac58-goog
Re: [PATCH v2 1/3] rust: kunit: add KUnit case and suite macros
On Tue, Oct 29, 2024 at 10:24 AM David Gow wrote:
>
> From: José Expósito
>
> Add a couple of Rust const functions and macros to allow to develop
> KUnit tests without relying on generated C code:
>
> - The `kunit_unsafe_test_suite!` Rust macro is similar to the
>`kunit_test_suite` C macro. It requires a NULL-terminated array of
>test cases (see below).
> - The `kunit_case` Rust function is similar to the `KUNIT_CASE` C macro.
>It generates as case from the name and function.
> - The `kunit_case_null` Rust function generates a NULL test case, which
>is to be used as delimiter in `kunit_test_suite!`.
>
> While these functions and macros can be used on their own, a future
> patch will introduce another macro to create KUnit tests using a
> user-space like syntax.
>
> Signed-off-by: José Expósito
> Co-developed-by: Matt Gilbride
> Signed-off-by: Matt Gilbride
> Co-developed-by: David Gow
> Signed-off-by: David Gow
> ---
>
> Changes since v1:
> https://lore.kernel.org/lkml/20230720-rustbind-v1-1-c80db349e...@google.com/
> - Rebase on top of rust-next
> - As a result, KUnit attributes are new set. These are hardcoded to the
> defaults of "normal" speed and no module name.
> - Split the kunit_case!() macro into two const functions, kunit_case()
> and kunit_case_null() (for the NULL terminator).
>
> ---
> rust/kernel/kunit.rs | 108 +++
> rust/kernel/lib.rs | 1 +
> 2 files changed, 109 insertions(+)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 824da0e9738a..fc2d259db458 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -161,3 +161,111 @@ macro_rules! kunit_assert_eq {
> $crate::kunit_assert!($name, $file, $diff, $left == $right);
> }};
> }
> +
> +/// Represents an individual test case.
> +///
> +/// The test case should have the signature
> +/// `unsafe extern "C" fn test_case(test: *mut crate::bindings::kunit)`.
> +///
> +/// The `kunit_unsafe_test_suite!` macro expects a NULL-terminated list of
> test cases.
> +/// Use `kunit_case_null` to generate such a delimeter.
> +const fn kunit_case(name: &kernel::str::CStr, run_case: unsafe extern "C"
> fn(*mut kernel::bindings::kunit)) -> kernel::bindings::kunit_case {
This should probably say `name: &'static CStr` to require that the
name lives forever.
> +/// Registers a KUnit test suite.
> +///
> +/// # Safety
> +///
> +/// `test_cases` must be a NULL terminated array of test cases.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// unsafe extern "C" fn test_fn(_test: *mut crate::bindings::kunit) {
> +/// let actual = 1 + 1;
> +/// let expected = 2;
> +/// assert_eq!(actual, expected);
> +/// }
> +///
> +/// static mut KUNIT_TEST_CASE: crate::bindings::kunit_case =
> crate::kunit_case(name, test_fn);
> +/// static mut KUNIT_NULL_CASE: crate::bindings::kunit_case =
> crate::kunit_case_null();
> +/// static mut KUNIT_TEST_CASES: &mut[crate::bindings::kunit_case] = unsafe {
> +/// &mut[KUNIT_TEST_CASE, KUNIT_NULL_CASE]
> +/// };
> +/// crate::kunit_unsafe_test_suite!(suite_name, KUNIT_TEST_CASES);
> +/// ```
> +#[macro_export]
> +macro_rules! kunit_unsafe_test_suite {
> +($name:ident, $test_cases:ident) => {
> +const _: () = {
> +static KUNIT_TEST_SUITE_NAME: [i8; 256] = {
> +let name_u8 = core::stringify!($name).as_bytes();
> +let mut ret = [0; 256];
> +
> +let mut i = 0;
> +while i < name_u8.len() {
> +ret[i] = name_u8[i] as i8;
> +i += 1;
> +}
I assume the name must be zero-terminated? If so, you probably need to
enforce that somehow, e.g. by failing if `name_u8` is longer than 255
bytes.
> +
> +static mut KUNIT_TEST_SUITE:
> core::cell::UnsafeCell<$crate::bindings::kunit_suite> =
You don't actually need UnsafeCell here since it's already `static mut`.
> +core::cell::UnsafeCell::new($crate::bindings::kunit_suite {
> +name: KUNIT_TEST_SUITE_NAME,
> +// SAFETY: User is expected to pass a correct
> `test_cases`, hence this macro
> +// named 'unsafe'.
> +test_cases: unsafe { $test_cases.as_mut_ptr() },
> +suite_init: None,
> +suite_exit: None,
> +init: None,
> +exit: None,
> +attr: $crate::bindings::kunit_attributes {
> +speed:
> $crate::bindings::kunit_speed_KUNIT_SPEED_NORMAL,
> +},
> +status_comment: [0; 256usize],
> +debugfs: core::ptr::null_mut(),
> +log: core::ptr::null_mut(),
> +suite_init_err: 0,
> +is_init: false,
> +});
> +
> +#[used]
> +#[link_section = ".kunit_test_su
Re: [PATCH v2 1/3] rust: kunit: add KUnit case and suite macros
On Wed, Oct 30, 2024 at 5:59 AM David Gow wrote:
>
> On Tue, 29 Oct 2024 at 20:08, Alice Ryhl wrote:
> >
> > On Tue, Oct 29, 2024 at 10:24 AM David Gow wrote:
> > >
> > > From: José Expósito
> > >
> > > Add a couple of Rust const functions and macros to allow to develop
> > > KUnit tests without relying on generated C code:
> > >
> > > - The `kunit_unsafe_test_suite!` Rust macro is similar to the
> > >`kunit_test_suite` C macro. It requires a NULL-terminated array of
> > >test cases (see below).
> > > - The `kunit_case` Rust function is similar to the `KUNIT_CASE` C macro.
> > >It generates as case from the name and function.
> > > - The `kunit_case_null` Rust function generates a NULL test case, which
> > >is to be used as delimiter in `kunit_test_suite!`.
> > >
> > > While these functions and macros can be used on their own, a future
> > > patch will introduce another macro to create KUnit tests using a
> > > user-space like syntax.
> > >
> > > Signed-off-by: José Expósito
> > > Co-developed-by: Matt Gilbride
> > > Signed-off-by: Matt Gilbride
> > > Co-developed-by: David Gow
> > > Signed-off-by: David Gow
> > > ---
> > >
> > > Changes since v1:
> > > https://lore.kernel.org/lkml/20230720-rustbind-v1-1-c80db349e...@google.com/
> > > - Rebase on top of rust-next
> > > - As a result, KUnit attributes are new set. These are hardcoded to the
> > > defaults of "normal" speed and no module name.
> > > - Split the kunit_case!() macro into two const functions, kunit_case()
> > > and kunit_case_null() (for the NULL terminator).
> > >
> > > ---
> > > rust/kernel/kunit.rs | 108 +++
> > > rust/kernel/lib.rs | 1 +
> > > 2 files changed, 109 insertions(+)
> > >
> > > diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> > > index 824da0e9738a..fc2d259db458 100644
> > > --- a/rust/kernel/kunit.rs
> > > +++ b/rust/kernel/kunit.rs
> > > @@ -161,3 +161,111 @@ macro_rules! kunit_assert_eq {
> > > $crate::kunit_assert!($name, $file, $diff, $left == $right);
> > > }};
> > > }
> > > +
> > > +/// Represents an individual test case.
> > > +///
> > > +/// The test case should have the signature
> > > +/// `unsafe extern "C" fn test_case(test: *mut crate::bindings::kunit)`.
> > > +///
> > > +/// The `kunit_unsafe_test_suite!` macro expects a NULL-terminated list
> > > of test cases.
> > > +/// Use `kunit_case_null` to generate such a delimeter.
> > > +const fn kunit_case(name: &kernel::str::CStr, run_case: unsafe extern
> > > "C" fn(*mut kernel::bindings::kunit)) -> kernel::bindings::kunit_case {
> >
> > This should probably say `name: &'static CStr` to require that the
> > name lives forever.
>
> Fixed in v3, thanks.
>
> > > +/// Registers a KUnit test suite.
> > > +///
> > > +/// # Safety
> > > +///
> > > +/// `test_cases` must be a NULL terminated array of test cases.
> > > +///
> > > +/// # Examples
> > > +///
> > > +/// ```ignore
> > > +/// unsafe extern "C" fn test_fn(_test: *mut crate::bindings::kunit) {
> > > +/// let actual = 1 + 1;
> > > +/// let expected = 2;
> > > +/// assert_eq!(actual, expected);
> > > +/// }
> > > +///
> > > +/// static mut KUNIT_TEST_CASE: crate::bindings::kunit_case =
> > > crate::kunit_case(name, test_fn);
> > > +/// static mut KUNIT_NULL_CASE: crate::bindings::kunit_case =
> > > crate::kunit_case_null();
> > > +/// static mut KUNIT_TEST_CASES: &mut[crate::bindings::kunit_case] =
> > > unsafe {
> > > +/// &mut[KUNIT_TEST_CASE, KUNIT_NULL_CASE]
> > > +/// };
> > > +/// crate::kunit_unsafe_test_suite!(suite_name, KUNIT_TEST_CASES);
> > > +/// ```
> > > +#[macro_export]
> > > +macro_rules! kunit_unsafe_test_suite {
> > > +($name:ident, $test_cases:ident) => {
> > > +const _: () = {
> > > +static KUNIT_TEST_SUITE_NAME: [i8; 256] = {
> > > +let name_u8 = core::stringify!($name).as_bytes();
> > > +let mut ret = [0; 256];
> > > +
> > > +let mut i = 0;
> > > +while i < name_u8.len() {
> > > +ret[i] = name_u8[i] as i8;
> > > +i += 1;
> > > +}
> >
> > I assume the name must be zero-terminated? If so, you probably need to
> > enforce that somehow, e.g. by failing if `name_u8` is longer than 255
> > bytes.
>
> Nice catch. I'm not sure how to nicely throw a compile time error in
> this function, so I'm truncating it here and doing a compile error in
> the macro in patch #2. This isn't ideal, but seems to work.
You should be able to just panic! if it happens.
Also, I believe it should be i < 255 instead of 256?
Alice
Re: [PATCH 5/5] rust: cleanup unnecessary casts
On Fri, Sep 13, 2024 at 11:33 PM Gary Guo wrote: > > With `long` mapped to `isize`, `size_t`/`__kernel_size_t` mapped to > usize and `char` mapped to `u8`, many of the existing casts are no > longer necessary. > > Signed-off-by: Gary Guo This is great! Reviewed-by: Alice Ryhl
Re: [PATCH v5 3/5] rust: str: implement `strip_prefix` for `BStr`
On Tue, Feb 4, 2025 at 1:05 PM Andreas Hindborg wrote:
>
> Implement `strip_prefix` for `BStr` by deferring to `slice::strip_prefix`
> on the underlying `&[u8]`.
>
> Reviewed-by: Gary Guo
> Reviewed-by: Alice Ryhl
> Signed-off-by: Andreas Hindborg
> ---
>
> It is also possible to get this method by implementing
> `core::slice::SlicePattern` for `BStr`. `SlicePattern` is unstable, so this
> seems more reasonable.
> ---
> rust/kernel/str.rs | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index
> 1eb945bed77d6592216cf30678fcca70d4c0b3b3..80601206961e5b2d682af5f7028434bba1604272
> 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -31,6 +31,22 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
> // SAFETY: `BStr` is transparent to `[u8]`.
> unsafe { &*(bytes as *const [u8] as *const BStr) }
> }
> +
> +/// Strip a prefix from `self`. Delegates to [`slice::strip_prefix`].
> +///
> +/// # Example
> +/// ```
> +/// use kernel::b_str;
> +/// assert_eq!(Some(b_str!("bar")),
> b_str!("foobar").strip_prefix(b_str!("foo")));
> +/// assert_eq!(None, b_str!("foobar").strip_prefix(b_str!("bar")));
> +/// assert_eq!(Some(b_str!("foobar")),
> b_str!("foobar").strip_prefix(b_str!("")));
> +/// assert_eq!(Some(b_str!("")),
> b_str!("foobar").strip_prefix(b_str!("foobar")));
> +/// ```
> +pub fn strip_prefix(&self, pattern: &Self) -> Option<&BStr> {
Perhaps the pattern should just be &[u8]?
The code itself is fine, so
Reviewed-by: Alice Ryhl
Alice
Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng wrote:
>
> To provide using LKMM atomics for Rust code, a generic `Atomic` is
> added, currently `T` needs to be Send + Copy because these are the
> straightforward usages and all basic types support this. The trait
> `AllowAtomic` should be only ipmlemented inside atomic mod until the
> generic atomic framework is mature enough (unless the ipmlementer is a
> `#[repr(transparent)]` new type).
>
> `AtomicIpml` types are automatically `AllowAtomic`, and so far only
> basic operations load() and store() are introduced.
The ipml typo continues in this patch.
> Signed-off-by: Boqun Feng
> ---
> rust/kernel/sync/atomic.rs | 2 +
> rust/kernel/sync/atomic/generic.rs | 253 +
> 2 files changed, 255 insertions(+)
> create mode 100644 rust/kernel/sync/atomic/generic.rs
>
> diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs
> index be2e8583595f..b791abc59b61 100644
> --- a/rust/kernel/sync/atomic.rs
> +++ b/rust/kernel/sync/atomic.rs
> @@ -16,7 +16,9 @@
> //!
> //! [`LKMM`]: srctree/tools/memory-mode/
>
> +pub mod generic;
> pub mod ops;
> pub mod ordering;
>
> +pub use generic::Atomic;
> pub use ordering::{Acquire, Full, Relaxed, Release};
> diff --git a/rust/kernel/sync/atomic/generic.rs
> b/rust/kernel/sync/atomic/generic.rs
> new file mode 100644
> index ..204da38e2691
> --- /dev/null
> +++ b/rust/kernel/sync/atomic/generic.rs
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Generic atomic primitives.
> +
> +use super::ops::*;
> +use super::ordering::*;
> +use crate::types::Opaque;
> +
> +/// A generic atomic variable.
> +///
> +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be
> chosen.
> +///
> +/// # Invariants
> +///
> +/// Doing an atomic operation while holding a reference of [`Self`] won't
> cause a data race, this
> +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and the
> extra safety requirement
> +/// of the usage on pointers returned by [`Self::as_ptr`].
> +#[repr(transparent)]
> +pub struct Atomic(Opaque);
> +
> +// SAFETY: `Atomic` is safe to share among execution contexts because all
> accesses are atomic.
> +unsafe impl Sync for Atomic {}
Surely it should also be Send?
> +/// Atomics that support basic atomic operations.
> +///
> +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an
> existing [`AllowAtomic`], the
> +/// impl block should be only done in atomic mod. And currently only basic
> integer types can
> +/// implement this trait in atomic mod.
What's up with this TODO? Can't you just write an appropriate safety
requirement?
> +/// # Safety
> +///
> +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> +/// The backing atomic implementation type.
> +type Repr: AtomicImpl;
> +
> +/// Converts into a [`Self::Repr`].
> +fn into_repr(self) -> Self::Repr;
> +
> +/// Converts from a [`Self::Repr`].
> +fn from_repr(repr: Self::Repr) -> Self;
What do you need these methods for?
> +}
> +
> +// SAFETY: `T::Repr` is `Self` (i.e. `T`), so they have the same size and
> alignment.
> +unsafe impl AllowAtomic for T {
> +type Repr = Self;
> +
> +fn into_repr(self) -> Self::Repr {
> +self
> +}
> +
> +fn from_repr(repr: Self::Repr) -> Self {
> +repr
> +}
> +}
> +
> +impl Atomic {
> +/// Creates a new atomic.
> +pub const fn new(v: T) -> Self {
> +Self(Opaque::new(v))
> +}
> +
> +/// Creates a reference to [`Self`] from a pointer.
> +///
> +/// # Safety
> +///
> +/// - `ptr` has to be a valid pointer.
> +/// - `ptr` has to be valid for both reads and writes for the whole
> lifetime `'a`.
> +/// - For the whole lifetime of '`a`, other accesses to the object
> cannot cause data races
> +/// (defined by [`LKMM`]) against atomic operations on the returned
> reference.
> +///
> +/// [`LKMM`]: srctree/tools/memory-model
> +///
> +/// # Examples
> +///
> +/// Using [`Atomic::from_ptr()`] combined with [`Atomic::load()`] or
> [`Atomic::store()`] can
> +/// achieve the same functionality as `READ_ONCE()`/`smp_load_acquire()`
> or
> +/// `WRITE_ONCE()`/`smp_store_release()` in C side:
> +///
> +/// ```rust
> +/// # use kernel::types::Opaque;
> +/// use kernel::sync::atomic::{Atomic, Relaxed, Release};
> +///
> +/// // Assume there is a C struct `Foo`.
> +/// mod cbindings {
> +/// #[repr(C)]
> +/// pub(crate) struct foo { pub(crate) a: i32, pub(crate) b: i32 }
> +/// }
> +///
> +/// let tmp = Opaque::new(cbindings::foo { a: 1, b: 2});
> +///
> +/// // struct foo *foo_ptr = ..;
> +/// let foo_ptr = tmp.get();
> +///
> +/// // SAFETY: `foo_ptr` is a valid pointer, and `.a` is inbound.
> +/// let foo_a_ptr = unsa
Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng wrote:
>
> Preparation for generic atomic implementation. To unify the
> ipmlementation of a generic method over `i32` and `i64`, the C side
> atomic methods need to be grouped so that in a generic method, they can
> be referred as ::, otherwise their parameters and return
> value are different between `i32` and `i64`, which would require using
> `transmute()` to unify the type into a `T`.
>
> Introduce `AtomicIpml` to represent a basic type in Rust that has the
> direct mapping to an atomic implementation from C. This trait is sealed,
> and currently only `i32` and `i64` ipml this.
There seems to be quite a few instances of "impl" spelled as "ipml" here.
> Further, different methods are put into different `*Ops` trait groups,
> and this is for the future when smaller types like `i8`/`i16` are
> supported but only with a limited set of API (e.g. only set(), load(),
> xchg() and cmpxchg(), no add() or sub() etc).
>
> While the atomic mod is introduced, documentation is also added for
> memory models and data races.
>
> Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> my responsiblity on the Rust atomic mod.
>
> Signed-off-by: Boqun Feng
> ---
> MAINTAINERS| 4 +-
> rust/kernel/sync.rs| 1 +
> rust/kernel/sync/atomic.rs | 19
> rust/kernel/sync/atomic/ops.rs | 199 +
> 4 files changed, 222 insertions(+), 1 deletion(-)
> create mode 100644 rust/kernel/sync/atomic.rs
> create mode 100644 rust/kernel/sync/atomic/ops.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b77f4495dcf4..e09471027a63 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3635,7 +3635,7 @@ F:drivers/input/touchscreen/atmel_mxt_ts.c
> ATOMIC INFRASTRUCTURE
> M: Will Deacon
> M: Peter Zijlstra
> -R: Boqun Feng
> +M: Boqun Feng
> R: Mark Rutland
> L: linux-kernel@vger.kernel.org
> S: Maintained
> @@ -3644,6 +3644,8 @@ F:arch/*/include/asm/atomic*.h
> F: include/*/atomic*.h
> F: include/linux/refcount.h
> F: scripts/atomic/
> +F: rust/kernel/sync/atomic.rs
> +F: rust/kernel/sync/atomic/
This is why mod.rs files are superior :)
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Atomic primitives.
> +//!
> +//! These primitives have the same semantics as their C counterparts: and
> the precise definitions of
> +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory
> (Consistency) Model is the
> +//! only model for Rust code in kernel, and Rust's own atomics should be
> avoided.
> +//!
> +//! # Data races
> +//!
> +//! [`LKMM`] atomics have different rules regarding data races:
> +//!
> +//! - A normal read doesn't data-race with an atomic read.
This was fixed:
https://github.com/rust-lang/rust/pull/128778
> +mod private {
> +/// Sealed trait marker to disable customized impls on atomic
> implementation traits.
> +pub trait Sealed {}
> +}
Just make the trait unsafe?
Alice
Re: [RFC v2 02/13] rust: sync: Add basic atomic operation mapping framework
On Thu, Dec 12, 2024 at 6:07 PM Boqun Feng wrote:
>
> On Thu, Dec 12, 2024 at 11:51:23AM +0100, Alice Ryhl wrote:
> > On Fri, Nov 1, 2024 at 7:03 AM Boqun Feng wrote:
> > >
> > > Preparation for generic atomic implementation. To unify the
> > > ipmlementation of a generic method over `i32` and `i64`, the C side
> > > atomic methods need to be grouped so that in a generic method, they can
> > > be referred as ::, otherwise their parameters and return
> > > value are different between `i32` and `i64`, which would require using
> > > `transmute()` to unify the type into a `T`.
> > >
> > > Introduce `AtomicIpml` to represent a basic type in Rust that has the
> > > direct mapping to an atomic implementation from C. This trait is sealed,
> > > and currently only `i32` and `i64` ipml this.
> >
> > There seems to be quite a few instances of "impl" spelled as "ipml" here.
> >
>
> Will fix!
>
> > > Further, different methods are put into different `*Ops` trait groups,
> > > and this is for the future when smaller types like `i8`/`i16` are
> > > supported but only with a limited set of API (e.g. only set(), load(),
> > > xchg() and cmpxchg(), no add() or sub() etc).
> > >
> > > While the atomic mod is introduced, documentation is also added for
> > > memory models and data races.
> > >
> > > Also bump my role to the maintainer of ATOMIC INFRASTRUCTURE to reflect
> > > my responsiblity on the Rust atomic mod.
> > >
> > > Signed-off-by: Boqun Feng
> > > ---
> > > MAINTAINERS| 4 +-
> > > rust/kernel/sync.rs| 1 +
> > > rust/kernel/sync/atomic.rs | 19
> > > rust/kernel/sync/atomic/ops.rs | 199 +
> > > 4 files changed, 222 insertions(+), 1 deletion(-)
> > > create mode 100644 rust/kernel/sync/atomic.rs
> > > create mode 100644 rust/kernel/sync/atomic/ops.rs
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b77f4495dcf4..e09471027a63 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -3635,7 +3635,7 @@ F:drivers/input/touchscreen/atmel_mxt_ts.c
> > > ATOMIC INFRASTRUCTURE
> > > M: Will Deacon
> > > M: Peter Zijlstra
> > > -R: Boqun Feng
> > > +M: Boqun Feng
> > > R: Mark Rutland
> > > L: linux-kernel@vger.kernel.org
> > > S: Maintained
> > > @@ -3644,6 +3644,8 @@ F:arch/*/include/asm/atomic*.h
> > > F: include/*/atomic*.h
> > > F: include/linux/refcount.h
> > > F: scripts/atomic/
> > > +F: rust/kernel/sync/atomic.rs
> > > +F: rust/kernel/sync/atomic/
> >
> > This is why mod.rs files are superior :)
> >
>
> ;-) Not going to do anything right now, but let me think about this.
>
> > > @@ -0,0 +1,19 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Atomic primitives.
> > > +//!
> > > +//! These primitives have the same semantics as their C counterparts:
> > > and the precise definitions of
> > > +//! semantics can be found at [`LKMM`]. Note that Linux Kernel Memory
> > > (Consistency) Model is the
> > > +//! only model for Rust code in kernel, and Rust's own atomics should be
> > > avoided.
> > > +//!
> > > +//! # Data races
> > > +//!
> > > +//! [`LKMM`] atomics have different rules regarding data races:
> > > +//!
> > > +//! - A normal read doesn't data-race with an atomic read.
> >
> > This was fixed:
> > https://github.com/rust-lang/rust/pull/128778
> >
>
> Yeah, I was aware of that effort, and good to know it's finally merged.
> Thanks!
>
> This will be in 1.83, right? If so, we will still need the above until
> we bump up the minimal rustc version to 1.83 or beyond. I will handle
> this properly with the minimal rustc 1.83 (i.e. if this goes in first,
> will send a follow up patch). I will also mention in the above that this
> has been changed in 1.83.
>
> This also reminds that I should add that LKMM allows mixed-size atomic
> accesses (as non data race), I will add that in the version.
This is just documentation. I don't think you need to do any special
MSRV handling.
> > > +mod private {
> > > +/// Sealed trait marker to disable customized impls on atomic
> > > implementation traits.
> > > +pub trait Sealed {}
> > > +}
> >
> > Just make the trait unsafe?
> >
>
> And make the safety requirement of `AtomicImpl` something like:
>
> The type must have the implementation for atomic operations.
>
> ? Hmm.. I don't think that's a good safety requirement TBH. Actually the
> reason that we need to restrict `AtomicImpl` types is more of an
> iplementation issue (the implementation need to be done if we want to
> support i8 or i16) rather than safety issue. So a sealed trait is proper
> here. Does this make sense? Or am I missing something?
Where is the AtomicImpl trait used?
Alice
Re: [RFC v2 04/13] rust: sync: atomic: Add generic atomics
On Thu, Dec 12, 2024 at 6:34 PM Boqun Feng wrote:
>
> On Thu, Dec 12, 2024 at 11:57:07AM +0100, Alice Ryhl wrote:
> [...]
> > > diff --git a/rust/kernel/sync/atomic/generic.rs
> > > b/rust/kernel/sync/atomic/generic.rs
> > > new file mode 100644
> > > index ..204da38e2691
> > > --- /dev/null
> > > +++ b/rust/kernel/sync/atomic/generic.rs
> > > @@ -0,0 +1,253 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +
> > > +//! Generic atomic primitives.
> > > +
> > > +use super::ops::*;
> > > +use super::ordering::*;
> > > +use crate::types::Opaque;
> > > +
> > > +/// A generic atomic variable.
> > > +///
> > > +/// `T` must impl [`AllowAtomic`], that is, an [`AtomicImpl`] has to be
> > > chosen.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// Doing an atomic operation while holding a reference of [`Self`]
> > > won't cause a data race, this
> > > +/// is guaranteed by the safety requirement of [`Self::from_ptr`] and
> > > the extra safety requirement
> > > +/// of the usage on pointers returned by [`Self::as_ptr`].
> > > +#[repr(transparent)]
> > > +pub struct Atomic(Opaque);
> > > +
> > > +// SAFETY: `Atomic` is safe to share among execution contexts because
> > > all accesses are atomic.
> > > +unsafe impl Sync for Atomic {}
> >
> > Surely it should also be Send?
> >
>
> It's `Send` here because `Opaque` is `Send` when `T` is `Send`. And
> in patch #9, I changed the definition of `AllowAtomic`, which is not a
> subtrait of `Send` anymore, and an `impl Send` block was added there.
>
> > > +/// Atomics that support basic atomic operations.
> > > +///
> > > +/// TODO: Unless the `impl` is a `#[repr(transparet)]` new type of an
> > > existing [`AllowAtomic`], the
> > > +/// impl block should be only done in atomic mod. And currently only
> > > basic integer types can
> > > +/// implement this trait in atomic mod.
> >
> > What's up with this TODO? Can't you just write an appropriate safety
> > requirement?
> >
>
> Because the limited scope of types that allows atomic is an artificial
> choice, i.e. we want to start with a limited number of types and make
> forward progress, and the types that we don't want to support atomics
> for now are not because of safety reasons, but more of a lack of
> users/motivations. So I don't think this is something we should use
> safety requirement to describe.
I found the wording very confusing. Could you reword it to say
something about future possibilities?
> > > +/// # Safety
> > > +///
> > > +/// [`Self`] must have the same size and alignment as [`Self::Repr`].
> > > +pub unsafe trait AllowAtomic: Sized + Send + Copy {
> > > +/// The backing atomic implementation type.
> > > +type Repr: AtomicImpl;
> > > +
> > > +/// Converts into a [`Self::Repr`].
> > > +fn into_repr(self) -> Self::Repr;
> > > +
> > > +/// Converts from a [`Self::Repr`].
> > > +fn from_repr(repr: Self::Repr) -> Self;
> >
> > What do you need these methods for?
> >
>
> Converting a `AtomicImpl` value (currently only `i32` and `i64`) to a
> `AllowAtomic` value without using transmute in `impl` block of
> `Atomic`. Any better idea?
You could use transmute?
Alice
Re: [RFC PATCH RESEND v2 1/2] mm/memfd: Add support for F_SEAL_FUTURE_EXEC to memfd
On Tue, Jan 7, 2025 at 6:21 AM Jeff Xu wrote:
> Do you know which code checks for VM_MAYEXEC flag in the mprotect code
> path ? it isn't obvious to me, i.e. when I grep the VM_MAYEXEC inside
> mm path, it only shows one place in mprotect and that doesn't do the
> work.
>
> ~/mm/mm$ grep VM_MAYEXEC *
> mmap.c: mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
> mmap.c: vm_flags &= ~VM_MAYEXEC;
> mprotect.c: if (rier && (vma->vm_flags & VM_MAYEXEC))
> nommu.c: vm_flags |= VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
> nommu.c: vm_flags |= VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
The check happens here:
/* newflags >> 4 shift VM_MAY% in place of VM_% */
if ((newflags & ~(newflags >> 4)) & VM_ACCESS_FLAGS) {
error = -EACCES;
break;
}
Alice
Re: [RFC PATCH v2 2/2] selftests/memfd: Add tests for F_SEAL_FUTURE_EXEC
On Fri, Dec 27, 2024 at 2:52 AM Isaac J. Manjarres wrote: > > Add tests to ensure that F_SEAL_FUTURE_EXEC behaves as expected. > > Signed-off-by: Isaac J. Manjarres Reviewed-by: Alice Ryhl
Re: [PATCH v4 2/4] rust: str: implement `strip_prefix` for `BStr`
On Thu, Jan 9, 2025 at 11:56 AM Andreas Hindborg wrote: > > Implement `strip_prefix` for `BStr` by deferring to `slice::strip_prefix` > on the underlying `&[u8]`. > > Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl
Re: [PATCH v11 2/3] rust: add parameter support to the `module!` macro
On Mon, May 05, 2025 at 11:55:33AM +0200, Andreas Hindborg wrote: > "Alice Ryhl" writes: > > > On Fri, May 02, 2025 at 02:16:35PM +0200, Andreas Hindborg wrote: > > It would be a use-after-free to > > access it during module teardown. For example, what if I access this > > static during its own destructor? Or during the destructor of another > > module parameter? > > Yes, that is a problem. > > We can get around it for now by just not calling `free` for now. We only > support simple types that do not need drop. I think we would have to > seal the `ModuleParam` trait for this. > > For a proper solution, we could > - Require a token to read the parameter. > - Synchronize on a module private field and return an option from the >parameter getter. This would require module exit to run before param >free. I think this is the case, but I did not check. > - Use a `Revocable` and revoke the parameter in `free`. > > Any other ideas or comments on the outlined solutions? I think the simplest you can do right now is trait ModuleParam: Copy so that it can't contain any non-trivial values. That way you don't need Drop either. Long term, I think we need a way to detect whether it's safe to access module globals. The exact same problem applies to the existing global for the module itself - except it's worse there because we can't access that one during init either. Alice
Re: [PATCH v11 2/3] rust: add parameter support to the `module!` macro
On Fri, May 02, 2025 at 02:16:35PM +0200, Andreas Hindborg wrote:
> Add support for module parameters to the `module!` macro. Implement read
> only support for integer types without `sysfs` support.
>
> Acked-by: Petr Pavlu # from modules perspective
> Tested-by: Daniel Gomez
> Signed-off-by: Andreas Hindborg
> +unsafe extern "C" fn set_param(
> +val: *const kernel::ffi::c_char,
> +param: *const crate::bindings::kernel_param,
> +) -> core::ffi::c_int
> +where
> +T: ModuleParam,
> +{
> +// NOTE: If we start supporting arguments without values, val _is_
> allowed
> +// to be null here.
> +if val.is_null() {
> +// TODO: Use pr_warn_once available.
> +crate::pr_warn!("Null pointer passed to `module_param::set_param`");
> +return EINVAL.to_errno();
> +}
> +
> +// SAFETY: By function safety requirement, val is non-null and
> +// null-terminated. By C API contract, `val` is live and valid for reads
> +// for the duration of this function.
> +let arg = unsafe { CStr::from_char_ptr(val) };
> +
> +crate::error::from_result(|| {
> +let new_value = T::try_from_param_arg(arg)?;
> +
> +// SAFETY: `param` is guaranteed to be valid by C API contract
> +// and `arg` is guaranteed to point to an instance of `T`.
> +let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut T };
> +
> +// SAFETY: `old_value` is valid for writes, as we have exclusive
> +// access. `old_value` is pointing to an initialized static, and
> +// so it is properly initialized.
> +unsafe { core::ptr::replace(old_value, new_value) };
You don't use the return value of this, so this is equivalent to
unsafe { *old_value = new_value };
> +macro_rules! make_param_ops {
> +($ops:ident, $ty:ty) => {
> +///
> +/// Static [`kernel_param_ops`](srctree/include/linux/moduleparam.h)
> +/// struct generated by `make_param_ops`
> +#[doc = concat!("for [`", stringify!($ty), "`].")]
> +pub static $ops: $crate::bindings::kernel_param_ops =
> $crate::bindings::kernel_param_ops {
> +flags: 0,
> +set: Some(set_param::<$ty>),
> +get: None,
> +free: Some(free::<$ty>),
You could potentially only include `free` if
`core::mem::needs_drop::()` as an optimization.
> +fn emit_params(&mut self, info: &ModuleInfo) {
> +let Some(params) = &info.params else {
> +return;
> +};
> +
> +for param in params {
> +let ops = param_ops_path(¶m.ptype);
> +
> +// Note: The spelling of these fields is dictated by the user
> space
> +// tool `modinfo`.
> +self.emit_param("parmtype", ¶m.name, ¶m.ptype);
> +self.emit_param("parm", ¶m.name, ¶m.description);
> +
> +write!(
> +self.param_buffer,
> +"
> +pub(crate) static {param_name}:
> +
> ::kernel::module_param::ModuleParamAccess<{param_type}> =
> +
> ::kernel::module_param::ModuleParamAccess::new({param_default});
Is this global accessible to the user? It would be a use-after-free to
access it during module teardown. For example, what if I access this
static during its own destructor? Or during the destructor of another
module parameter?
Alice
Re: [PATCH v8 2/7] rust: str: implement `Index` for `BStr`
On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg wrote: > > The `Index` implementation on `BStr` was lost when we switched `BStr` from > a type alias of `[u8]` to a newtype. Add back `Index` by implementing > `Index` for `BStr` when `Index` would be implemented for `[u8]`. > > Reviewed-by: Daniel Almeida > Tested-by: Daniel Almeida > Reviewed-by: Fiona Behrens > Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl
Re: [PATCH] rust: replace length checks with match
On Tue, May 27, 2025 at 12:09:36PM -0400, Tamir Duberstein wrote: > Use a match expression with slice patterns instead of length checks and > indexing. The result is more idiomatic, which is a better example for > future Rust code authors. > > Signed-off-by: Tamir Duberstein Reviewed-by: Alice Ryhl
Re: [PATCH v14 1/7] rust: sync: add `OnceLock`
On Wed, Jul 2, 2025 at 3:19 PM Andreas Hindborg wrote:
>
> Introduce the `OnceLock` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg
This type provides no way to wait for initialization to finish if it's
ongoing. Do you not need that?
> ---
> rust/kernel/sync.rs | 1 +
> rust/kernel/sync/once_lock.rs | 104
> ++
> 2 files changed, 105 insertions(+)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index c7c0e552bafe..f2ee07315091 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -15,6 +15,7 @@
> mod condvar;
> pub mod lock;
> mod locked_by;
> +pub mod once_lock;
I would add a re-export so that users can import this as kernel::sync::OnceLock.
> pub mod poll;
> pub mod rcu;
>
> diff --git a/rust/kernel/sync/once_lock.rs b/rust/kernel/sync/once_lock.rs
> new file mode 100644
> index ..cd311bea3919
> --- /dev/null
> +++ b/rust/kernel/sync/once_lock.rs
> @@ -0,0 +1,104 @@
> +//! A container that can be initialized at most once.
> +
> +use super::atomic::ordering::Acquire;
> +use super::atomic::ordering::Release;
> +use super::atomic::Atomic;
> +use kernel::types::Opaque;
> +
> +/// A container that can be populated at most once. Thread safe.
> +///
> +/// Once the a [`OnceLock`] is populated, it remains populated by the same
> object for the
> +/// lifetime `Self`.
> +///
> +/// # Invariants
> +///
> +/// `init` tracks the state of the container:
> +///
> +/// - If the container is empty, `init` is `0`.
> +/// - If the container is mutably accessed, `init` is `1`.
I would phrase this as "being initialized" instead of "mutably
accessed". I initially thought this was talking about someone calling
a &mut self method.
> +/// - If the container is populated and ready for shared access, `init` is
> `2`.
> +///
> +/// # Example
> +///
> +/// ```
> +/// # use kernel::sync::once_lock::OnceLock;
> +/// let value = OnceLock::new();
> +/// assert_eq!(None, value.as_ref());
> +///
> +/// let status = value.populate(42u8);
> +/// assert_eq!(true, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +///
> +/// let status = value.populate(101u8);
> +/// assert_eq!(false, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +/// ```
> +pub struct OnceLock {
> +init: Atomic,
> +value: Opaque,
Opaque does not destroy the inner value. You are missing a destructor.
> +}
> +
> +impl Default for OnceLock {
> +fn default() -> Self {
> +Self::new()
> +}
> +}
> +
> +impl OnceLock {
> +/// Create a new [`OnceLock`].
> +///
> +/// The returned instance will be empty.
> +pub const fn new() -> Self {
> +// INVARIANT: The container is empty and we set `init` to `0`.
> +Self {
> +value: Opaque::uninit(),
> +init: Atomic::new(0),
> +}
> +}
> +
> +/// Get a reference to the contained object.
> +///
> +/// Returns [`None`] if this [`OnceLock`] is empty.
> +pub fn as_ref(&self) -> Option<&T> {
> +if self.init.load(Acquire) == 2 {
> +// SAFETY: As determined by the load above, the object is ready
> for shared access.
> +Some(unsafe { &*self.value.get() })
> +} else {
> +None
> +}
> +}
> +
> +/// Populate the [`OnceLock`].
> +///
> +/// Returns `true` if the [`OnceLock`] was successfully populated.
> +pub fn populate(&self, value: T) -> bool {
> +// INVARIANT: We obtain exclusive access to the contained allocation
> and write 1 to
> +// `init`.
> +if let Ok(0) = self.init.cmpxchg(0, 1, Acquire) {
This acquire can be Relaxed. All other accesses to self.value
synchronize with the release store below, so you do not need acquire
here to obtain exclusive access.
> +// SAFETY: We obtained exclusive access to the contained object.
> +unsafe { core::ptr::write(self.value.get(), value) };
> +// INVARIANT: We release our exclusive access and transition the
> object to shared
> +// access.
> +self.init.store(2, Release);
> +true
> +} else {
> +false
> +}
> +}
> +}
> +
> +impl OnceLock {
> +/// Get a copy of the contained object.
> +///
> +/// Returns [`None`] if the [`OnceLock`] is empty.
> +pub fn copy(&self) -> Option {
> +if self.init.load(Acquire) == 2 {
> +// SAFETY: As determined by the load above, the object is ready
> for shared access.
> +Some(unsafe { *self.value.get() })
> +} else {
> +None
> +}
> +}
> +}
>
> --
> 2.47.2
>
>
Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
On Tue, Jul 8, 2025 at 10:48 AM Andreas Hindborg wrote:
>
> "Alice Ryhl" writes:
>
> > On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg
> > wrote:
> >>
> >> Introduce the `SetOnce` type, a container that can only be written once.
> >> The container uses an internal atomic to synchronize writes to the internal
> >> value.
> >>
> >> Signed-off-by: Andreas Hindborg
> >
> > LGTM:
> > Reviewed-by: Alice Ryhl
> >
> >> +impl Drop for SetOnce {
> >> +fn drop(&mut self) {
> >> +if self.init.load(Acquire) == 2 {
> >> +// SAFETY: By the type invariants of `Self`, `self.init == 2`
> >> means that `self.value`
> >> +// contains a valid value. We have exclusive access, as we
> >> hold a `mut` reference to
> >> +// `self`.
> >> +unsafe { drop_in_place(self.value.get()) };
> >
> > This load does not need to be Acquire. It can be a Relaxed load or
> > even an unsynchronized one since the access is exclusive.
>
> Right, that is actually very cool. My rationale was that if a reference
> has been shared to another thread of execution, we would need to
> synchronize here to see a possible initialization from that other
> thread. But I guess it is impossible to end the lifetime of a reference
> without doing a synchronization somewhere else.
Yup, a mutable reference generally implies synchronization.
Alice
Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg wrote:
>
> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg
LGTM:
Reviewed-by: Alice Ryhl
> +impl Drop for SetOnce {
> +fn drop(&mut self) {
> +if self.init.load(Acquire) == 2 {
> +// SAFETY: By the type invariants of `Self`, `self.init == 2`
> means that `self.value`
> +// contains a valid value. We have exclusive access, as we hold
> a `mut` reference to
> +// `self`.
> +unsafe { drop_in_place(self.value.get()) };
This load does not need to be Acquire. It can be a Relaxed load or
even an unsynchronized one since the access is exclusive.
Alice
Re: [PATCH v14 1/7] rust: sync: add `OnceLock`
On Wed, Jul 2, 2025 at 3:54 PM Andreas Hindborg wrote: > > "Alice Ryhl" writes: > > > On Wed, Jul 2, 2025 at 3:19 PM Andreas Hindborg > > wrote: > >> > >> Introduce the `OnceLock` type, a container that can only be written once. > >> The container uses an internal atomic to synchronize writes to the internal > >> value. > >> > >> Signed-off-by: Andreas Hindborg > > > > This type provides no way to wait for initialization to finish if it's > > ongoing. Do you not need that? > > I don't, and in my use case it would cause a deadlock to wait. Anyway, > it might be useful to others. Would you add it now, or wait for a user? Waiting would require additional fields so it should probably be a different type. It's more that we probably want the OnceLock name for that other type for consistency with stdlib, so perhaps this should be renamed? The name could be SetOnce or similar. Alice
Re: [PATCH v14 1/7] rust: sync: add `OnceLock`
On Wed, Jul 2, 2025 at 5:07 PM Benno Lossin wrote:
>
> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
> > +impl OnceLock {
> > +/// Get a copy of the contained object.
> > +///
> > +/// Returns [`None`] if the [`OnceLock`] is empty.
> > +pub fn copy(&self) -> Option {
> > +if self.init.load(Acquire) == 2 {
> > +// SAFETY: As determined by the load above, the object is
> > ready for shared access.
> > +Some(unsafe { *self.value.get() })
> > +} else {
> > +None
> > +}
>
> The impl can just be:
>
> self.as_ref().copied()
>
> Would it make sense for this function to take `self` instead & we make
> the `OnceLock` also `Copy` if `T: Copy`? Maybe not...
Atomics are not Copy.
Alice
