Re: [PATCH] cred: rust: mark Credential methods inline

[Boqun Feng <boqun.feng@gmail.com>]

Hi Alice,

On Mon, Mar 03, 2025 at 03:28:50PM +0000, Alice Ryhl wrote:
> I'm seeing Binder generating calls to methods on Credential such as

I would suggest using impersonal facts to explain why the changes are
needed. For example, you can show a compile result (with the version of
rust provided), which has the function callsites, in this way, it'll be
easy for people to verify this on their own. Thanks!

With this changed, feel free to add

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> get_secid, inc_ref, and dec_ref without inlining. Since these methods
> are really simple wrappers around C functions, mark the methods to
> inline to avoid generating these useless small functions.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/cred.rs | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
> index 81d67789b16f..2599f01e8b28 100644
> --- a/rust/kernel/cred.rs
> +++ b/rust/kernel/cred.rs
> @@ -47,6 +47,7 @@ impl Credential {
>      ///
>      /// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
>      /// returned [`Credential`] reference.
> +    #[inline]
>      pub unsafe fn from_ptr<'a>(ptr: *const bindings::cred) -> &'a Credential {
>          // SAFETY: The safety requirements guarantee the validity of the dereference, while the
>          // `Credential` type being transparent makes the cast ok.
> @@ -54,6 +55,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::cred) -> &'a Credential {
>      }
>  
>      /// Get the id for this security context.
> +    #[inline]
>      pub fn get_secid(&self) -> u32 {
>          let mut secid = 0;
>          // SAFETY: The invariants of this type ensures that the pointer is valid.
> @@ -62,6 +64,7 @@ pub fn get_secid(&self) -> u32 {
>      }
>  
>      /// Returns the effective UID of the given credential.
> +    #[inline]
>      pub fn euid(&self) -> Kuid {
>          // SAFETY: By the type invariant, we know that `self.0` is valid. Furthermore, the `euid`
>          // field of a credential is never changed after initialization, so there is no potential
> @@ -72,11 +75,13 @@ pub fn euid(&self) -> Kuid {
>  
>  // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
>  unsafe impl AlwaysRefCounted for Credential {
> +    #[inline]
>      fn inc_ref(&self) {
>          // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>          unsafe { bindings::get_cred(self.0.get()) };
>      }
>  
> +    #[inline]
>      unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
>          // SAFETY: The safety requirements guarantee that the refcount is nonzero. The cast is okay
>          // because `Credential` has the same representation as `struct cred`.
> 
> ---
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> change-id: 20250303-inline-cred-1d1050785e5c
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
>