[PATCH v4 00/11] rust: bindings: Auto-generate inline static functions

Alistair Francis posted 11 patches 1 year, 2 months ago
There is a newer version of this series
Documentation/rust/general-information.rst | 10 +++---
rust/.gitignore                            |  2 ++
rust/Makefile                              | 37 ++++++++++++++++++++--
rust/bindgen_static_functions              | 32 +++++++++++++++++++
rust/bindings/bindings_helper.h            |  6 ++++
rust/bindings/lib.rs                       |  4 +++
rust/exports.c                             |  1 +
rust/helpers/blk.c                         | 14 --------
rust/helpers/err.c                         | 18 -----------
rust/helpers/helpers.c                     | 11 ++-----
rust/helpers/kunit.c                       |  8 -----
rust/helpers/page.c                        |  5 ---
rust/helpers/rbtree.c                      |  9 ------
rust/helpers/refcount.c                    | 10 ------
rust/helpers/signal.c                      |  8 -----
rust/helpers/spinlock.c                    | 15 ---------
rust/helpers/task.c                        | 10 ------
rust/helpers/uaccess.c                     | 15 ---------
18 files changed, 87 insertions(+), 128 deletions(-)
create mode 100644 rust/bindgen_static_functions
delete mode 100644 rust/helpers/blk.c
delete mode 100644 rust/helpers/err.c
delete mode 100644 rust/helpers/kunit.c
delete mode 100644 rust/helpers/rbtree.c
delete mode 100644 rust/helpers/signal.c
delete mode 100644 rust/helpers/uaccess.c
[PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Alistair Francis 1 year, 2 months ago
The kernel includes a large number of static inline functions that are
defined in header files. One example is the crypto_shash_descsize()
function which is defined in hash.h as

```
static inline unsigned int crypto_shash_descsize(struct crypto_shash *tfm)
{
        return tfm->descsize;
}
```

bindgen is currently unable to generate bindings to these functions as
they are not publically exposed (they are static after all).

The Rust code currently uses rust_helper_* functions, such as
rust_helper_alloc_pages() for example to call the static inline
functions. But this is a hassle as someone needs to write a C helper
function.

Instead we can use the bindgen wrap-static-fns feature. The feature
is marked as experimental, but has recently been promoted to
non-experimental (depending on your version of bindgen).

By supporting wrap-static-fns we automatically generate a C file called
extern.c that exposes the static inline functions, for example like this

```
unsigned int crypto_shash_descsize__extern(struct crypto_shash *tfm) { return crypto_shash_descsize(tfm); }
```

The nice part is that this is auto-generated.

We then also get a bindings_generate_static.rs file with the Rust
binding, like this

```
extern "C" {
    #[link_name = "crypto_shash_descsize__extern"]
    pub fn crypto_shash_descsize(tfm: *mut crypto_shash) -> core::ffi::c_uint;
}
```

So now we can use the static inline functions just like normal
functions.

There are a bunch of static inline functions that don't work though, because
the C compiler fails to build extern.c:
 * functions with inline asm generate "operand probably does not match constraints"
   errors (rip_rel_ptr() for example)
 * functions with bit masks (u32_encode_bits() and friends) result in
   "call to ‘__bad_mask’ declared with attribute error: bad bitfield mask"
   errors

As well as that any static inline function that calls a function that has been
kconfig-ed out will fail to link as the function being called isn't built
(mdio45_ethtool_gset_npage for example)

Due to these failures we use a allow-list system (where functions must
be manually enabled).

This series adds support for bindgen generating wrappers for inline statics and
then converts the existing helper functions to this new method. This doesn't
work for C macros, so we can't reamove all of the helper functions, but we
can remove most.

v4:
 - Fix out of tree builds
v3:
 - Change SoB email address to match from address
 - Fixup kunit test build failure
 - Update Rust binding documentation
v2:
 - Fixup build failures report by test bots
 - Rebase on rust-next (ae7851c29747fa376)

Alistair Francis (11):
  rust: bindings: Support some inline static functions
  rust: helpers: Remove blk helper
  rust: helpers: Remove err helper
  rust: helpers: Remove kunit helper
  rust: helpers: Remove some page helpers
  rust: helpers: Remove rbtree helper
  rust: helpers: Remove some refcount helpers
  rust: helpers: Remove signal helper
  rust: helpers: Remove some spinlock helpers
  rust: helpers: Remove some task helpers
  rust: helpers: Remove uaccess helpers

 Documentation/rust/general-information.rst | 10 +++---
 rust/.gitignore                            |  2 ++
 rust/Makefile                              | 37 ++++++++++++++++++++--
 rust/bindgen_static_functions              | 32 +++++++++++++++++++
 rust/bindings/bindings_helper.h            |  6 ++++
 rust/bindings/lib.rs                       |  4 +++
 rust/exports.c                             |  1 +
 rust/helpers/blk.c                         | 14 --------
 rust/helpers/err.c                         | 18 -----------
 rust/helpers/helpers.c                     | 11 ++-----
 rust/helpers/kunit.c                       |  8 -----
 rust/helpers/page.c                        |  5 ---
 rust/helpers/rbtree.c                      |  9 ------
 rust/helpers/refcount.c                    | 10 ------
 rust/helpers/signal.c                      |  8 -----
 rust/helpers/spinlock.c                    | 15 ---------
 rust/helpers/task.c                        | 10 ------
 rust/helpers/uaccess.c                     | 15 ---------
 18 files changed, 87 insertions(+), 128 deletions(-)
 create mode 100644 rust/bindgen_static_functions
 delete mode 100644 rust/helpers/blk.c
 delete mode 100644 rust/helpers/err.c
 delete mode 100644 rust/helpers/kunit.c
 delete mode 100644 rust/helpers/rbtree.c
 delete mode 100644 rust/helpers/signal.c
 delete mode 100644 rust/helpers/uaccess.c

-- 
2.47.0

Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Andreas Hindborg 1 year, 2 months ago
Hi Alistair,

"Alistair Francis" <alistair@alistair23.me> writes:
>
> This series adds support for bindgen generating wrappers for inline statics and
> then converts the existing helper functions to this new method. This doesn't
> work for C macros, so we can't reamove all of the helper functions, but we
> can remove most.

Are you aware of the helper LTO patches by Gary [1]? Can you tell if
this series will compose with the LTO patches?

Also see this Zulip thread for discussion on the LTO patches [2].


Best regards,
Andreas Hindborg



[1] https://github.com/nbdd0121/linux/tree/thin-lto
[2] https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/LTO.20Rust.20modules.20with.20C.20helpers
Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Alistair Francis 1 year, 1 month ago
On Wed, Dec 4, 2024 at 11:14 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>
> Hi Alistair,
>
> "Alistair Francis" <alistair@alistair23.me> writes:
> >
> > This series adds support for bindgen generating wrappers for inline statics and
> > then converts the existing helper functions to this new method. This doesn't
> > work for C macros, so we can't reamove all of the helper functions, but we
> > can remove most.
>
> Are you aware of the helper LTO patches by Gary [1]? Can you tell if

I was not, but that's exciting to see

> this series will compose with the LTO patches?

I think it will still work, although it might take some extra effort.

I assume my series isn't going to be accepted though, so I'm not going
to try and get the LTO series to work on top of it. It's a lot of work
to rebase this series as every new binding causes a conflict and it
seems to have stalled

Alistair

>
> Also see this Zulip thread for discussion on the LTO patches [2].
>
>
> Best regards,
> Andreas Hindborg
>
>
>
> [1] https://github.com/nbdd0121/linux/tree/thin-lto
> [2] https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/LTO.20Rust.20modules.20with.20C.20helpers
>
Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Andreas Hindborg 1 year, 1 month ago
"Alistair Francis" <alistair23@gmail.com> writes:

> On Wed, Dec 4, 2024 at 11:14 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>>
>> Hi Alistair,
>>
>> "Alistair Francis" <alistair@alistair23.me> writes:
>> >
>> > This series adds support for bindgen generating wrappers for inline statics and
>> > then converts the existing helper functions to this new method. This doesn't
>> > work for C macros, so we can't reamove all of the helper functions, but we
>> > can remove most.
>>
>> Are you aware of the helper LTO patches by Gary [1]? Can you tell if
>
> I was not, but that's exciting to see
>
>> this series will compose with the LTO patches?
>
> I think it will still work, although it might take some extra effort.
>
> I assume my series isn't going to be accepted though, so I'm not going
> to try and get the LTO series to work on top of it. It's a lot of work
> to rebase this series as every new binding causes a conflict and it
> seems to have stalled

I would assume that it _will_ be merged in some form. Why not? It's just
a matter of getting it reviewed and making sure that it is going to work
with the LTO inlining.

The less boiler plate code we have to write by hand, the better!


Best regards,
Andreas Hindborg
Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Alistair Francis 1 year, 1 month ago
On Wed, Dec 18, 2024 at 7:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alistair Francis" <alistair23@gmail.com> writes:
>
> > On Wed, Dec 4, 2024 at 11:14 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >>
> >> Hi Alistair,
> >>
> >> "Alistair Francis" <alistair@alistair23.me> writes:
> >> >
> >> > This series adds support for bindgen generating wrappers for inline statics and
> >> > then converts the existing helper functions to this new method. This doesn't
> >> > work for C macros, so we can't reamove all of the helper functions, but we
> >> > can remove most.
> >>
> >> Are you aware of the helper LTO patches by Gary [1]? Can you tell if
> >
> > I was not, but that's exciting to see
> >
> >> this series will compose with the LTO patches?
> >
> > I think it will still work, although it might take some extra effort.
> >
> > I assume my series isn't going to be accepted though, so I'm not going
> > to try and get the LTO series to work on top of it. It's a lot of work
> > to rebase this series as every new binding causes a conflict and it
> > seems to have stalled
>
> I would assume that it _will_ be merged in some form. Why not? It's just
> a matter of getting it reviewed and making sure that it is going to work
> with the LTO inlining.

Ah, I just haven't heard a lot of feedback, which seems like there
isn't interest in this.

I tried to apply the LTO patches, but I do see build failures, I get
errors like this:

ld.lld: error: duplicate symbol: RUST_CONST_HELPER_ARCH_SLAB_MINALIGN
>>> defined at llvm-link
>>>            rust/kernel.o:(RUST_CONST_HELPER_ARCH_SLAB_MINALIGN) in archive vmlinux.a
>>> defined at llvm-link
>>>            drivers/b

It seems like all of the const's in rust/bindings/bindings_helper.h
are failing to link, but I'm not sure why

Alistair

>
> The less boiler plate code we have to write by hand, the better!
>
>
> Best regards,
> Andreas Hindborg
>
>
>
Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Alistair Francis 1 year, 1 month ago
On Fri, Dec 20, 2024 at 12:55 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Dec 18, 2024 at 7:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> > "Alistair Francis" <alistair23@gmail.com> writes:
> >
> > > On Wed, Dec 4, 2024 at 11:14 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> > >>
> > >>
> > >> Hi Alistair,
> > >>
> > >> "Alistair Francis" <alistair@alistair23.me> writes:
> > >> >
> > >> > This series adds support for bindgen generating wrappers for inline statics and
> > >> > then converts the existing helper functions to this new method. This doesn't
> > >> > work for C macros, so we can't reamove all of the helper functions, but we
> > >> > can remove most.
> > >>
> > >> Are you aware of the helper LTO patches by Gary [1]? Can you tell if
> > >
> > > I was not, but that's exciting to see
> > >
> > >> this series will compose with the LTO patches?
> > >
> > > I think it will still work, although it might take some extra effort.
> > >
> > > I assume my series isn't going to be accepted though, so I'm not going
> > > to try and get the LTO series to work on top of it. It's a lot of work
> > > to rebase this series as every new binding causes a conflict and it
> > > seems to have stalled
> >
> > I would assume that it _will_ be merged in some form. Why not? It's just
> > a matter of getting it reviewed and making sure that it is going to work
> > with the LTO inlining.

I have my patches building with the LTO inlining work. The kernel
still boots and it seems like the inlining is working

You can see it here if you are interested in testing:
https://github.com/alistair23/linux/commit/e6b847324b4f5e904e007c0e288c88d2483928a8

Alistair
Re: [PATCH v4 00/11] rust: bindings: Auto-generate inline static functions
Posted by Alistair Francis 1 year, 2 months ago
On Thu, Nov 14, 2024 at 9:56 AM Alistair Francis <alistair@alistair23.me> wrote:
>
> The kernel includes a large number of static inline functions that are
> defined in header files. One example is the crypto_shash_descsize()
> function which is defined in hash.h as
>
> ```
> static inline unsigned int crypto_shash_descsize(struct crypto_shash *tfm)
> {
>         return tfm->descsize;
> }
> ```
>
> bindgen is currently unable to generate bindings to these functions as
> they are not publically exposed (they are static after all).
>
> The Rust code currently uses rust_helper_* functions, such as
> rust_helper_alloc_pages() for example to call the static inline
> functions. But this is a hassle as someone needs to write a C helper
> function.
>
> Instead we can use the bindgen wrap-static-fns feature. The feature
> is marked as experimental, but has recently been promoted to
> non-experimental (depending on your version of bindgen).
>
> By supporting wrap-static-fns we automatically generate a C file called
> extern.c that exposes the static inline functions, for example like this
>
> ```
> unsigned int crypto_shash_descsize__extern(struct crypto_shash *tfm) { return crypto_shash_descsize(tfm); }
> ```
>
> The nice part is that this is auto-generated.
>
> We then also get a bindings_generate_static.rs file with the Rust
> binding, like this
>
> ```
> extern "C" {
>     #[link_name = "crypto_shash_descsize__extern"]
>     pub fn crypto_shash_descsize(tfm: *mut crypto_shash) -> core::ffi::c_uint;
> }
> ```
>
> So now we can use the static inline functions just like normal
> functions.
>
> There are a bunch of static inline functions that don't work though, because
> the C compiler fails to build extern.c:
>  * functions with inline asm generate "operand probably does not match constraints"
>    errors (rip_rel_ptr() for example)
>  * functions with bit masks (u32_encode_bits() and friends) result in
>    "call to ‘__bad_mask’ declared with attribute error: bad bitfield mask"
>    errors
>
> As well as that any static inline function that calls a function that has been
> kconfig-ed out will fail to link as the function being called isn't built
> (mdio45_ethtool_gset_npage for example)
>
> Due to these failures we use a allow-list system (where functions must
> be manually enabled).
>
> This series adds support for bindgen generating wrappers for inline statics and
> then converts the existing helper functions to this new method. This doesn't
> work for C macros, so we can't reamove all of the helper functions, but we
> can remove most.

Any more comments?

Alistair

>
> v4:
>  - Fix out of tree builds
> v3:
>  - Change SoB email address to match from address
>  - Fixup kunit test build failure
>  - Update Rust binding documentation
> v2:
>  - Fixup build failures report by test bots
>  - Rebase on rust-next (ae7851c29747fa376)
>
> Alistair Francis (11):
>   rust: bindings: Support some inline static functions
>   rust: helpers: Remove blk helper
>   rust: helpers: Remove err helper
>   rust: helpers: Remove kunit helper
>   rust: helpers: Remove some page helpers
>   rust: helpers: Remove rbtree helper
>   rust: helpers: Remove some refcount helpers
>   rust: helpers: Remove signal helper
>   rust: helpers: Remove some spinlock helpers
>   rust: helpers: Remove some task helpers
>   rust: helpers: Remove uaccess helpers
>
>  Documentation/rust/general-information.rst | 10 +++---
>  rust/.gitignore                            |  2 ++
>  rust/Makefile                              | 37 ++++++++++++++++++++--
>  rust/bindgen_static_functions              | 32 +++++++++++++++++++
>  rust/bindings/bindings_helper.h            |  6 ++++
>  rust/bindings/lib.rs                       |  4 +++
>  rust/exports.c                             |  1 +
>  rust/helpers/blk.c                         | 14 --------
>  rust/helpers/err.c                         | 18 -----------
>  rust/helpers/helpers.c                     | 11 ++-----
>  rust/helpers/kunit.c                       |  8 -----
>  rust/helpers/page.c                        |  5 ---
>  rust/helpers/rbtree.c                      |  9 ------
>  rust/helpers/refcount.c                    | 10 ------
>  rust/helpers/signal.c                      |  8 -----
>  rust/helpers/spinlock.c                    | 15 ---------
>  rust/helpers/task.c                        | 10 ------
>  rust/helpers/uaccess.c                     | 15 ---------
>  18 files changed, 87 insertions(+), 128 deletions(-)
>  create mode 100644 rust/bindgen_static_functions
>  delete mode 100644 rust/helpers/blk.c
>  delete mode 100644 rust/helpers/err.c
>  delete mode 100644 rust/helpers/kunit.c
>  delete mode 100644 rust/helpers/rbtree.c
>  delete mode 100644 rust/helpers/signal.c
>  delete mode 100644 rust/helpers/uaccess.c
>
> --
> 2.47.0
>