rust: page:: optimize rust symbol generation for Page
Subsystems (2)
PATCH 1/1 rust: page:: optimize rust symbol generation for Page
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'.*Page | rustfiltffff8000805b6f98 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.
--- rust/kernel/page.rs | 6 ++++++ 1 file changed, 6 insertions(+)
rust/kernel/page.rs +6 -0
@@ -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 ) };
Signature · 1 line
2.43.0
Alice Ryhl
maintainerQuoted 18 lines · Kunwu Chan
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'.*Page | rustfilt
ffff8000805b6f98T <kernel::page::Page>::alloc_pageffff8000805b715cT <kernel::page::Page>::fill_zero_rawffff8000805b720cT <kernel::page::Page>::copy_from_user_slice_rawffff8000805b6fb4T <kernel::page::Page>::read_rawffff8000805b7088T <kernel::page::Page>::write_rawffff8000805b72fcT <kernel::page::Pageas core::ops::drop::Drop>::dropThese Rust symbols are trivial wrappers around the C functions
alloc_pages,kunmap_localand__free_pages.
It doesn't make sense to go through a trivial wrapper for these functions, so mark them inline.
Quoted 5 lines
Suggested-by Alice Ryhl <[email protected]>Co-developed-by Grace Deng <[email protected]>Signed-off-by Grace Deng <[email protected]>Signed-off-by Kunwu Chan <[email protected]>
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
Quoted 24 lines · Alice Ryhl
Quoted 18 lines · Kunwu Chan
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'.*Page | rustfilt
ffff8000805b6f98T <kernel::page::Page>::alloc_pageffff8000805b715cT <kernel::page::Page>::fill_zero_rawffff8000805b720cT <kernel::page::Page>::copy_from_user_slice_rawffff8000805b6fb4T <kernel::page::Page>::read_rawffff8000805b7088T <kernel::page::Page>::write_rawffff8000805b72fcT <kernel::page::Pageas core::ops::drop::Drop>::dropThese Rust symbols are trivial wrappers around the C functions
alloc_pages,kunmap_localand__free_pages.
It doesn't make sense to go through a trivial wrapper for these functions, so mark them inline.Quoted 5 lines
Suggested-by Alice Ryhl <[email protected]>Co-developed-by Grace Deng <[email protected]>Signed-off-by Grace Deng <[email protected]>Signed-off-by Kunwu Chan <[email protected]>
Quoted 5 lines
For sure
alloc_pageanddropshould 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 cffff8000805b6f70: b13ffc08 adds x8, x0, #0xfffffff8000805b6f74: 54000062 b.cs ffff8000805b6f80 <kernel::page::page_align+0x14> // b.hs, b.nlastffff8000805b6f78: 9274cd00 and x0, x8, #0xfffffffffffff000ffff8000805b6f7c: d65f03c0 retffff8000805b6f80: d503233f paciaspffff8000805b6f84: a9bf7bfd stp x29, x30, [sp, #-16]!ffff8000805b6f88: 910003fd mov x29, spffff8000805b6f8c: d0006420 adrp x0, ffff80008123c000 <core::unicode::unicode_data::white_sp ace::WHITESPACE_MAP+0x6756>ffff8000805b6f90: 910b6000 add x0, x0, #0x2d8ffff8000805b6f94: 97e98ac3 bl ffff800080019aa0 <core::panicking::panic_const::panic_const _add_overflow>
ffff8000805b6f98 <<kernel::pci::Device>::as_raw>:ffff8000805b6f98: d503245f bti cffff8000805b6f9c: f9400008 ldr x8, [x0]ffff8000805b6fa0: f1031d1f cmp x8, #0xc7ffff8000805b6fa4: 54000069 b.ls ffff8000805b6fb0 <<kernel::pci::Device>::as_raw+0x18> // b .plastffff8000805b6fa8: d1032100 sub x0, x8, #0xc8ffff8000805b6fac: d65f03c0 retffff8000805b6fb0: d503233f paciaspffff8000805b6fb4: a9bf7bfd stp x29, x30, [sp, #-16]!ffff8000805b6fb8: 910003fd mov x29, spffff8000805b6fbc: 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.
Quoted 2 lines
Alice
Signature · 2 lines
Thanks, Kunwu.Chan