* [PATCH] rust: kernel: Support more jump_label api
@ 2025-11-04 2:04 chenmiao
2025-11-04 14:07 ` Alice Ryhl
2025-11-04 14:14 ` Miguel Ojeda
0 siblings, 2 replies; 12+ messages in thread
From: chenmiao @ 2025-11-04 2:04 UTC (permalink / raw)
To: ojeda
Cc: Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
The initial implementation of arch_static_branch was achieved by accessing
the offset from the original type. However, this approach extended the
path and introduced redundant calculations when dealing with types like
`static_key_true/false`, as shown below:
```
static_brach_unlikely(tp, tracepoint, key)
=> tracepoint->key->key
=> &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
=> off: tracepoint->key - tracepoint
```
In practice, the implementation of `arch_static_branch` overlooked many
detailed descriptions. To improve clarity, additional comments have been
added to the original logic. The approach has been modified to directly
locate the corresponding `static_key` instead of using offsets, thereby
reducing computational overhead.
If finding the offset from the primitive type is necessary for this
implementation, I will abandon this change.
Additionally, support for the `static_branch_enable/disable` APIs has been
introduced.
Signed-off-by: chenmiao <chenmiao@openatom.club>
---
.../generated_arch_static_branch_asm.rs.S | 2 +-
rust/kernel/jump_label.rs | 169 ++++++++++++++++--
2 files changed, 155 insertions(+), 16 deletions(-)
diff --git a/rust/kernel/generated_arch_static_branch_asm.rs.S b/rust/kernel/generated_arch_static_branch_asm.rs.S
index 2afb638708db3..08603dc2d61e7 100644
--- a/rust/kernel/generated_arch_static_branch_asm.rs.S
+++ b/rust/kernel/generated_arch_static_branch_asm.rs.S
@@ -4,4 +4,4 @@
// Cut here.
-::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{symb} + {off} + {branch}", "{l_yes}"))
+::kernel::concat_literals!(ARCH_STATIC_BRANCH_ASM("{real_key} + {branch}", "{l_yes}"))
diff --git a/rust/kernel/jump_label.rs b/rust/kernel/jump_label.rs
index 4e974c768dbd5..12ae78f95c761 100644
--- a/rust/kernel/jump_label.rs
+++ b/rust/kernel/jump_label.rs
@@ -6,22 +6,63 @@
//!
//! C header: [`include/linux/jump_label.h`](srctree/include/linux/jump_label.h).
-/// Branch based on a static key.
+/// The key used for the static_key_false/true.
///
-/// Takes three arguments:
+/// If the key just contains a static_key, like: `struct tracepoint`;
///
-/// * `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`.
+/// ```
+/// pub struct tracepoint {
+/// ...,
+/// key: static_key,
+/// ...,
+/// }
///
-/// # Safety
+/// // When you use the tracepoint as the parameter.
+/// if static_branch_unlikely!(tp, tracepoint, key) {
+/// // Do something
+/// }
///
-/// The macro must be used with a real static key defined by C.
-#[macro_export]
-macro_rules! static_branch_unlikely {
- ($key:path, $keytyp:ty, $field:ident) => {{
+/// // It just like:
+/// let _key: *const crate::bindings::tracepoint = ::core::ptr::addr_of!($key);
+/// let _key: *const crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).key);
+/// let _key: *const crate::bindings::static_key = _key.cast();
+/// ```
+///
+/// If the key just contains a single static_key, like: `struct static_key_false`;
+///
+/// ```
+/// pub struct static_key_false {
+/// key: static_key,
+/// }
+///
+/// // When you use the static_key_false as the parameter.
+/// if static_branch_unlikely!(key) {
+/// // Do something
+/// }
+///
+/// // It just like:
+/// let _key: *const crate::bindings::static_key_false = ::core::ptr::addr_of!($key);
+/// let _key: *const crate::bindings::static_key = _key.cast();
+/// ```
+///
+macro_rules! __static_branch_base {
+ ($basety:ty, $branch:expr, $key:path) => {{
+ let _key: *const $basety = ::core::ptr::addr_of!($key);
+ let _key: *const $crate::bindings::static_key = _key.cast();
+
+ #[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, $branch }
+ }
+ }};
+ ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
let _key: *const $keytyp = ::core::ptr::addr_of!($key);
- let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
+ let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
let _key: *const $crate::bindings::static_key = _key.cast();
#[cfg(not(CONFIG_JUMP_LABEL))]
@@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
}
#[cfg(CONFIG_JUMP_LABEL)]
- $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
+ {
+ $crate::jump_label::arch_static_branch! { _key, $branch }
+ }
+ }};
+}
+
+/// Branch based on a static key.
+///
+/// Takes two type arguments:
+///
+/// First Type takes one argument:
+///
+/// * `key` - the static variable containing the `static_key`.
+///
+/// Second Type 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
+///
+/// ```
+/// let tp: tracepoint = tracepoint::new();
+/// if static_key_likely!(tp, tracepoint, key) {
+/// // Do something
+/// }
+///
+/// let key: static_key_false = static_key_true::new();
+/// if static_key_likely!(key) {
+/// // Do something
+/// }
+/// ```
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_branch_likely {
+ ($key:path) => {{
+ __static_branch_base! { $crate::bindings::static_key_true, true, $key }
+ }};
+ ($key:path, $keytyp:ty, $field:ident) => {{
+ __static_branch_base! { $crate::bindings::static_key_true, true, $key, $keytyp, $field }
+ }};
+}
+pub use static_branch_likely;
+
+/// Branch based on a static key.
+///
+/// Takes two type arguments:
+///
+/// First Type takes one argument:
+///
+/// * `key` - the static variable containing the `static_key`.
+///
+/// Second Type 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
+///
+/// ```
+/// let tp: tracepoint = tracepoint::new();
+/// if static_key_unlikely!(tp, tracepoint, key) {
+/// // Do something
+/// }
+///
+/// let key: static_key_false = static_key_false::new();
+/// if static_key_unlikely!(key) {
+/// // Do something
+/// }
+/// ```
+///
+/// The macro must be used with a real static key defined by C.
+#[macro_export]
+macro_rules! static_branch_unlikely {
+ ($key:path) => {{
+ static_branch_base! { $crate::bindings::static_key_false, false, $key }
+ }};
+ ($key:path, $keytyp:ty, $field:ident) => {{
+ static_branch_base! { $crate::bindings::static_key_false, false, $key, $keytyp, $field }
}};
}
pub use static_branch_unlikely;
@@ -46,14 +168,13 @@ macro_rules! static_branch_unlikely {
#[doc(hidden)]
#[cfg(CONFIG_JUMP_LABEL)]
macro_rules! arch_static_branch {
- ($key:path, $keytyp:ty, $field:ident, $branch:expr) => {'my_label: {
+ ($key:path, $branch:expr) => {'my_label: {
$crate::asm!(
include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_static_branch_asm.rs"));
l_yes = label {
break 'my_label true;
},
- symb = sym $key,
- off = const ::core::mem::offset_of!($keytyp, $field),
+ real_key = sym $key,
branch = const $crate::jump_label::bool_to_int($branch),
);
@@ -72,3 +193,21 @@ macro_rules! arch_static_branch {
pub const fn bool_to_int(b: bool) -> i32 {
b as i32
}
+
+/// Enable a static branch.
+#[macro_export]
+macro_rules! static_branch_enable {
+ ($key:path) => {{
+ let _key: *const $crate::bindings::static_key = ::core::ptr::addr_of!($key);
+ $crate::bindings::static_key_enable(_key.cast_mut());
+ }};
+}
+
+/// Disable a static branch.
+#[macro_export]
+macro_rules! static_branch_disable {
+ ($key:path) => {{
+ let _key: *const $crate::bindings::static_key = ::core::ptr::addr_of!($key);
+ $crate::bindings::static_key_disable(_key.cast_mut());
+ }};
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-04 2:04 [PATCH] rust: kernel: Support more jump_label api chenmiao
@ 2025-11-04 14:07 ` Alice Ryhl
2025-11-05 13:28 ` Chen Miao
2025-11-04 14:14 ` Miguel Ojeda
1 sibling, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2025-11-04 14:07 UTC (permalink / raw)
To: chenmiao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list
On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
> The initial implementation of arch_static_branch was achieved by accessing
> the offset from the original type. However, this approach extended the
> path and introduced redundant calculations when dealing with types like
> `static_key_true/false`, as shown below:
>
> ```
> static_brach_unlikely(tp, tracepoint, key)
> => tracepoint->key->key
> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
> => off: tracepoint->key - tracepoint
> ```
>
> In practice, the implementation of `arch_static_branch` overlooked many
> detailed descriptions. To improve clarity, additional comments have been
> added to the original logic. The approach has been modified to directly
> locate the corresponding `static_key` instead of using offsets, thereby
> reducing computational overhead.
>
> If finding the offset from the primitive type is necessary for this
> implementation, I will abandon this change.
>
> Additionally, support for the `static_branch_enable/disable` APIs has been
> introduced.
>
> Signed-off-by: chenmiao <chenmiao@openatom.club>
> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
> let _key: *const $crate::bindings::static_key = _key.cast();
>
> #[cfg(not(CONFIG_JUMP_LABEL))]
> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
> }
>
> #[cfg(CONFIG_JUMP_LABEL)]
> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
> + {
> + $crate::jump_label::arch_static_branch! { _key, $branch }
> + }
So ... this is changing from $key to _key. That's replacing the global
variable with a local variable holding a pointer to the global variable.
However, the arch_static_branch! macro uses the `sym` operand which
requires you to pass it the global directly.
Did you try this code? I don't believe it will work.
Alice
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-04 2:04 [PATCH] rust: kernel: Support more jump_label api chenmiao
2025-11-04 14:07 ` Alice Ryhl
@ 2025-11-04 14:14 ` Miguel Ojeda
2025-11-05 13:35 ` Chen Miao
1 sibling, 1 reply; 12+ messages in thread
From: Miguel Ojeda @ 2025-11-04 14:14 UTC (permalink / raw)
To: chenmiao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
Hi chenmiao,
A few procedural bits below for future versions/patches...
On Tue, Nov 4, 2025 at 3:04 AM chenmiao <chenmiao@openatom.club> wrote:
>
> If finding the offset from the primitive type is necessary for this
> implementation, I will abandon this change.
In general, this should not be part of the commit message, but go
below the `---` line.
And if the commit is not ready, typically it should be marked as RFC.
> Additionally, support for the `static_branch_enable/disable` APIs has been
> introduced.
Please split different changes in different patches if possible.
> Signed-off-by: chenmiao <chenmiao@openatom.club>
In another email you used Chen Miao in the From -- in general, please
note that the SoB is supposed to have the "full name" ("known
identity") as you would normally write it. I mention this since
sometimes people use the local part of the email address, i.e. `...@`,
but that is usually not the normal spelling of a full name. Of course,
please ignore this if it is correct already.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-04 14:07 ` Alice Ryhl
@ 2025-11-05 13:28 ` Chen Miao
2025-11-05 13:38 ` Alice Ryhl
0 siblings, 1 reply; 12+ messages in thread
From: Chen Miao @ 2025-11-05 13:28 UTC (permalink / raw)
To: Alice Ryhl
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list,
hust-os-kernel-patches
On 11/4/2025 10:07 PM, Alice Ryhl wrote:
> On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
>> The initial implementation of arch_static_branch was achieved by accessing
>> the offset from the original type. However, this approach extended the
>> path and introduced redundant calculations when dealing with types like
>> `static_key_true/false`, as shown below:
>>
>> ```
>> static_brach_unlikely(tp, tracepoint, key)
>> => tracepoint->key->key
>> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
>> => off: tracepoint->key - tracepoint
>> ```
>>
>> In practice, the implementation of `arch_static_branch` overlooked many
>> detailed descriptions. To improve clarity, additional comments have been
>> added to the original logic. The approach has been modified to directly
>> locate the corresponding `static_key` instead of using offsets, thereby
>> reducing computational overhead.
>>
>> If finding the offset from the primitive type is necessary for this
>> implementation, I will abandon this change.
>>
>> Additionally, support for the `static_branch_enable/disable` APIs has been
>> introduced.
>>
>> Signed-off-by: chenmiao <chenmiao@openatom.club>
>> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
>> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
>> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
>> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
>> let _key: *const $crate::bindings::static_key = _key.cast();
>>
>> #[cfg(not(CONFIG_JUMP_LABEL))]
>> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
>> }
>>
>> #[cfg(CONFIG_JUMP_LABEL)]
>> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
>> + {
>> + $crate::jump_label::arch_static_branch! { _key, $branch }
>> + }
> So ... this is changing from $key to _key. That's replacing the global
> variable with a local variable holding a pointer to the global variable.
> However, the arch_static_branch! macro uses the `sym` operand which
> requires you to pass it the global directly.
>
> Did you try this code? I don't believe it will work.
>
> Alice
I'm very sorry for making a fatal mistake. My misunderstanding of sym led to
this issue, so I shouldn't make any changes to that part. However, regarding
the other modifications, I believe it is necessary to support the direct
passing of structures similar to `static_key_false`, just as in C language.
Chen Miao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-04 14:14 ` Miguel Ojeda
@ 2025-11-05 13:35 ` Chen Miao
2025-11-05 13:40 ` Miguel Ojeda
0 siblings, 1 reply; 12+ messages in thread
From: Chen Miao @ 2025-11-05 13:35 UTC (permalink / raw)
To: Miguel Ojeda
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
On 11/4/2025 10:14 PM, Miguel Ojeda wrote:
> Hi chenmiao,
>
> A few procedural bits below for future versions/patches...
>
> On Tue, Nov 4, 2025 at 3:04 AM chenmiao <chenmiao@openatom.club> wrote:
>> If finding the offset from the primitive type is necessary for this
>> implementation, I will abandon this change.
> In general, this should not be part of the commit message, but go
> below the `---` line.
>
> And if the commit is not ready, typically it should be marked as RFC.
Understood. I will pay attention to this in my future possible emails.
>> Additionally, support for the `static_branch_enable/disable` APIs has been
>> introduced.
> Please split different changes in different patches if possible.
As the same I said above.
>> Signed-off-by: chenmiao <chenmiao@openatom.club>
> In another email you used Chen Miao in the From -- in general, please
> note that the SoB is supposed to have the "full name" ("known
> identity") as you would normally write it. I mention this since
> sometimes people use the local part of the email address, i.e. `...@`,
> but that is usually not the normal spelling of a full name. Of course,
> please ignore this if it is correct already.
>
> Thanks!
>
> Cheers,
> Miguel
I can understand, but I'd like to make a brief explanation here. "Chen Miao"
is my name, so the prefix of all my email addresses is "chenmiao." Therefore,
"Chen Miao" and "chenmiao" are equivalent.
Regards,
Chen Miao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:28 ` Chen Miao
@ 2025-11-05 13:38 ` Alice Ryhl
2025-11-05 13:55 ` Chen Miao
0 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2025-11-05 13:38 UTC (permalink / raw)
To: Chen Miao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list,
hust-os-kernel-patches
On Wed, Nov 05, 2025 at 09:28:39PM +0800, Chen Miao wrote:
> On 11/4/2025 10:07 PM, Alice Ryhl wrote:
> > On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
> >> The initial implementation of arch_static_branch was achieved by accessing
> >> the offset from the original type. However, this approach extended the
> >> path and introduced redundant calculations when dealing with types like
> >> `static_key_true/false`, as shown below:
> >>
> >> ```
> >> static_brach_unlikely(tp, tracepoint, key)
> >> => tracepoint->key->key
> >> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
> >> => off: tracepoint->key - tracepoint
> >> ```
> >>
> >> In practice, the implementation of `arch_static_branch` overlooked many
> >> detailed descriptions. To improve clarity, additional comments have been
> >> added to the original logic. The approach has been modified to directly
> >> locate the corresponding `static_key` instead of using offsets, thereby
> >> reducing computational overhead.
> >>
> >> If finding the offset from the primitive type is necessary for this
> >> implementation, I will abandon this change.
> >>
> >> Additionally, support for the `static_branch_enable/disable` APIs has been
> >> introduced.
> >>
> >> Signed-off-by: chenmiao <chenmiao@openatom.club>
> >> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
> >> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
> >> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
> >> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
> >> let _key: *const $crate::bindings::static_key = _key.cast();
> >>
> >> #[cfg(not(CONFIG_JUMP_LABEL))]
> >> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
> >> }
> >>
> >> #[cfg(CONFIG_JUMP_LABEL)]
> >> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
> >> + {
> >> + $crate::jump_label::arch_static_branch! { _key, $branch }
> >> + }
> > So ... this is changing from $key to _key. That's replacing the global
> > variable with a local variable holding a pointer to the global variable.
> > However, the arch_static_branch! macro uses the `sym` operand which
> > requires you to pass it the global directly.
> >
> > Did you try this code? I don't believe it will work.
> >
> > Alice
>
> I'm very sorry for making a fatal mistake. My misunderstanding of sym led to
> this issue, so I shouldn't make any changes to that part. However, regarding
> the other modifications, I believe it is necessary to support the direct
> passing of structures similar to `static_key_false`, just as in C language.
It sounds like you are adding a new use-case for this macro. Can you
provide more information for this new feature? It is currently unclear
to me exactly how this will be used.
Alice
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:35 ` Chen Miao
@ 2025-11-05 13:40 ` Miguel Ojeda
2025-11-05 13:46 ` Chen Miao
0 siblings, 1 reply; 12+ messages in thread
From: Miguel Ojeda @ 2025-11-05 13:40 UTC (permalink / raw)
To: Chen Miao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
On Wed, Nov 5, 2025 at 2:35 PM Chen Miao <chenmiao@openatom.club> wrote:
>
> I can understand, but I'd like to make a brief explanation here. "Chen Miao"
> is my name, so the prefix of all my email addresses is "chenmiao." Therefore,
> "Chen Miao" and "chenmiao" are equivalent.
That is fine, but what I was trying to say is that, in that case the
Signed-off-by should be "Chen Miao".
Please see https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
-- thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:40 ` Miguel Ojeda
@ 2025-11-05 13:46 ` Chen Miao
2025-11-05 13:49 ` Miguel Ojeda
0 siblings, 1 reply; 12+ messages in thread
From: Chen Miao @ 2025-11-05 13:46 UTC (permalink / raw)
To: Miguel Ojeda
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
On 11/5/2025 9:40 PM, Miguel Ojeda wrote:
> On Wed, Nov 5, 2025 at 2:35 PM Chen Miao <chenmiao@openatom.club> wrote:
>> I can understand, but I'd like to make a brief explanation here. "Chen Miao"
>> is my name, so the prefix of all my email addresses is "chenmiao." Therefore,
>> "Chen Miao" and "chenmiao" are equivalent.
> That is fine, but what I was trying to say is that, in that case the
> Signed-off-by should be "Chen Miao".
>
> Please see https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> -- thanks!
>
> Cheers,
> Miguel
Well, I will follow this rule and change the Signed-off-by to "Chen Miao"
before the next push.
Regards,
Chen Miao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:46 ` Chen Miao
@ 2025-11-05 13:49 ` Miguel Ojeda
0 siblings, 0 replies; 12+ messages in thread
From: Miguel Ojeda @ 2025-11-05 13:49 UTC (permalink / raw)
To: Chen Miao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Steven Rostedt (Google), open list:RUST,
open list
On Wed, Nov 5, 2025 at 2:46 PM Chen Miao <chenmiao@openatom.club> wrote:
>
> Well, I will follow this rule and change the Signed-off-by to "Chen Miao"
> before the next push.
Yeah, the rules are a bit particular w.r.t. other projects, so it
takes a bit of time to get used to them -- no worries and thanks! :)
Cheers,
Miguel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:38 ` Alice Ryhl
@ 2025-11-05 13:55 ` Chen Miao
2025-11-05 14:13 ` Alice Ryhl
0 siblings, 1 reply; 12+ messages in thread
From: Chen Miao @ 2025-11-05 13:55 UTC (permalink / raw)
To: Alice Ryhl
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list,
hust-os-kernel-patches
On 11/5/2025 9:38 PM, Alice Ryhl wrote:
> On Wed, Nov 05, 2025 at 09:28:39PM +0800, Chen Miao wrote:
>> On 11/4/2025 10:07 PM, Alice Ryhl wrote:
>>> On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
>>>> The initial implementation of arch_static_branch was achieved by accessing
>>>> the offset from the original type. However, this approach extended the
>>>> path and introduced redundant calculations when dealing with types like
>>>> `static_key_true/false`, as shown below:
>>>>
>>>> ```
>>>> static_brach_unlikely(tp, tracepoint, key)
>>>> => tracepoint->key->key
>>>> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
>>>> => off: tracepoint->key - tracepoint
>>>> ```
>>>>
>>>> In practice, the implementation of `arch_static_branch` overlooked many
>>>> detailed descriptions. To improve clarity, additional comments have been
>>>> added to the original logic. The approach has been modified to directly
>>>> locate the corresponding `static_key` instead of using offsets, thereby
>>>> reducing computational overhead.
>>>>
>>>> If finding the offset from the primitive type is necessary for this
>>>> implementation, I will abandon this change.
>>>>
>>>> Additionally, support for the `static_branch_enable/disable` APIs has been
>>>> introduced.
>>>>
>>>> Signed-off-by: chenmiao <chenmiao@openatom.club>
>>>> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
>>>> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
>>>> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
>>>> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
>>>> let _key: *const $crate::bindings::static_key = _key.cast();
>>>>
>>>> #[cfg(not(CONFIG_JUMP_LABEL))]
>>>> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
>>>> }
>>>>
>>>> #[cfg(CONFIG_JUMP_LABEL)]
>>>> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
>>>> + {
>>>> + $crate::jump_label::arch_static_branch! { _key, $branch }
>>>> + }
>>> So ... this is changing from $key to _key. That's replacing the global
>>> variable with a local variable holding a pointer to the global variable.
>>> However, the arch_static_branch! macro uses the `sym` operand which
>>> requires you to pass it the global directly.
>>>
>>> Did you try this code? I don't believe it will work.
>>>
>>> Alice
>> I'm very sorry for making a fatal mistake. My misunderstanding of sym led to
>> this issue, so I shouldn't make any changes to that part. However, regarding
>> the other modifications, I believe it is necessary to support the direct
>> passing of structures similar to `static_key_false`, just as in C language.
> It sounds like you are adding a new use-case for this macro. Can you
> provide more information for this new feature? It is currently unclear
> to me exactly how this will be used.
>
> Alice
If there's a binding-required driver implementation in the future where a key
function uses if (static_branch_unlikely(&zoned_enabled))— defined by
DEFINE_STATIC_KEY_FALSE(zoned_enabled);— then in Rust we can directly
implement it using if static_branch_unlikely!(zoned_enabled), without having
to call it via if static_branch_unlikely!(zoned_enabled,
bindings::static_key_false, key).
static_branch_unlikely!(zoned_enabled) instead of
static_branch_unlikely!(zoned_enabled, bindings::static_key_false, key).
Chen Miao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 13:55 ` Chen Miao
@ 2025-11-05 14:13 ` Alice Ryhl
2025-11-05 14:16 ` Chen Miao
0 siblings, 1 reply; 12+ messages in thread
From: Alice Ryhl @ 2025-11-05 14:13 UTC (permalink / raw)
To: Chen Miao
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list,
hust-os-kernel-patches
On Wed, Nov 05, 2025 at 09:55:27PM +0800, Chen Miao wrote:
> On 11/5/2025 9:38 PM, Alice Ryhl wrote:
> > On Wed, Nov 05, 2025 at 09:28:39PM +0800, Chen Miao wrote:
> >> On 11/4/2025 10:07 PM, Alice Ryhl wrote:
> >>> On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
> >>>> The initial implementation of arch_static_branch was achieved by accessing
> >>>> the offset from the original type. However, this approach extended the
> >>>> path and introduced redundant calculations when dealing with types like
> >>>> `static_key_true/false`, as shown below:
> >>>>
> >>>> ```
> >>>> static_brach_unlikely(tp, tracepoint, key)
> >>>> => tracepoint->key->key
> >>>> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
> >>>> => off: tracepoint->key - tracepoint
> >>>> ```
> >>>>
> >>>> In practice, the implementation of `arch_static_branch` overlooked many
> >>>> detailed descriptions. To improve clarity, additional comments have been
> >>>> added to the original logic. The approach has been modified to directly
> >>>> locate the corresponding `static_key` instead of using offsets, thereby
> >>>> reducing computational overhead.
> >>>>
> >>>> If finding the offset from the primitive type is necessary for this
> >>>> implementation, I will abandon this change.
> >>>>
> >>>> Additionally, support for the `static_branch_enable/disable` APIs has been
> >>>> introduced.
> >>>>
> >>>> Signed-off-by: chenmiao <chenmiao@openatom.club>
> >>>> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
> >>>> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
> >>>> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
> >>>> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
> >>>> let _key: *const $crate::bindings::static_key = _key.cast();
> >>>>
> >>>> #[cfg(not(CONFIG_JUMP_LABEL))]
> >>>> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
> >>>> }
> >>>>
> >>>> #[cfg(CONFIG_JUMP_LABEL)]
> >>>> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
> >>>> + {
> >>>> + $crate::jump_label::arch_static_branch! { _key, $branch }
> >>>> + }
> >>> So ... this is changing from $key to _key. That's replacing the global
> >>> variable with a local variable holding a pointer to the global variable.
> >>> However, the arch_static_branch! macro uses the `sym` operand which
> >>> requires you to pass it the global directly.
> >>>
> >>> Did you try this code? I don't believe it will work.
> >>>
> >>> Alice
> >> I'm very sorry for making a fatal mistake. My misunderstanding of sym led to
> >> this issue, so I shouldn't make any changes to that part. However, regarding
> >> the other modifications, I believe it is necessary to support the direct
> >> passing of structures similar to `static_key_false`, just as in C language.
> > It sounds like you are adding a new use-case for this macro. Can you
> > provide more information for this new feature? It is currently unclear
> > to me exactly how this will be used.
> >
> > Alice
>
> If there's a binding-required driver implementation in the future where a key
> function uses if (static_branch_unlikely(&zoned_enabled))— defined by
> DEFINE_STATIC_KEY_FALSE(zoned_enabled);— then in Rust we can directly
> implement it using if static_branch_unlikely!(zoned_enabled), without having
> to call it via if static_branch_unlikely!(zoned_enabled,
> bindings::static_key_false, key).
>
> static_branch_unlikely!(zoned_enabled) instead of
> static_branch_unlikely!(zoned_enabled, bindings::static_key_false, key).
In general, you would never use "static_key_false" as the second
argument to static_branch_unlikely!. The second argument is the name of
the struct *containing* a field of type static_key_false.
I guess your point is that there's no way to use the macro right now if
the global is a bare static_key_false that is not wrapped in a struct?
Alice
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rust: kernel: Support more jump_label api
2025-11-05 14:13 ` Alice Ryhl
@ 2025-11-05 14:16 ` Chen Miao
0 siblings, 0 replies; 12+ messages in thread
From: Chen Miao @ 2025-11-05 14:16 UTC (permalink / raw)
To: Alice Ryhl
Cc: ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Steven Rostedt (Google), open list:RUST, open list,
hust-os-kernel-patches
On 11/5/2025 10:13 PM, Alice Ryhl wrote:
> On Wed, Nov 05, 2025 at 09:55:27PM +0800, Chen Miao wrote:
>> On 11/5/2025 9:38 PM, Alice Ryhl wrote:
>>> On Wed, Nov 05, 2025 at 09:28:39PM +0800, Chen Miao wrote:
>>>> On 11/4/2025 10:07 PM, Alice Ryhl wrote:
>>>>> On Tue, Nov 04, 2025 at 02:04:17AM +0000, chenmiao wrote:
>>>>>> The initial implementation of arch_static_branch was achieved by accessing
>>>>>> the offset from the original type. However, this approach extended the
>>>>>> path and introduced redundant calculations when dealing with types like
>>>>>> `static_key_true/false`, as shown below:
>>>>>>
>>>>>> ```
>>>>>> static_brach_unlikely(tp, tracepoint, key)
>>>>>> => tracepoint->key->key
>>>>>> => &tracepoint->key(static_key_false) == &tracepoint->key.key(static_key)
>>>>>> => off: tracepoint->key - tracepoint
>>>>>> ```
>>>>>>
>>>>>> In practice, the implementation of `arch_static_branch` overlooked many
>>>>>> detailed descriptions. To improve clarity, additional comments have been
>>>>>> added to the original logic. The approach has been modified to directly
>>>>>> locate the corresponding `static_key` instead of using offsets, thereby
>>>>>> reducing computational overhead.
>>>>>>
>>>>>> If finding the offset from the primitive type is necessary for this
>>>>>> implementation, I will abandon this change.
>>>>>>
>>>>>> Additionally, support for the `static_branch_enable/disable` APIs has been
>>>>>> introduced.
>>>>>>
>>>>>> Signed-off-by: chenmiao <chenmiao@openatom.club>
>>>>>> + ($basety:ty, $branch:expr, $key:path, $keytyp:ty, $field:ident) => {{
>>>>>> let _key: *const $keytyp = ::core::ptr::addr_of!($key);
>>>>>> - let _key: *const $crate::bindings::static_key_false = ::core::ptr::addr_of!((*_key).$field);
>>>>>> + let _key: *const $basety = ::core::ptr::addr_of!((*_key).$field);
>>>>>> let _key: *const $crate::bindings::static_key = _key.cast();
>>>>>>
>>>>>> #[cfg(not(CONFIG_JUMP_LABEL))]
>>>>>> @@ -30,7 +71,88 @@ macro_rules! static_branch_unlikely {
>>>>>> }
>>>>>>
>>>>>> #[cfg(CONFIG_JUMP_LABEL)]
>>>>>> - $crate::jump_label::arch_static_branch! { $key, $keytyp, $field, false }
>>>>>> + {
>>>>>> + $crate::jump_label::arch_static_branch! { _key, $branch }
>>>>>> + }
>>>>> So ... this is changing from $key to _key. That's replacing the global
>>>>> variable with a local variable holding a pointer to the global variable.
>>>>> However, the arch_static_branch! macro uses the `sym` operand which
>>>>> requires you to pass it the global directly.
>>>>>
>>>>> Did you try this code? I don't believe it will work.
>>>>>
>>>>> Alice
>>>> I'm very sorry for making a fatal mistake. My misunderstanding of sym led to
>>>> this issue, so I shouldn't make any changes to that part. However, regarding
>>>> the other modifications, I believe it is necessary to support the direct
>>>> passing of structures similar to `static_key_false`, just as in C language.
>>> It sounds like you are adding a new use-case for this macro. Can you
>>> provide more information for this new feature? It is currently unclear
>>> to me exactly how this will be used.
>>>
>>> Alice
>> If there's a binding-required driver implementation in the future where a key
>> function uses if (static_branch_unlikely(&zoned_enabled))— defined by
>> DEFINE_STATIC_KEY_FALSE(zoned_enabled);— then in Rust we can directly
>> implement it using if static_branch_unlikely!(zoned_enabled), without having
>> to call it via if static_branch_unlikely!(zoned_enabled,
>> bindings::static_key_false, key).
>>
>> static_branch_unlikely!(zoned_enabled) instead of
>> static_branch_unlikely!(zoned_enabled, bindings::static_key_false, key).
> In general, you would never use "static_key_false" as the second
> argument to static_branch_unlikely!. The second argument is the name of
> the struct *containing* a field of type static_key_false.
>
> I guess your point is that there's no way to use the macro right now if
> the global is a bare static_key_false that is not wrapped in a struct?
>
> Alice
Yes, you're right. But in fact, currently there are no other uses of
static_branch_unlikely in Rust for Linux (except for tracepoint), so this is
also a bit awkward.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-05 15:03 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 2:04 [PATCH] rust: kernel: Support more jump_label api chenmiao
2025-11-04 14:07 ` Alice Ryhl
2025-11-05 13:28 ` Chen Miao
2025-11-05 13:38 ` Alice Ryhl
2025-11-05 13:55 ` Chen Miao
2025-11-05 14:13 ` Alice Ryhl
2025-11-05 14:16 ` Chen Miao
2025-11-04 14:14 ` Miguel Ojeda
2025-11-05 13:35 ` Chen Miao
2025-11-05 13:40 ` Miguel Ojeda
2025-11-05 13:46 ` Chen Miao
2025-11-05 13:49 ` Miguel Ojeda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).