[PATCH] rust: page:: optimize rust symbol generation for Page

[PATCH] rust: page:: optimize rust symbol generation for Page

[Kunwu Chan]

From: Kunwu Chan <kunwu.chan@hotmail.com>

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'.*Page | rustfilt
ffff8000805b6f98 T <kernel::page::Page>::alloc_page
ffff8000805b715c T <kernel::page::Page>::fill_zero_raw
ffff8000805b720c T <kernel::page::Page>::copy_from_user_slice_raw
ffff8000805b6fb4 T <kernel::page::Page>::read_raw
ffff8000805b7088 T <kernel::page::Page>::write_raw
ffff8000805b72fc T <kernel::page::Page as core::ops::drop::Drop>::drop

These Rust symbols are trivial wrappers around the C functions
alloc_pages, kunmap_local and __free_pages.
It doesn't make sense to go through a trivial wrapper for these
functions, so mark them inline.

Link: https://github.com/Rust-for-Linux/linux/issues/1145
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Kunwu Chan <kunwu.chan@hotmail.com>
---
 rust/kernel/page.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index f6126aca33a6..e75cbc5cafd4 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -69,6 +69,7 @@ impl Page {
     /// let page = Page::alloc_page(GFP_KERNEL | __GFP_ZERO)?;
     /// # Ok::<(), kernel::alloc::AllocError>(())
     /// ```
+    #[inline]
     pub fn alloc_page(flags: Flags) -> Result<Self, AllocError> {
         // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
         // is always safe to call this method.
@@ -170,6 +171,7 @@ fn with_pointer_into_page<T>(
     /// * Callers must ensure that `dst` is valid for writing `len` bytes.
     /// * Callers must ensure that this call does not race with a write to the same page that
     ///   overlaps with this read.
+    #[inline]
     pub unsafe fn read_raw(&self, dst: *mut u8, offset: usize, len: usize) -> Result {
         self.with_pointer_into_page(offset, len, move |src| {
             // SAFETY: If `with_pointer_into_page` calls into this closure, then
@@ -192,6 +194,7 @@ pub unsafe fn read_raw(&self, dst: *mut u8, offset: usize, len: usize) -> Result
     /// * Callers must ensure that `src` is valid for reading `len` bytes.
     /// * Callers must ensure that this call does not race with a read or write to the same page
     ///   that overlaps with this write.
+    #[inline]
     pub unsafe fn write_raw(&self, src: *const u8, offset: usize, len: usize) -> Result {
         self.with_pointer_into_page(offset, len, move |dst| {
             // SAFETY: If `with_pointer_into_page` calls into this closure, then it has performed a
@@ -212,6 +215,7 @@ pub unsafe fn write_raw(&self, src: *const u8, offset: usize, len: usize) -> Res
     ///
     /// Callers must ensure that this call does not race with a read or write to the same page that
     /// overlaps with this write.
+    #[inline]
     pub unsafe fn fill_zero_raw(&self, offset: usize, len: usize) -> Result {
         self.with_pointer_into_page(offset, len, move |dst| {
             // SAFETY: If `with_pointer_into_page` calls into this closure, then it has performed a
@@ -235,6 +239,7 @@ pub unsafe fn fill_zero_raw(&self, offset: usize, len: usize) -> Result {
     ///
     /// Callers must ensure that this call does not race with a read or write to the same page that
     /// overlaps with this write.
+    #[inline]
     pub unsafe fn copy_from_user_slice_raw(
         &self,
         reader: &mut UserSliceReader,
@@ -251,6 +256,7 @@ pub unsafe fn copy_from_user_slice_raw(
 }
 
 impl Drop for Page {
+    #[inline]
     fn drop(&mut self) {
         // SAFETY: By the type invariants, we have ownership of the page and can free it.
         unsafe { bindings::__free_pages(self.page.as_ptr(), 0) };
-- 
2.43.0

Re: [PATCH] rust: page:: optimize rust symbol generation for Page

[Alice Ryhl]

On Mon, Mar 17, 2025 at 05:40:04PM +0800, Kunwu Chan wrote:
> From: Kunwu Chan <kunwu.chan@hotmail.com>
> 
> 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'.*Page | rustfilt
> ffff8000805b6f98 T <kernel::page::Page>::alloc_page
> ffff8000805b715c T <kernel::page::Page>::fill_zero_raw
> ffff8000805b720c T <kernel::page::Page>::copy_from_user_slice_raw
> ffff8000805b6fb4 T <kernel::page::Page>::read_raw
> ffff8000805b7088 T <kernel::page::Page>::write_raw
> ffff8000805b72fc T <kernel::page::Page as core::ops::drop::Drop>::drop
> 
> These Rust symbols are trivial wrappers around the C functions
> alloc_pages, kunmap_local and __free_pages.
> It doesn't make sense to go through a trivial wrapper for these
> functions, so mark them inline.
> 
> Link: https://github.com/Rust-for-Linux/linux/issues/1145
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Co-developed-by: Grace Deng <Grace.Deng006@Gmail.com>
> Signed-off-by: Grace Deng <Grace.Deng006@Gmail.com>
> Signed-off-by: Kunwu Chan <kunwu.chan@hotmail.com>

For sure `alloc_page` and `drop` should be inline, but the other methods
are not as simple. It is less clear that they should be inline.

At the very least, the claim that they are a trivial wrapper around
"kunmap_local" is false. They don't just call that method.

Alice

Re: [PATCH] rust: page:: optimize rust symbol generation for Page

[Kunwu Chan]

On 2025/3/17 18:33, Alice Ryhl wrote:
> On Mon, Mar 17, 2025 at 05:40:04PM +0800, Kunwu Chan wrote:
>> From: Kunwu Chan <kunwu.chan@hotmail.com>
>>
>> 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'.*Page | rustfilt
>> ffff8000805b6f98 T <kernel::page::Page>::alloc_page
>> ffff8000805b715c T <kernel::page::Page>::fill_zero_raw
>> ffff8000805b720c T <kernel::page::Page>::copy_from_user_slice_raw
>> ffff8000805b6fb4 T <kernel::page::Page>::read_raw
>> ffff8000805b7088 T <kernel::page::Page>::write_raw
>> ffff8000805b72fc T <kernel::page::Page as core::ops::drop::Drop>::drop
>>
>> These Rust symbols are trivial wrappers around the C functions
>> alloc_pages, kunmap_local and __free_pages.
>> It doesn't make sense to go through a trivial wrapper for these
>> functions, so mark them inline.
>>
>> Link: https://github.com/Rust-for-Linux/linux/issues/1145
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Co-developed-by: Grace Deng <Grace.Deng006@Gmail.com>
>> Signed-off-by: Grace Deng <Grace.Deng006@Gmail.com>
>> Signed-off-by: Kunwu Chan <kunwu.chan@hotmail.com>
> For sure `alloc_page` and `drop` should be inline, but the other methods
> are not as simple. It is less clear that they should be inline.
>
> At the very least, the claim that they are a trivial wrapper around
> "kunmap_local" is false. They don't just call that method.

Yes, I'm not sure if that's the case, cause there are more layers of 
nesting and it's more complex.

 From objdump, it can be seen that LLVM will currently inline according 
to the 'inline' mark.

$aarch64-linux-gnu-objdump -d vmlinux | rustfilt | grep -A 20 
"kernel::page::"
ffff8000805b6f6c <kernel::page::page_align>:
ffff8000805b6f6c:       d503245f        bti     c
ffff8000805b6f70:       b13ffc08        adds    x8, x0, #0xfff
ffff8000805b6f74:       54000062        b.cs    ffff8000805b6f80 
<kernel::page::page_align+0x14>  // b.hs,
b.nlast
ffff8000805b6f78:       9274cd00        and     x0, x8, #0xfffffffffffff000
ffff8000805b6f7c:       d65f03c0        ret
ffff8000805b6f80:       d503233f        paciasp
ffff8000805b6f84:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
ffff8000805b6f88:       910003fd        mov     x29, sp
ffff8000805b6f8c:       d0006420        adrp    x0, ffff80008123c000 
<core::unicode::unicode_data::white_sp
ace::WHITESPACE_MAP+0x6756>
ffff8000805b6f90:       910b6000        add     x0, x0, #0x2d8
ffff8000805b6f94:       97e98ac3        bl      ffff800080019aa0 
<core::panicking::panic_const::panic_const
_add_overflow>

ffff8000805b6f98 <<kernel::pci::Device>::as_raw>:
ffff8000805b6f98:       d503245f        bti     c
ffff8000805b6f9c:       f9400008        ldr     x8, [x0]
ffff8000805b6fa0:       f1031d1f        cmp     x8, #0xc7
ffff8000805b6fa4:       54000069        b.ls    ffff8000805b6fb0 
<<kernel::pci::Device>::as_raw+0x18>  // b
.plast
ffff8000805b6fa8:       d1032100        sub     x0, x8, #0xc8
ffff8000805b6fac:       d65f03c0        ret
ffff8000805b6fb0:       d503233f        paciasp
ffff8000805b6fb4:       a9bf7bfd        stp     x29, x30, [sp, #-16]!
ffff8000805b6fb8:       910003fd        mov     x29, sp
ffff8000805b6fbc:       b0006420        adrp    x0, ffff80008123b000 
<core::unicode::unicode_data::white_sp
ace::WHITESPACE_MAP+0x5756>


Either we commits and merges the 'alloc_page' and 'drop' first.

I'll change it in the v2 version.

>
> Alice

-- 
Thanks,
   Kunwu.Chan