* Re: [PATCH] cred: rust: mark Credential methods inline
2025-03-03 15:28 ` [PATCH] cred: rust: mark Credential methods inline Alice Ryhl
@ 2025-03-03 15:51 ` Boqun Feng
2025-03-03 18:37 ` Andreas Hindborg
2025-03-04 9:31 ` Christian Brauner
2 siblings, 0 replies; 4+ messages in thread
From: Boqun Feng @ 2025-03-03 15:51 UTC (permalink / raw)
To: Alice Ryhl
Cc: Paul Moore, James Morris, Serge E. Hallyn, Christian Brauner,
Jens Axboe, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, rust-for-linux,
linux-security-module, linux-kernel
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>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] cred: rust: mark Credential methods inline
2025-03-03 15:28 ` [PATCH] cred: rust: mark Credential methods inline Alice Ryhl
2025-03-03 15:51 ` Boqun Feng
@ 2025-03-03 18:37 ` Andreas Hindborg
2025-03-04 9:31 ` Christian Brauner
2 siblings, 0 replies; 4+ messages in thread
From: Andreas Hindborg @ 2025-03-03 18:37 UTC (permalink / raw)
To: Alice Ryhl
Cc: Paul Moore, James Morris, Serge E. Hallyn, Christian Brauner,
Jens Axboe, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross, rust-for-linux,
linux-security-module, linux-kernel
"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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cred: rust: mark Credential methods inline
2025-03-03 15:28 ` [PATCH] cred: rust: mark Credential methods inline Alice Ryhl
2025-03-03 15:51 ` Boqun Feng
2025-03-03 18:37 ` Andreas Hindborg
@ 2025-03-04 9:31 ` Christian Brauner
2 siblings, 0 replies; 4+ messages in thread
From: Christian Brauner @ 2025-03-04 9:31 UTC (permalink / raw)
To: Alice Ryhl
Cc: Paul Moore, James Morris, Serge E. Hallyn, Jens Axboe,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, rust-for-linux,
linux-security-module, linux-kernel
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>
^ permalink raw reply [flat|nested] 4+ messages in thread