| Message ID | 20240606-tracepoint-v1-1-6551627bf51b@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Tracepoints and static branch/call in Rust | expand |
On Thu, Jun 06, 2024 at 03:05:24PM +0000, Alice Ryhl wrote: > 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. This is absolutely unreadable gibberish -- how am I supposed to keep this in sync with the rest of the static_call infrastructure? > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > 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 000000000000..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 >
On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> 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. 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. Cheers, Miguel
On Thu, Jun 06, 2024 at 09:09:00PM +0200, Miguel Ojeda wrote: > On Thu, Jun 6, 2024 at 7:19 PM Peter Zijlstra <peterz@infradead.org> 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 ? > 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.
Peter Zijlstra <peterz@infradead.org> 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 <peterz@infradead.org> 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
On Fri, Jun 07, 2024 at 09:43:29AM +0000, Alice Ryhl wrote: > Peter Zijlstra <peterz@infradead.org> 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 <peterz@infradead.org> 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. I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going to go away. So I'm reading your macros do NOT rule. > 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. The above exaple fails, because the next token is :ident, whatever the heck that might be. Also, extra points for line-noise due to lack of whitespace. > So for example, you might invoke the macro like this: > > static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct)); static_call NOT (blah dog blah); > 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 Like I said before, the creator of Rust must've been an esoteric language freak and must've wanted to make this unreadable on purpose :/ Also, why the white space beteen the :: scope operator and the [< thing? that's just weird. I would then expect the output to be: ...::bindings:: __SCK__my_static_key > 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; But it doesn't, so it isn't unmodified, it seems to strip whitespace. > 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. You can have clang read the header files and compile them into Intermediate-Representation, and have it splice the lot into the Rust crap's IR and voila, compile time. You just need to extend the rust thing to be able to consume C header files.
On Fri, Jun 7, 2024 at 12:52 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Fri, Jun 07, 2024 at 09:43:29AM +0000, Alice Ryhl wrote: > > Peter Zijlstra <peterz@infradead.org> 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 <peterz@infradead.org> 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. > > I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going > to go away. So I'm reading your macros do NOT rule. So you already understand ! in two ways, but you don't want to add a third? That seems to violate the Zero One Infinity rule. :) https://en.wikipedia.org/wiki/Zero_one_infinity_rule > > 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. > > The above exaple fails, because the next token is :ident, whatever the > heck that might be. Also, extra points for line-noise due to lack of > whitespace. The :ident part means that $name should be parsed as an identifier. Similarly, the :expr part means that $args should be parsed as an expression. It doesn't mean that the input should literally contain ":ident". > > So for example, you might invoke the macro like this: > > > > static_call!(tp_func_my_tracepoint(__data, &mut my_task_struct)); > > static_call NOT (blah dog blah); > > > 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 > > Like I said before, the creator of Rust must've been an esoteric > language freak and must've wanted to make this unreadable on purpose :/ > > Also, why the white space beteen the :: scope operator and the [< thing? > that's just weird. I would then expect the output to be: > > ...::bindings:: __SCK__my_static_key Sorry, you are right. There is a space in the output. > > 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; > > But it doesn't, so it isn't unmodified, it seems to strip whitespace. Thanks for the correction. The actual output is: $crate::bindings:: __SCK__my_static_key; However, although whitespace is generally not used here, the syntax allows it. > > 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. > > You can have clang read the header files and compile them into > Intermediate-Representation, and have it splice the lot into the Rust > crap's IR and voila, compile time. > > You just need to extend the rust thing to be able to consume C header > files. I wish! There are people, including me, who want this. See e.g. this very recent document: https://rust-lang.github.io/rust-project-goals/2024h2/Seamless-C-Support.html But there are also people who dislike the idea, so it does not have unanimous support yet, unfortunately. Ultimately, I have to work with what exists today. Alice
On Fri, Jun 7, 2024 at 12:52 PM Peter Zijlstra <peterz@infradead.org> wrote: > > I'm sorry, but 30+ years of reading ! as NOT (or factorial) isn't going > to go away. So I'm reading your macros do NOT rule. It makes it clear what is macro call or not. They could have gone for UPPERCASE names (for instance), yes. On the other hand, they do not work like C macros and are ~hygienic, so it also makes sense to avoid confusion here. I mean, I am not suggesting to do a CPP-pass to Rust files, but if someone really, really wanted to mix them in a single file, it would be nice to not confuse the two kinds. :) Generally they feel "closer" to the language (given what they do/support) compared to the CPP ones, so it also makes sense they don't "shout" so much compared to UPPERCASE, if that makes sense. > The above exaple fails, because the next token is :ident, whatever the > heck that might be. Also, extra points for line-noise due to lack of > whitespace. $name:ident means "match what Rust would consider an identifier here and call it $name for the purposes of this macro". So, for instance, $x:ident matches: a a2 a_b But it would not match: 2a a-b a _b For the usual reasons why those are not identifiers. https://godbolt.org/z/G7v4j67dc fn f(x: i32) -> i32 { x * 2 } macro_rules! f { ($x:ident) => { $x * 2 } } fn main() { let a = 42; let b = f(a); // Function. let c = f!(a); // Macro. //let c = f!(a2); // Works, but the variable does not exist. //let c = f!(2a); // Error: no rules expected the token `2a`. //let c = f!(a_b); // Works, but the variable does not exist. //let c = f!(a-b); // Error: no rules expected the token `-`. //let c = f!(a_ b); // Error: no rules expected the token `b`. println!("{a} {b} {c}"); } I hope this makes it clearer. > You just need to extend the rust thing to be able to consume C header > files. I agree, because in practice it is quite useful for a language like Rust that consuming C header files is "natively" supported. Though it also has downsides and is a big decision, which is why, like Alice mentioned, some people agree, and some people don't. Nevertheless, we have been doing our best for a long time to get the best we can for the kernel -- just 2 days ago we told the Rust project in one of our meetings that it would be nice to see that particular "project goal" from that document realized (among others). Cheers, Miguel
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 000000000000..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};
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 <aliceryhl@google.com> --- rust/kernel/lib.rs | 1 + rust/kernel/static_call.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+)