| Message ID | 20250303-inline-cred-v1-1-b2527beace76@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | cred: rust: mark Credential methods inline | expand |
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> >
"Alice Ryhl" <aliceryhl@google.com> writes: > I'm seeing Binder generating calls to methods on Credential such as > 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> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> Best regards, Andreas Hindborg
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 > 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> > --- Thanks! Reviewed-by: Christian Brauner <brauner@kernel.org>
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`.
I'm seeing Binder generating calls to methods on Credential such as 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(+) --- base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 change-id: 20250303-inline-cred-1d1050785e5c Best regards,