[RFC,04/11] rust: block: introduce `kernel::block::bio` module

  From: Andreas Hindborg <a.hindborg@samsung.com>

Add abstractions for working with `struct bio`.

Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com>
---
 rust/kernel/block.rs            |   1 +
 rust/kernel/block/bio.rs        |  93 ++++++++++++++++
 rust/kernel/block/bio/vec.rs    | 181 ++++++++++++++++++++++++++++++++
 rust/kernel/block/mq/request.rs |  16 +++
 4 files changed, 291 insertions(+)
 create mode 100644 rust/kernel/block/bio.rs
 create mode 100644 rust/kernel/block/bio/vec.rs
  

On Wednesday, May 3rd, 2023 at 11:07, Andreas Hindborg <nmi@metaspace.dk> wrote:
> From: Andreas Hindborg <a.hindborg@samsung.com>
> 
> Add abstractions for working with `struct bio`.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com>
> ---
>  rust/kernel/block.rs            |   1 +
>  rust/kernel/block/bio.rs        |  93 ++++++++++++++++
>  rust/kernel/block/bio/vec.rs    | 181 ++++++++++++++++++++++++++++++++
>  rust/kernel/block/mq/request.rs |  16 +++
>  4 files changed, 291 insertions(+)
>  create mode 100644 rust/kernel/block/bio.rs
>  create mode 100644 rust/kernel/block/bio/vec.rs
> 
> diff --git a/rust/kernel/block.rs b/rust/kernel/block.rs
> index 4c93317a568a..1797859551fd 100644
> --- a/rust/kernel/block.rs
> +++ b/rust/kernel/block.rs
> @@ -2,4 +2,5 @@
> 
>  //! Types for working with the block layer
> 
> +pub mod bio;
>  pub mod mq;
> diff --git a/rust/kernel/block/bio.rs b/rust/kernel/block/bio.rs
> new file mode 100644
> index 000000000000..6e93e4420105
> --- /dev/null
> +++ b/rust/kernel/block/bio.rs
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Types for working with the bio layer.
> +//!
> +//! C header: [`include/linux/blk_types.h`](../../include/linux/blk_types.h)
> +
> +use core::fmt;
> +use core::ptr::NonNull;
> +
> +mod vec;
> +
> +pub use vec::BioSegmentIterator;
> +pub use vec::Segment;
> +
> +/// A wrapper around a `struct bio` pointer
> +///
> +/// # Invariants
> +///
> +/// First field must alwyas be a valid pointer to a valid `struct bio`.
> +pub struct Bio<'a>(
> +    NonNull<crate::bindings::bio>,
> +    core::marker::PhantomData<&'a ()>,

Please make this a struct with named fields. Also this is rather a
`BioRef`, right? Why can't `&Bio` be used and `Bio` embeds
`bindings::bio`?

> +);
> +
> +impl<'a> Bio<'a> {
> +    /// Returns an iterator over segments in this `Bio`. Does not consider
> +    /// segments of other bios in this bio chain.
> +    #[inline(always)]

Why are these `inline(always)`? The compiler should inline them
automatically?

> +    pub fn segment_iter(&'a self) -> BioSegmentIterator<'a> {
> +        BioSegmentIterator::new(self)
> +    }
> +
> +    /// Get a pointer to the `bio_vec` off this bio
> +    #[inline(always)]
> +    fn io_vec(&self) -> *const bindings::bio_vec {
> +        // SAFETY: By type invariant, get_raw() returns a valid pointer to a
> +        // valid `struct bio`
> +        unsafe { (*self.get_raw()).bi_io_vec }
> +    }
> +
> +    /// Return a copy of the `bvec_iter` for this `Bio`
> +    #[inline(always)]
> +    fn iter(&self) -> bindings::bvec_iter {

Why does this return the bindings iter? Maybe rename to `raw_iter`?

> +        // SAFETY: self.0 is always a valid pointer
> +        unsafe { (*self.get_raw()).bi_iter }
> +    }
> +
> +    /// Get the next `Bio` in the chain
> +    #[inline(always)]
> +    fn next(&self) -> Option<Bio<'a>> {
> +        // SAFETY: self.0 is always a valid pointer
> +        let next = unsafe { (*self.get_raw()).bi_next };
> +        Some(Self(NonNull::new(next)?, core::marker::PhantomData))

Missing `INVARIANT` explaining why `next` is valid or null. Also why not
use `Self::from_raw` here?

> +    }
> +
> +    /// Return the raw pointer of the wrapped `struct bio`
> +    #[inline(always)]
> +    fn get_raw(&self) -> *const bindings::bio {
> +        self.0.as_ptr()
> +    }
> +
> +    /// Create an instance of `Bio` from a raw pointer. Does check that the
> +    /// pointer is not null.
> +    #[inline(always)]
> +    pub(crate) unsafe fn from_raw(ptr: *mut bindings::bio) -> Option<Bio<'a>> {
> +        Some(Self(NonNull::new(ptr)?, core::marker::PhantomData))
> +    }
> +}
> +
> +impl core::fmt::Display for Bio<'_> {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        write!(f, "Bio {:?}", self.0.as_ptr())

