Re: [RFC PATCH 09/19] rust: folio: introduce basic support for folios

[Matthew Wilcox <willy@infradead.org>]

On Mon, Oct 23, 2023 at 12:48:33PM +0200, Andreas Hindborg (Samsung) wrote:
> The build system and Rust compiler can inline and optimize across
> function calls and languages when LTO is enabled. Some patches are
> needed to make it work though.

That's fine, but something like folio_put() is performance-critical.

Relying on the linker to figure out that it _should_ inline through

+void rust_helper_folio_put(struct folio *folio)
+{
+	folio_put(folio);
+}
+EXPORT_SYMBOL_GPL(rust_helper_folio_put);

seems like a very bad idea to me.  For reference, folio_put() is
defined as:

static inline void folio_put(struct folio *folio)
{
        if (folio_put_testzero(folio))
                __folio_put(folio);
}

which turns into (once you work your way through all the gunk that hasn't
been cleaned up yet)

	if (atomic_dec_and_test(&folio->_refcount))
		__folio_put(folio)

ie it's a single dec-and-test insn followed by a conditional function
call.  Yes, there's some expensive debug you can turn on in there, but
it's an incredibly frequent call, and we shouldn't be relying on linker
magic to optimise it all away.

Of course, I don't want to lose the ability to turn on the debug code,
so folio.put() can't be as simple as the call to atomic_dec_and_test(),
but I hope you see my point.

Wedson wrote in a later email,
> Having said that, while it's possible to do what you suggested above,
> we try to avoid it so that maintainers can continue to have a single
> place they need to change if they ever decide to change things. A
> simple example from above is order(), if you decide to implement it
> differently (I don't know, if you change the flag, you decide to have
> an explicit field, whatever), then you'd have to change the C _and_
> the Rust versions. Worse yet, there's a chance that forgetting to
> update the Rust version wouldn't break the build, which would make it
> harder to catch mismatched versions.

I understand that concern!  Indeed, I did change the implementation
of folio_order() recently.  I'm happy to commit to keeping the Rust
implementation updated as I modify the C implementation of folios,
but I appreciate that other maintainers may not be willing to make such
a commitment.

I'm all the way up to Chapter 5: References in the Blandy book now!
I expect to understand the patches you're sending any week now ;-)