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

From: Benno Lossin <benno.lossin@proton.me>
To: Andreas Hindborg <nmi@metaspace.dk>
Cc: "Jens Axboe" <axboe@kernel.dk>, "Christoph Hellwig" <hch@lst.de>,
	"Keith Busch" <kbusch@kernel.org>,
	"Damien Le Moal" <Damien.LeMoal@wdc.com>,
	"Hannes Reinecke" <hare@suse.de>,
	lsf-pc@lists.linux-foundation.org,
	rust-for-linux@vger.kernel.org, linux-block@vger.kernel.org,
	"Matthew Wilcox" <willy@infradead.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	linux-kernel@vger.kernel.org, gost.dev@samsung.com
Subject: Re: [RFC PATCH 04/11] rust: block: introduce `kernel::block::bio` module
Date: Sat, 09 Mar 2024 12:30:49 +0000	[thread overview]
Message-ID: <6cd89162-a11f-4a2f-a609-b6d51caf6ba1@proton.me> (raw)
In-Reply-To: <878r34aboo.fsf@metaspace.dk>

On 2/28/24 15:31, Andreas Hindborg wrote:
> 
> 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?

I think that you should do whatever leads to the best results in
practice. I know that the compiler developers spend considerable time
coming up with smart algorithms for deciding when and when not to inline
functions. But they aren't perfect, so if you find them necessary then
please add them.
What I want to avoid is that we end up tagging every function
`inline(always)`, or at least we do not check if it makes a difference.

-- 
Cheers,
Benno