Re: [PATCH] rust: task: mark Task methods inline

[Alice Ryhl <aliceryhl@google.com>]

On Mon, Mar 10, 2025 at 04:09:09PM +0000, Panagiotis Foliadis wrote:
> 
> 
> On 10/3/25 12:23, Alice Ryhl wrote:
> > On Mon, Mar 10, 2025 at 10:40 AM Panagiotis Foliadis
> > <pfoliadis@posteo.net> wrote:
> > > When you build the kernel using the llvm-18.1.3-rust-1.85.0-x86_64
> > > toolchain provided by kernel.org, the following symbols are generated:
> > > 
> > > $ nm vmlinux | grep ' _R'.*Task | rustfilt
> > > ffffffff817b2d30 T <kernel::task::Task>::get_pid_ns
> > > ffffffff817b2d50 T <kernel::task::Task>::tgid_nr_ns
> > > ffffffff817b2c90 T <kernel::task::Task>::current_pid_ns
> > > ffffffff817b2d00 T <kernel::task::Task>::signal_pending
> > > ffffffff817b2cc0 T <kernel::task::Task>::uid
> > > ffffffff817b2ce0 T <kernel::task::Task>::euid
> > > ffffffff817b2c70 T <kernel::task::Task>::current
> > > ffffffff817b2d70 T <kernel::task::Task>::wake_up
> > > ffffffff817b2db0 T <kernel::task::Task as kernel::types::AlwaysRefCounted>::dec_ref
> > > ffffffff817b2d90 T <kernel::task::Task as kernel::types::AlwaysRefCounted>::inc_ref
> > > 
> > > Most of these Rust symbols are trivial wrappers around the C functions
> > > signal_pending, uid, euid, wake_up, dec_ref and inc_ref.It doesn't
> > > make sense to go through a trivial wrapper for these functions, so
> > > mark them inline.
> > There's no C function called dec_ref or inc_ref? Please use the C
> > function names instead of the Rust ones.
> > 
> > > After applying this patch, the above command will produce this output:
> > > 
> > > ffff8000805aa004 T <kernel::task::Task>::get_pid_ns
> > > ffff8000805aa01c T <kernel::task::Task>::tgid_nr_ns
> > > ffff8000805a9fe8 T <kernel::task::Task>::current_pid_ns
> > > ffff8000805a9fd0 T <kernel::task::Task>::current
> > I think it'd be nice with an explanation of why you did not mark these
> > #[inline].
> 
> Since the issue focuses on the functions that are trivial wrappers around
> c and `do nothing that call a C function` I thought i would leave this out
> since there is some other functionality (albeit sometimes minimal) other
> that being just a c-wrapper.

As far as I can tell, all four functions will compile down to a simple
call to a C function.

get_pid_ns, tgid_nr_ns: These functions have a pointer where they say
"if the pointer is null, replace it with null, otherwise just use the
pointer". The optimizer can optimize that to "just use the pointer", and
in fact I checked that this optimization happens when we wrote
tgid_nr_ns.

current_pid_ns: If you add #[inline] to PidNamespace::from_ptr, then
this is just a call to task_active_pid_ns.

current: This is just a call to Task::current_raw which in turn is just
a call to `get_current`.

Alice