this will display as `Bio 0x7ff0654..` I think there should be some
symbol wrapping the pointer like `Bio(0x7ff0654)` or `Bio { ptr: 0x7ff0654 }`.

> +    }
> +}
> +
> +/// An iterator over `Bio`
> +pub struct BioIterator<'a> {
> +    pub(crate) bio: Option<Bio<'a>>,
> +}
> +
> +impl<'a> core::iter::Iterator for BioIterator<'a> {
> +    type Item = Bio<'a>;
> +
> +    #[inline(always)]
> +    fn next(&mut self) -> Option<Bio<'a>> {
> +        if let Some(current) = self.bio.take() {
> +            self.bio = current.next();
> +            Some(current)
> +        } else {
> +            None
> +        }

Can be rewritten as:
    let current = self.bio.take()?;
    self.bio = current.next();
    Some(cur)

> +    }
> +}
> diff --git a/rust/kernel/block/bio/vec.rs b/rust/kernel/block/bio/vec.rs
> new file mode 100644
> index 000000000000..acd328a6fe54
> --- /dev/null
> +++ b/rust/kernel/block/bio/vec.rs
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Types for working with `struct bio_vec` IO vectors
> +//!
> +//! C header: [`include/linux/bvec.h`](../../include/linux/bvec.h)
> +
> +use super::Bio;
> +use crate::error::Result;
> +use crate::pages::Pages;
> +use core::fmt;
> +use core::mem::ManuallyDrop;
> +
> +#[inline(always)]
> +fn mp_bvec_iter_offset(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
> +    (unsafe { (*bvec_iter_bvec(bvec, iter)).bv_offset }) + iter.bi_bvec_done
> +}
> +
> +#[inline(always)]
> +fn mp_bvec_iter_page(
> +    bvec: *const bindings::bio_vec,
> +    iter: &bindings::bvec_iter,
> +) -> *mut bindings::page {
> +    unsafe { (*bvec_iter_bvec(bvec, iter)).bv_page }
> +}
> +
> +#[inline(always)]
> +fn mp_bvec_iter_page_idx(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> usize {
> +    (mp_bvec_iter_offset(bvec, iter) / crate::PAGE_SIZE) as usize
> +}
> +
> +#[inline(always)]
> +fn mp_bvec_iter_len(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
> +    iter.bi_size
> +        .min(unsafe { (*bvec_iter_bvec(bvec, iter)).bv_len } - iter.bi_bvec_done)
> +}
> +
> +#[inline(always)]
> +fn bvec_iter_bvec(
> +    bvec: *const bindings::bio_vec,
> +    iter: &bindings::bvec_iter,
> +) -> *const bindings::bio_vec {
> +    unsafe { bvec.add(iter.bi_idx as usize) }
> +}
> +
> +#[inline(always)]
> +fn bvec_iter_page(
> +    bvec: *const bindings::bio_vec,
> +    iter: &bindings::bvec_iter,
> +) -> *mut bindings::page {
> +    unsafe { mp_bvec_iter_page(bvec, iter).add(mp_bvec_iter_page_idx(bvec, iter)) }
> +}
> +
> +#[inline(always)]
> +fn bvec_iter_len(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
> +    mp_bvec_iter_len(bvec, iter).min(crate::PAGE_SIZE - bvec_iter_offset(bvec, iter))
> +}
> +
> +#[inline(always)]
> +fn bvec_iter_offset(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
> +    mp_bvec_iter_offset(bvec, iter) % crate::PAGE_SIZE
> +}

Why are these functions:
- not marked as `unsafe`?
- undocumented,
- free functions.

Can't these be directly implemented on `Segment<'_>`? If not,
I think we should find some better way or make them `unsafe`.

> +
> +/// A wrapper around a `strutct bio_vec` - a contiguous range of physical memory addresses
> +///
> +/// # Invariants
> +///
> +/// `bio_vec` must always be initialized and valid
> +pub struct Segment<'a> {
> +    bio_vec: bindings::bio_vec,
> +    _marker: core::marker::PhantomData<&'a ()>,
> +}
> +
> +impl Segment<'_> {
> +    /// Get he lenght of the segment in bytes
> +    #[inline(always)]
> +    pub fn len(&self) -> usize {
> +        self.bio_vec.bv_len as usize
> +    }
> +
> +    /// Returns true if the length of the segment is 0
> +    #[inline(always)]
> +    pub fn is_empty(&self) -> bool {
> +        self.len() == 0
> +    }
> +
> +    /// Get the offset field of the `bio_vec`
> +    #[inline(always)]
> +    pub fn offset(&self) -> usize {
> +        self.bio_vec.bv_offset as usize
> +    }
> +
> +    /// Copy data of this segment into `page`.
> +    #[inline(always)]
> +    pub fn copy_to_page_atomic(&self, page: &mut Pages<0>) -> Result {
> +        // SAFETY: self.bio_vec is valid and thus bv_page must be a valid
> +        // pointer to a `struct page`. We do not own the page, but we prevent
> +        // drop by wrapping the `Pages` in `ManuallyDrop`.
> +        let our_page = ManuallyDrop::new(unsafe { Pages::<0>::from_raw(self.bio_vec.bv_page) });
> +        let our_map = our_page.kmap_atomic();
> +
> +        // TODO: Checck offset is within page - what guarantees does `bio_vec` provide?
> +        let ptr = unsafe { (our_map.get_ptr() as *const u8).add(self.offset()) };
> +
> +        unsafe { page.write_atomic(ptr, self.offset(), self.len()) }
> +    }
> +
> +    /// Copy data from `page` into this segment
> +    #[inline(always)]
> +    pub fn copy_from_page_atomic(&mut self, page: &Pages<0>) -> Result {
> +        // SAFETY: self.bio_vec is valid and thus bv_page must be a valid
> +        // pointer to a `struct page`. We do not own the page, but we prevent
> +        // drop by wrapping the `Pages` in `ManuallyDrop`.
> +        let our_page = ManuallyDrop::new(unsafe { Pages::<0>::from_raw(self.bio_vec.bv_page) });
> +        let our_map = our_page.kmap_atomic();
> +
> +        // TODO: Checck offset is within page
> +        let ptr = unsafe { (our_map.get_ptr() as *mut u8).add(self.offset()) };
> +
> +        unsafe { page.read_atomic(ptr, self.offset(), self.len()) }
> +    }
> +}
> +
> +impl core::fmt::Display for Segment<'_> {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        write!(
> +            f,
> +            "Segment {:?} len: {}",
> +            self.bio_vec.bv_page, self.bio_vec.bv_len
> +        )
> +    }
> +}
> +
> +/// An iterator over `Segment`
> +pub struct BioSegmentIterator<'a> {
> +    bio: &'a Bio<'a>,
> +    iter: bindings::bvec_iter,
> +}
> +
> +impl<'a> BioSegmentIterator<'a> {
> +    #[inline(always)]
> +    pub(crate) fn new(bio: &'a Bio<'a>) -> BioSegmentIterator<'_> {
> +        Self {
> +            bio,
> +            iter: bio.iter(),
> +        }
> +    }
> +}
> +
> +impl<'a> core::iter::Iterator for BioSegmentIterator<'a> {
> +    type Item = Segment<'a>;
> +
> +    #[inline(always)]
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.iter.bi_size == 0 {
> +            return None;
> +        }
> +
> +        // Macro
> +        // bio_vec = bio_iter_iovec(bio, self.iter)
> +        // bio_vec = bvec_iter_bvec(bio.bi_io_vec, self.iter);

Weird comment?

> +        let bio_vec_ret = bindings::bio_vec {
> +            bv_page: bvec_iter_page(self.bio.io_vec(), &self.iter),
> +            bv_len: bvec_iter_len(self.bio.io_vec(), &self.iter),
> +            bv_offset: bvec_iter_offset(self.bio.io_vec(), &self.iter),
> +        };
> +
> +        // Static C function
> +        unsafe {
> +            bindings::bio_advance_iter_single(
> +                self.bio.get_raw(),
> +                &mut self.iter as *mut bindings::bvec_iter,
> +                bio_vec_ret.bv_len,
> +            )
> +        };
> +
> +        Some(Segment {
> +            bio_vec: bio_vec_ret,
> +            _marker: core::marker::PhantomData,
> +        })
> +    }
> +}
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index e95ae3fd71ad..ccb1033b64b6 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -11,6 +11,9 @@ use crate::{
>  };
>  use core::marker::PhantomData;
> 
> +use crate::block::bio::Bio;
> +use crate::block::bio::BioIterator;
> +
>  /// A wrapper around a blk-mq `struct request`. This represents an IO request.
>  pub struct Request<T: Operations> {
>      ptr: *mut bindings::request,
> @@ -63,6 +66,19 @@ impl<T: Operations> Request<T> {
>          }
>      }
> 
> +    /// Get a wrapper for the first Bio in this request
> +    #[inline(always)]
> +    pub fn bio(&self) -> Option<Bio<'_>> {
> +        let ptr = unsafe { (*self.ptr).bio };
> +        unsafe { Bio::from_raw(ptr) }
> +    }
> +
> +    /// Get an iterator over all bio structurs in this request
> +    #[inline(always)]
> +    pub fn bio_iter(&self) -> BioIterator<'_> {
> +        BioIterator { bio: self.bio() }
> +    }
> +
>      /// Get the target sector for the request
>      #[inline(always)]
>      pub fn sector(&self) -> usize {
> --
> 2.40.0
> 

--
Cheers,
Benno
  

Benno Lossin <benno.lossin@proton.me> writes:

>> +/// A wrapper around a `struct bio` pointer
>> +///
>> +/// # Invariants
>> +///
>> +/// First field must alwyas be a valid pointer to a valid `struct bio`.
>> +pub struct Bio<'a>(
>> +    NonNull<crate::bindings::bio>,
>> +    core::marker::PhantomData<&'a ()>,
>
> Please make this a struct with named fields. Also this is rather a
> `BioRef`, right? Why can't `&Bio` be used and `Bio` embeds
> `bindings::bio`?

Yes, it feels better with a &Bio reference, thanks.

>> +);
>> +
>> +impl<'a> Bio<'a> {
>> +    /// Returns an iterator over segments in this `Bio`. Does not consider
>> +    /// segments of other bios in this bio chain.
>> +    #[inline(always)]
>
> Why are these `inline(always)`? The compiler should inline them
> automatically?

No, the compiler would not inline into modules without them. I'll check
again if that is still the case.

>
>> +    pub fn segment_iter(&'a self) -> BioSegmentIterator<'a> {
>> +        BioSegmentIterator::new(self)
>> +    }
>> +
>> +    /// Get a pointer to the `bio_vec` off this bio
>> +    #[inline(always)]
>> +    fn io_vec(&self) -> *const bindings::bio_vec {
>> +        // SAFETY: By type invariant, get_raw() returns a valid pointer to a
>> +        // valid `struct bio`
>> +        unsafe { (*self.get_raw()).bi_io_vec }
>> +    }
>> +
>> +    /// Return a copy of the `bvec_iter` for this `Bio`
>> +    #[inline(always)]
>> +    fn iter(&self) -> bindings::bvec_iter {
>
> Why does this return the bindings iter? Maybe rename to `raw_iter`?

Makes sense to rename it, thanks.

>> +        // SAFETY: self.0 is always a valid pointer
>> +        unsafe { (*self.get_raw()).bi_iter }
>> +    }
>> +
>> +    /// Get the next `Bio` in the chain
>> +    #[inline(always)]
>> +    fn next(&self) -> Option<Bio<'a>> {
>> +        // SAFETY: self.0 is always a valid pointer
>> +        let next = unsafe { (*self.get_raw()).bi_next };
>> +        Some(Self(NonNull::new(next)?, core::marker::PhantomData))
>
> Missing `INVARIANT` explaining why `next` is valid or null. Also why not
> use `Self::from_raw` here?

Thanks, will change that.

>
>> +    }
>> +
>> +    /// Return the raw pointer of the wrapped `struct bio`
>> +    #[inline(always)]
>> +    fn get_raw(&self) -> *const bindings::bio {
>> +        self.0.as_ptr()
>> +    }
>> +
>> +    /// Create an instance of `Bio` from a raw pointer. Does check that the
>> +    /// pointer is not null.
>> +    #[inline(always)]
>> +    pub(crate) unsafe fn from_raw(ptr: *mut bindings::bio) -> Option<Bio<'a>> {
>> +        Some(Self(NonNull::new(ptr)?, core::marker::PhantomData))
>> +    }
>> +}
>> +
>> +impl core::fmt::Display for Bio<'_> {
>> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> +        write!(f, "Bio {:?}", self.0.as_ptr())
>
> this will display as `Bio 0x7ff0654..` I think there should be some
> symbol wrapping the pointer like `Bio(0x7ff0654)` or `Bio { ptr: 0x7ff0654 }`.
>

Sure 👍

>> +    }
>> +}
>> +
>> +/// An iterator over `Bio`
>> +pub struct BioIterator<'a> {
>> +    pub(crate) bio: Option<Bio<'a>>,
>> +}
>> +
>> +impl<'a> core::iter::Iterator for BioIterator<'a> {
>> +    type Item = Bio<'a>;
>> +
>> +    #[inline(always)]
>> +    fn next(&mut self) -> Option<Bio<'a>> {
>> +        if let Some(current) = self.bio.take() {
>> +            self.bio = current.next();
>> +            Some(current)
>> +        } else {
>> +            None
>> +        }
>
> Can be rewritten as:
>     let current = self.bio.take()?;
>     self.bio = current.next();
>     Some(cur)
>

Thanks 👍

>> +    }
>> +}
>> diff --git a/rust/kernel/block/bio/vec.rs b/rust/kernel/block/bio/vec.rs
>> new file mode 100644
>> index 000000000000..acd328a6fe54
>> --- /dev/null
>> +++ b/rust/kernel/block/bio/vec.rs
>> @@ -0,0 +1,181 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Types for working with `struct bio_vec` IO vectors
>> +//!
>> +//! C header: [`include/linux/bvec.h`](../../include/linux/bvec.h)
>> +
>> +use super::Bio;
>> +use crate::error::Result;
>> +use crate::pages::Pages;
>> +use core::fmt;
>> +use core::mem::ManuallyDrop;
>> +
>> +#[inline(always)]
>> +fn mp_bvec_iter_offset(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
>> +    (unsafe { (*bvec_iter_bvec(bvec, iter)).bv_offset }) + iter.bi_bvec_done
>> +}
>> +
>> +#[inline(always)]
>> +fn mp_bvec_iter_page(
>> +    bvec: *const bindings::bio_vec,
>> +    iter: &bindings::bvec_iter,
>> +) -> *mut bindings::page {
>> +    unsafe { (*bvec_iter_bvec(bvec, iter)).bv_page }
>> +}
>> +
>> +#[inline(always)]
>> +fn mp_bvec_iter_page_idx(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> usize {
>> +    (mp_bvec_iter_offset(bvec, iter) / crate::PAGE_SIZE) as usize
>> +}
>> +
>> +#[inline(always)]
>> +fn mp_bvec_iter_len(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
>> +    iter.bi_size
>> +        .min(unsafe { (*bvec_iter_bvec(bvec, iter)).bv_len } - iter.bi_bvec_done)
>> +}
>> +
>> +#[inline(always)]
>> +fn bvec_iter_bvec(
>> +    bvec: *const bindings::bio_vec,
>> +    iter: &bindings::bvec_iter,
>> +) -> *const bindings::bio_vec {
>> +    unsafe { bvec.add(iter.bi_idx as usize) }
>> +}
>> +
>> +#[inline(always)]
>> +fn bvec_iter_page(
>> +    bvec: *const bindings::bio_vec,
>> +    iter: &bindings::bvec_iter,
>> +) -> *mut bindings::page {
>> +    unsafe { mp_bvec_iter_page(bvec, iter).add(mp_bvec_iter_page_idx(bvec, iter)) }
>> +}
>> +
>> +#[inline(always)]
>> +fn bvec_iter_len(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
>> +    mp_bvec_iter_len(bvec, iter).min(crate::PAGE_SIZE - bvec_iter_offset(bvec, iter))
>> +}
>> +
>> +#[inline(always)]
>> +fn bvec_iter_offset(bvec: *const bindings::bio_vec, iter: &bindings::bvec_iter) -> u32 {
>> +    mp_bvec_iter_offset(bvec, iter) % crate::PAGE_SIZE
>> +}
>
> Why are these functions:
> - not marked as `unsafe`?
> - undocumented,
> - free functions.
>
> Can't these be directly implemented on `Segment<'_>`? If not,
> I think we should find some better way or make them `unsafe`.

Yes, you are right. I will move them to `Segment` and document them
better. They definitely need to be unsafe because they rely on C API
contract that the iterator is not indexing out of bounds.

[...]

>> +impl<'a> core::iter::Iterator for BioSegmentIterator<'a> {
>> +    type Item = Segment<'a>;
>> +
>> +    #[inline(always)]
>> +    fn next(&mut self) -> Option<Self::Item> {
>> +        if self.iter.bi_size == 0 {
>> +            return None;
>> +        }
>> +
>> +        // Macro
>> +        // bio_vec = bio_iter_iovec(bio, self.iter)
>> +        // bio_vec = bvec_iter_bvec(bio.bi_io_vec, self.iter);
>
> Weird comment?

Yes, will fix, thanks.

Thanks for the comments!

BR Andreas
  

Hi Benno,

"Andreas Hindborg (Samsung)" <nmi@metaspace.dk> writes:

<cut>

>>> +);
>>> +
>>> +impl<'a> Bio<'a> {
>>> +    /// Returns an iterator over segments in this `Bio`. Does not consider
>>> +    /// segments of other bios in this bio chain.
>>> +    #[inline(always)]
>>
>> Why are these `inline(always)`? The compiler should inline them
>> automatically?
>
> No, the compiler would not inline into modules without them. I'll check
> again if that is still the case.

I just tested this again. If I remove the attribute, the compiler will
inline some of the functions but not others. I guess it depends on the
inlining heuristics of rustc. The majority of the attributes I have put
is not necessary, since the compiler will inline by default. But for
instance `<BioIterator as Iterator>::next` is not inlined by default and
it really should be inlined.

Since most of the attributes do not change compiler default behavior, I
would rather tag all functions that I want inlined than have to
disassemble build outputs to check which functions actually need the
attribute. With this approach, we are not affected by changes to
compiler heuristics either.

What do you think?

Best regards,
Andreas