Re: [PATCH v2] rust: devres: optimize type name allocation and fix truncation

[Aary Kinge]

On Wed, May 27, 2026 at 4:13 PM Aary Kinge <kingeaary@xxxxxxxxx> wrote:
>
>
> Hi Miguel,
>
> On Tue, May 26, 2026 at 6:07 PM Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx> wrote:
>>
>> On Tue, May 26, 2026 at 1:58 PM Aary Milind Kinge <kingeaary@xxxxxxxxx> wrote:
>> >
>> > The unconditional 128-byte const array allocation for every unique
>> > `Devres<T>` caused unnecessary .rodata bloat in production builds,
>>
>> Wasn't the string deduplicated?
>>
>
> You are right. The original string was identical for all T and would be deduplicated by the linker.
> I will reword the commit message to focus on the actual improvement: replacing the generic placeholder with the real type name
>
>>
>> > terminator), the copy routine now appends "..." at the end — copying
>>
>> Did an LLM assist this patch? If so, please add a tag:
>
>
> Yes. I will add the appropriate tag in v3.
>
>>
>>
>>   https://docs.kernel.org/process/generated-content.html
>>   https://docs.kernel.org/process/coding-assistants.html
>>
>> Moreover, this should be sent to the right maintainers and reviewers,
>> e.g. at least to "DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS". Cc'ing
>> them here, but also please Cc all the "RUST" entry.
>
>
> Yes, I will CC the right maintainers.
>
>>
>>
>> > +        let mut buf = [0u8; 128];
>>
>> Why is there a hardcoded literal? Please use constants where possible,
>> deriving the rest of the literals from that.
>
>
> I will replace the hardcoded values with derived constants in v3.
>
>>
>>
>> > +        let mut len = 0;
>> > +        while len < 128 && static_buf[len] != 0 {
>> > +            len += 1;
>> > +        }
>> > +
>> > +        // SAFETY: `static_buf` is promoted to static memory, and we verified the null byte.
>>
>> This should explain why this is all OK, e.g. why it doesn't go out of
>> bounds (a local-only reading of the loop above would appear to make it
>> so).
>
>
> Understood. I will improve the SAFETY comment and see if the unsafe can
> be avoided entirely.
>
>>
>>
>> Or avoid `unsafe` altogether if possible.
>>
>> > +    #[cfg(not(CONFIG_DEBUG_DEVRES))]
>> > +    const TYPE_NAME_CSTR: &'static crate::str::CStr = crate::c_str!("");
>>
>> Is the name not used at all under `!CONFIG_DEBUG_DEVRES`?
>>
>> Either way, could we move the other `const`s inside this one, and
>> remove the `cfg` here? i.e. reducing the number of conditional items
>> and avoiding to repeat the "signature" of this `const`.
>
>
> Yes, I will restructure this in v3.
>
> I dug into this and it turns out it is used. The naming is misleading — devres_set_node_dbginfo() stores node->name unconditionally, and the trace_devres_log() tracepoint in drivers/base/trace.h records it unconditionally too. The only thing gated behind CONFIG_DEBUG_DEVRES is the devres_dbg() printk, not the tracepoint.
>
> So if I gate the real type name behind CONFIG_DEBUG_DEVRES, the devres tracepoints show an empty string in production traces for every Rust Devres<T> — which is worse than the generic "Devres<T>" placeholder we had before. The correct approach is to just always provide the real type name. The overhead is negligible (a single const array per monomorphization of Devres<T>).
>
> That means I should drop the #[cfg] gating entirely on TYPE_NAME_CSTR and remove the #[cfg(not(CONFIG_DEBUG_DEVRES))] empty-string branch. Thoughts?
>
>>
>>
>> >  // Expected to become stable.
>> >  #![feature(arbitrary_self_types)]
>> >  #![feature(derive_coerce_pointee)]
>> > +#![feature(const_type_name)]
>>
>> New features need to be justified in the commit message, with a link
>> to the tracking issue:
>>
>>   https://github.com/rust-lang/rust/issues/63084
>>
>> In particular, please justify why you think it will become stable.
>> Usually, we need to double-check with upstream that is the case.
>
>
> I will update the commit message to justify the feature usage and add
> the tracking issue link, without making claims about stabilization.
>
>>
>>
>> Thanks!
>>
>> Cheers,
>> Miguel
>
>
> Aary

