On Tue, Feb 06, 2024 at 10:58:45PM -0700, Thomas Bertschinger wrote:
> This patch uses the bcachefs bindgen framework to introduce a Rust
> implementation of the module entry and exit functions. With this change,
> bcachefs is now a Rust kernel module (that calls C functions to do most
> of its work).
>
> This is only if CONFIG_BCACHEFS_RUST is defined; the C implementation of
> the module init and exit code is left around so that bcachefs remains
> usable in kernels compiled without Rust support.
so the module_init in Rust is an interesting test case, but - not much
point in checking it in if we're not deleting code on the C side.
Instead, have you looked at pulling the btree transaction layer bindings
from -tools into the kernel? That was going to be my starting point;
once we've got that there's a lot of code we'll be able to rewrite in
Rust piecmeal (fsck, debugs, alloc_background.c - anything that
primarily interacts with the transaction layer).
>
> Signed-off-by: Thomas Bertschinger <tahbertschinger@xxxxxxxxx>
> ---
> fs/bcachefs/Makefile | 3 ++
> fs/bcachefs/bcachefs.h | 5 ++
> fs/bcachefs/bcachefs_module.rs | 66 ++++++++++++++++++++++++++
> fs/bcachefs/bindgen_parameters | 13 ++++-
> fs/bcachefs/bindings/bindings_helper.h | 4 ++
> fs/bcachefs/bindings/mod.rs | 2 +
> fs/bcachefs/super.c | 31 ++++++++++--
> 7 files changed, 120 insertions(+), 4 deletions(-)
> create mode 100644 fs/bcachefs/bcachefs_module.rs
>
> diff --git a/fs/bcachefs/Makefile b/fs/bcachefs/Makefile
> index 3f209511149c..252810a4d9a0 100644
> --- a/fs/bcachefs/Makefile
> +++ b/fs/bcachefs/Makefile
> @@ -89,8 +89,11 @@ bcachefs-y := \
> varint.o \
> xattr.o
>
> +bcachefs-$(CONFIG_BCACHEFS_RUST) += bcachefs_module.o
> always-$(CONFIG_BCACHEFS_RUST) += bindings/bcachefs_generated.rs
>
> +$(obj)/bcachefs_module.o: $(src)/bindings/bcachefs_generated.rs
> +
> $(obj)/bindings/bcachefs_generated.rs: private bindgen_target_flags = \
> $(shell grep -Ev '^#|^$$' $(srctree)/$(src)/bindgen_parameters)
>
> diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h
> index b80c6c9efd8c..3a777592bff4 100644
> --- a/fs/bcachefs/bcachefs.h
> +++ b/fs/bcachefs/bcachefs.h
> @@ -1252,4 +1252,9 @@ static inline struct stdio_redirect *bch2_fs_stdio_redirect(struct bch_fs *c)
> #define BKEY_PADDED_ONSTACK(key, pad) \
> struct { struct bkey_i key; __u64 key ## _pad[pad]; }
>
> +#ifdef CONFIG_BCACHEFS_RUST
> +int bch2_kset_init(void);
> +void bch2_kset_exit(void);
> +#endif
> +
> #endif /* _BCACHEFS_H */
> diff --git a/fs/bcachefs/bcachefs_module.rs b/fs/bcachefs/bcachefs_module.rs
> new file mode 100644
> index 000000000000..8db2de8139bc
> --- /dev/null
> +++ b/fs/bcachefs/bcachefs_module.rs
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! bcachefs
> +//!
> +//! Rust kernel module for bcachefs.
> +
> +pub mod bindings;
> +
> +use kernel::prelude::*;
> +
> +use crate::bindings::*;
> +
> +module! {
> + type: Bcachefs,
> + name: "bcachefs",
> + author: "Kent Overstreet <kent.overstreet@xxxxxxxxx>",
> + description: "bcachefs filesystem",
> + license: "GPL",
> +}
> +
> +struct Bcachefs;
> +
> +impl kernel::Module for Bcachefs {
> + #[link_section = ".init.text"]
> + fn init(_module: &'static ThisModule) -> Result<Self> {
> + // SAFETY: this block registers the bcachefs services with the kernel. After succesful
> + // registration, all such services are guaranteed by the kernel to exist as long as the
> + // driver is loaded. In the event of any failure in the registration, all registered
> + // services are unregistered.
> + unsafe {
> + bch2_bkey_pack_test();
> +
> + if bch2_kset_init() != 0
> + || bch2_btree_key_cache_init() != 0
> + || bch2_chardev_init() != 0
> + || bch2_vfs_init() != 0
> + || bch2_debug_init() != 0
> + {
> + __drop();
> + return Err(ENOMEM);
> + }
> + }
> +
> + Ok(Bcachefs)
> + }
> +}
> +
> +fn __drop() {
> + // SAFETY: The kernel does not allow cleanup_module() (which results in
> + // drop()) to be called unless there are no users of the filesystem.
> + // The *_exit() functions only free data that they confirm is allocated, so
> + // this is safe to call even if the module's init() function did not finish.
> + unsafe {
> + bch2_debug_exit();
> + bch2_vfs_exit();
> + bch2_chardev_exit();
> + bch2_btree_key_cache_exit();
> + bch2_kset_exit();
> + }
> +}
> +
> +impl Drop for Bcachefs {
> + fn drop(&mut self) {
> + __drop();
> + }
> +}
> diff --git a/fs/bcachefs/bindgen_parameters b/fs/bcachefs/bindgen_parameters
> index 547212bebd6e..96a63e3a2cc3 100644
> --- a/fs/bcachefs/bindgen_parameters
> +++ b/fs/bcachefs/bindgen_parameters
> @@ -1,5 +1,16 @@
> # SPDX-License-Identifier: GPL-2.0
>
> ---allowlist-function ''
> +--allowlist-function bch2_bkey_pack_test
> +--allowlist-function bch2_kset_init
> +--allowlist-function bch2_btree_key_cache_init
> +--allowlist-function bch2_chardev_init
> +--allowlist-function bch2_vfs_init
> +--allowlist-function bch2_debug_init
> +--allowlist-function bch2_debug_exit
> +--allowlist-function bch2_vfs_exit
> +--allowlist-function bch2_chardev_exit
> +--allowlist-function bch2_btree_key_cache_exit
> +--allowlist-function bch2_kset_exit
> +
> --allowlist-type ''
> --allowlist-var ''
> diff --git a/fs/bcachefs/bindings/bindings_helper.h b/fs/bcachefs/bindings/bindings_helper.h
> index f8bef3676f71..8cf3c35e8ca1 100644
> --- a/fs/bcachefs/bindings/bindings_helper.h
> +++ b/fs/bcachefs/bindings/bindings_helper.h
> @@ -1,3 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
>
> #include "../bcachefs.h"
> +#include "../btree_key_cache.h"
> +#include "../chardev.h"
> +#include "../fs.h"
> +#include "../debug.h"
> diff --git a/fs/bcachefs/bindings/mod.rs b/fs/bcachefs/bindings/mod.rs
> index 19a3ae3c63c6..d1c3bbbd7b5a 100644
> --- a/fs/bcachefs/bindings/mod.rs
> +++ b/fs/bcachefs/bindings/mod.rs
> @@ -1,3 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#![allow(missing_docs)]
> +
> include!("bcachefs_generated.rs");
> diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
> index da8697c79a97..343c4bc6e81c 100644
> --- a/fs/bcachefs/super.c
> +++ b/fs/bcachefs/super.c
> @@ -69,9 +69,12 @@
> #include <linux/sysfs.h>
> #include <crypto/hash.h>
>
> +#ifndef CONFIG_BCACHEFS_RUST
> +/* when enabled, the Rust module exports these modinfo attributes */
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Kent Overstreet <kent.overstreet@xxxxxxxxx>");
> MODULE_DESCRIPTION("bcachefs filesystem");
> +#endif
> MODULE_SOFTDEP("pre: crc32c");
> MODULE_SOFTDEP("pre: crc64");
> MODULE_SOFTDEP("pre: sha256");
> @@ -2082,6 +2085,7 @@ struct bch_fs *bch2_fs_open(char * const *devices, unsigned nr_devices,
>
> /* Global interfaces/init */
>
> +#ifndef CONFIG_BCACHEFS_RUST
> static void bcachefs_exit(void)
> {
> bch2_debug_exit();
> @@ -2109,6 +2113,30 @@ static int __init bcachefs_init(void)
> return -ENOMEM;
> }
>
> +module_exit(bcachefs_exit);
> +module_init(bcachefs_init);
> +
> +#else /* CONFIG_BCACHEFS_RUST */
> +/*
> + * bch2_kset_init() and bch2_kset_exit() are wrappers around the kset functions
> + * to be called from the Rust module init and exit because there is not
> + * currently a Rust API for ksets. If/when a Rust API is provided, these
> + * wrappers can be removed and the Rust kernel module can use that directly.
> + */
> +int __init bch2_kset_init(void)
> +{
> + bcachefs_kset = kset_create_and_add("bcachefs", NULL, fs_kobj);
> +
> + return !bcachefs_kset;
> +}
> +
> +void bch2_kset_exit(void)
> +{
> + if (bcachefs_kset)
> + kset_unregister(bcachefs_kset);
> +}
> +#endif
> +
> #define BCH_DEBUG_PARAM(name, description) \
> bool bch2_##name; \
> module_param_named(name, bch2_##name, bool, 0644); \
> @@ -2119,6 +2147,3 @@ BCH_DEBUG_PARAMS()
> __maybe_unused
> static unsigned bch2_metadata_version = bcachefs_metadata_version_current;
> module_param_named(version, bch2_metadata_version, uint, 0400);
> -
> -module_exit(bcachefs_exit);
> -module_init(bcachefs_init);
> --
> 2.43.0
>
