rust: sync: optimize rust symbol generation for CondVar

From: Kunwu Chan <[email protected]>

When build the kernel using the llvm-18.1.3-rust-1.85.0-x86_64 with ARCH=arm64, the following symbols are generated:

$nm vmlinux | grep ' _R'.*CondVar | rustfilt
... T <kernel::sync::condvar::CondVar>::notify_all
... T <kernel::sync::condvar::CondVar>::notify_one
... T <kernel::sync::condvar::CondVar>::notify_sync
... T <kernel::sync::condvar::CondVar>::new::{closure#0}::{closure#0}::panic_cold_explicit ... T <kernel::sync::condvar::CondVar>::new::{closure#0}::{closure#0}::panic_cold_explicit ... T <kernel::sync::poll::PollCondVar>::new::{closure#0}::{closure#0}::panic_cold_explicit ... T <kernel::sync::poll::PollCondVar as core::ops::drop::Drop>::drop

These notify_* symbols are trivial wrappers around the C functions __wake_up and __wake_up_sync. It doesn't make sense to go through a trivial wrapper for these functions, so mark them inline.

---
Changes in v2:
- Remove '#[inline]' for notify()
- Reword commit msg
- v1 link: https://lore.kernel.org/rust-for-linux/[email protected]

---
 rust/kernel/sync/condvar.rs | 3 +++
 1 file changed, 3 insertions(+)

index caebf03f553b..c6ec64295c9f 100644
@@ -216,6 +216,7 @@ fn notify(&self, count: c_int) {
     /// This method behaves like `notify_one`, except that it hints to the scheduler that the
     /// current thread is about to go to sleep, so it should schedule the target thread on the same
     /// CPU.
+    #[inline]
     pub fn notify_sync(&self) {
         // SAFETY: `wait_queue_head` points to valid memory.
         unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), TASK_NORMAL) };

@@ -225,6 +226,7 @@ pub fn notify_sync(&self) {
     ///
     /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
     /// completely (as opposed to automatically waking up the next waiter).
+    #[inline]
     pub fn notify_one(&self) {
         self.notify(1);
     }

@@ -233,6 +235,7 @@ pub fn notify_one(&self) {
     ///
     /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
     /// completely (as opposed to automatically waking up the next waiter).
+    #[inline]
     pub fn notify_all(&self) {
         self.notify(0);
     }

2.43.0

Nit: I typically use Link after Suggested-by, to mimic what the docs require about Reported-by with Link. (No need to resend a new version just for this :)

It is good that the commit matches the log now, though I wonder if in the future we may want the #[inline] for notify anyway, even if LLVM does that one on its own today.

Cheers,
Miguel

IMO, inlining should be mostly decided by compilers instead of programmers, so if we don't have evidences that compilers need some guidance, we shouldn't introduce the #[inline] attribute.

Maybe Kunwu has an example saying otherwise?

Regards,
Boqun

Thanks for the kind reminder, I'll follow this next time

Now, the default '-Copt-level' is 2 when define 'CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE', and in  KBUILD_RUSTFLAGS the  '-Ccodegen-units' is 1.

So if the config not change, the result should be the same.

And the actualy compile steps for rust/kernel.o is as following:

make -f ./scripts/Makefile.build obj=rust
# RUSTC L rust/kernel.o
OBJTREE=/media/kunwuchan/68d922f3-5dde-45b0-af93-c45ba81d85ef/linux-next rustc --edition=2021 -Zbinary_dep_depinfo=y -Astable_features -Dnon_ascii_idents -Dunsafe_op_in_unsafe_fn -Wmissing_docs -Wrust_2018_idioms -Wunreachable_pub -Wclippy::all -Wclippy::ignored_unit_patterns -Wclippy::mut_mut -Wclippy::needless_bitwise_bool -Wclippy::needless_continue -Aclippy::needless_lifetimes -Wclippy::no_mangle_with_rust_abi -Wclippy::undocumented_unsafe_blocks -Wclippy::unnecessary_safety_comment -Wclippy::unnecessary_safety_doc -Wrustdoc::missing_crate_level_docs -Wrustdoc::unescaped_backticks -Cpanic=abort -Cembed-bitcode=n -Clto=n -Cforce-unwind-tables=n -Ccodegen-units=1 -Csymbol-mangling-version=v0 -Crelocation-model=static -Zfunction-sections=n -Wclippy::float_arithmetic --target=aarch64-unknown-none-softfloat -Cforce-unwind-tables=n -Zbranch-protection=bti,pac-ret -Copt-level=2 -Cdebug-assertions=y -Coverflow-checks=y -Cforce-frame-pointers=y -Cdebuginfo=1 @./include/generated/rustc_cfg --extern ffi --extern pin_init --extern build_error --extern macros --extern bindings --extern uapi --emit=dep-info=rust/.kernel.o.d --emit=obj=rust/kernel.o --emit=metadata=rust/libkernel.rmeta --crate-type rlib -L./rust --crate-name kernel rust/kernel/lib.rs --sysroot=/dev/null # EXPORTS rust/exports_kernel_generated.h
  llvm-nm -p --defined-only rust/kernel.o | awk '$2~/(T|R|D|B)/ && $3!~/__cfi/ && $3!~/__odr_asan/ { printf "EXPORT_SYMBOL_RUST_GPL(%s);
",$3 }' > rust/exports_kernel_generated.h

Thanks,
  Kunwu.Chan(Tao.Chan)

You're welcome!

I don't think that is true, because there are a few rustc versions as well as LLVM ones that we support, so how inlining happens (at both rustc and LLVM levels) may change even if the configuration is essentially the same.

And, of course, then there are other non-compiler-related kernel config options (i.e. not compiler flags, but other stuff, like cfgs) that influence which and how gets emitted and thus the inlining decisions.

Eventually we should have pure GCC builds too, which is yet another factor...

To be clear, I agree with Boqun that having to write #[inline] everywhere is not great. Rust 1.75 already started to tag small functions as #[inline] to try to prevent the proliferation of the attribute, which is good (i.e. which is what triggered the message in Compiler Explorer).

By the way, one difference is whether it is pub -- the notify() isn't (unlike the others), but if it were, then we would have needed the #[inline], from a quick test.

Thanks!

Cheers,
Miguel

Thanks for the detailed reply.

Sure on the one hand, the decisions is a result of many factors,

on the other hand the rustc and llvm is rapidly developing.

It's a complicated thing.

That is a easy way to judge. I learned.

Thanks,
  Kunwu.Chan(Tao.Chan)

I'm taking this via tip with the version as it is (i.e. no #[inline] for notify()), but will reorder the tags. Thanks!

Regards,
Boqun