On Tue, May 26, 2026 at 9:50 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, May 26, 2026 at 05:28:25PM +0530, Aary Milind Kinge wrote:
> > The unconditional 128-byte const array allocation for every unique
> > `Devres<T>` caused unnecessary .rodata bloat in production builds,
> > and type names exceeding 127 bytes were silently truncated without
> > any indication in debug logs.
> >
> > Gate the `TYPE_NAME`, `TYPE_NAME_BUF`, and `TYPE_NAME_CSTR` constants
> > behind `#[cfg(CONFIG_DEBUG_DEVRES)]` so the 128-byte buffer is only
> > allocated when device resource debugging is enabled. For production
> > builds, use `crate::c_str!("")` as a zero-cost empty C-string fallback.
> >
> > When the type name is too long to fit in the 127-byte buffer (plus null
> > terminator), the copy routine now appends "..." at the end — copying
> > only the first 124 bytes — to clearly indicate truncation rather than
> > silently dropping trailing characters.
> >
> > Signed-off-by: Aary Milind Kinge <kingeaary@xxxxxxxxx>
> > ---
> >  rust/kernel/devres.rs | 55 ++++++++++++++++++++++++++++++++++++++++---
> >  rust/kernel/lib.rs    |  1 +
> >  2 files changed, 53 insertions(+), 3 deletions(-)
> >
> > diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> > index 9e5f93aed20c..79cfe7d9a415 100644
> > --- a/rust/kernel/devres.rs
> > +++ b/rust/kernel/devres.rs
> > @@ -185,6 +185,57 @@ pub(super) unsafe fn devres_node_remove(
> >  }
> >
> >  impl<T: Send> Devres<T> {
> > +    #[cfg(CONFIG_DEBUG_DEVRES)]
> > +    const TYPE_NAME: &'static str = core::any::type_name::<T>();
> > +
> > +    #[cfg(CONFIG_DEBUG_DEVRES)]
> > +    const TYPE_NAME_BUF: [u8; 128] = {
> > +        let bytes = Self::TYPE_NAME.as_bytes();
> > +        let mut buf = [0u8; 128];
> > +        let mut i = 0;
> > +
> > +        if bytes.len() > 127 {
> > +            // Copy exactly 124 bytes, then append '...' to clearly indicate truncation
> > +            while i < 124 {
> > +                buf[i] = bytes[i];
> > +                i += 1;
> > +            }
> > +            buf[124] = b'.';
> > +            buf[125] = b'.';
> > +            buf[126] = b'.';
> > +            buf[127] = 0; // Null terminator
> > +        } else {
> > +            // Copy normally
> > +            while i < bytes.len() {
> > +                buf[i] = bytes[i];
> > +                i += 1;
> > +            }
> > +            buf[i] = 0; // Null terminator
> > +        }
> > +        buf
> > +    };
> > +
> > +    #[cfg(CONFIG_DEBUG_DEVRES)]
> > +    const TYPE_NAME_CSTR: &'static crate::str::CStr = {
> > +        let static_buf: &'static [u8; 128] = &Self::TYPE_NAME_BUF;
> > +
> > +        let mut len = 0;
> > +        while len < 128 && static_buf[len] != 0 {
> > +            len += 1;
> > +        }
> > +
> > +        // SAFETY: `static_buf` is promoted to static memory, and we verified the null byte.
> > +        unsafe {
> > +            crate::str::CStr::from_bytes_with_nul_unchecked(core::slice::from_raw_parts(
> > +                static_buf.as_ptr(),
> > +                len + 1,
> > +            ))
> > +        }
> > +    };
> > +
> > +    #[cfg(not(CONFIG_DEBUG_DEVRES))]
> > +    const TYPE_NAME_CSTR: &'static crate::str::CStr = crate::c_str!("");
> > +
> >      /// Creates a new [`Devres`] instance of the given `data`.
> >      ///
> >      /// The `data` encapsulated within the returned `Devres` instance' `data` will be
> > @@ -209,9 +260,7 @@ pub fn new<E>(dev: &Device<Bound>, data: impl PinInit<T, E>) -> Result<Self>
> >                      unsafe {
> >                          base::devres_set_node_dbginfo(
> >                              node,
> > -                            // TODO: Use `core::any::type_name::<T>()` once it is a `const fn`,
> > -                            // such that we can convert the `&str` to a `&CStr` at compile-time.
> > -                            c"Devres<T>".as_char_ptr(),
> > +                            Self::TYPE_NAME_CSTR.as_char_ptr(),
> >                              core::mem::size_of::<Revocable<T>>(),
> >                          )
> >                      };
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index b72b2fbe046d..48df181788f6 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -22,6 +22,7 @@
> >  // Expected to become stable.
> >  #![feature(arbitrary_self_types)]
> >  #![feature(derive_coerce_pointee)]
> > +#![feature(const_type_name)]
> >  //
> >  // To be determined.
> >  #![feature(used_with_arg)]
> > --
> > 2.51.0
> >
> >
>
> Hi,
>
> This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
> a patch that has triggered this response.  He used to manually respond
> to these common problems, but in order to save his sanity (he kept
> writing the same thing over and over, yet to different people), I was
> created.  Hopefully you will not take offence and will fix the problem
> in your patch and resubmit it so that it can be accepted into the Linux
> kernel tree.
>
> You are receiving this message because of the following common error(s)
> as indicated below:
>
> - This looks like a new version of a previously submitted patch, but you
>   did not list below the --- line any changes from the previous version.
>   Please read the section entitled "The canonical patch format" in the
>   kernel file, Documentation/process/submitting-patches.rst for what
>   needs to be done here to properly describe this.
>
> If you wish to discuss this problem further, or you have questions about
> how to resolve this issue, please feel free to respond to this email and
> Greg will reply once he has dug out from the pending patches received
> from other developers.
>
> thanks,
>
> greg k-h's patch email bot