[PATCH v6 0/3] rust: configfs abstractions

[PATCH v6 0/3] rust: configfs abstractions

[Andreas Hindborg]

Add a safe Rust API that allows Rust modules to interface the `configfs`
machinery.

Add an example for the samples folder to demonstrate usage of the API.

Add a maintainer entry for the Rust configfs abstractions in the last patch, to
make it absolutely clear that I will commit to maintain these abstractions, if
required.

The series is a dependency of `rnull`, the Rust null block driver.
Please see [1] for initial `configfs` support in `rnull`.

[1] https://github.com/metaspace/linux/tree/9ac53130f5fb05b9b3074fa261b445b8fde547dd/drivers/block/rnull

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v6:
- Use `&raw const` and `&raw mut` instead of `addr_of!` and
  `addr_of_mut!` macros.
- Drop use of `ForeignOwnable`.
- Link to v5: https://lore.kernel.org/r/20250227-configfs-v5-0-c40e8dc3b9cd@kernel.org

Changes in v5:
- Remove `as _` casts.
- Document `ID` type parameter of `AttributeOperations`.
- Add documentation at macro call sites in example.
- Add example expansion of `configfs_attrs!`.
- Move trait bound in `AttributeList::add`
- Improve safety requirement for `AttributeList::new`.
- Fix a copy/paste error in print in sample.
- Clarify use of `{}` for empty struct in sample.
- Improve documentation for `AttributeList`.
- Remove `kernel::` prefix from `container_of!` invocation.
- Reword safety comment in `get_group_data`.
- Correct commit message in relation to unstable feature additions.
- Use imperative language in commit messages.
- Consistently capitalize the word "Rust" in commit messages.
- Explain that "drop" in `GroupOperations::drop_item` is not related to Rust
  drop.
- Link to v4: https://lore.kernel.org/r/20250224-configfs-v4-0-9af9b5e611f6@kernel.org

Changes in v4:
- Fix a build issue by depending on v18 of "rust: types: add `ForeignOwnable::PointedTo`"
- Link to v3: https://lore.kernel.org/r/20250218-configfs-v3-0-0e40c0778187@kernel.org

Changes in v3:
- Allow trailing commas in invocation of `configfs_attrs!`.
- Use a more suitable C initialization function when initializing `Subsystem`.
- Split sample into separate patch.
- Add an inline example.

The remaining changes in this version are style fixes, documentation
improvements and typo fixes. They are enumerated below:
- Consolidate `paste` macro calls.
- Do not hard code page size in example.
- Remove prefix of `c_str!` in sample.
- Use a more descriptive variable name in `into_foreign`.
- Improve code formatting in macros invocations.
- Add comment related to null terminator in `configfs_attrs!`
- Move attributes below docstrings.
- Remove a rogue todo.
- Remove trait bound from struct definition `GroupOperationsVTable`.
- Remove `as _` casts.
- Remove `GroupOprations::Parent` associated type.
- General documentation improvements.
- Explicitly use `ArcBorrow` for `drop_item` parameter type.
- Add a comment describing expansion to a call to `Attribute::add`.
- Add a comment explaining bound check in `Attribute::add`.
- Link to v2: https://lore.kernel.org/r/20250207-configfs-v2-0-f7a60b24d38e@kernel.org

Changes in v2:
- Remove generalization over pointer type and enforce use of `Arc`.
- Use type system to enforce connection between `ItemType` and
  `Subsystem` or `Group`. Differentiate construction of vtables on this
  type difference.
- Move drop logic of child nodes from parent to child.
- Pick `ForeignOwnable::PointedTo` patch as dependency instead of
  including it here.
- Fix some rustdoc warnings.
- Use CamelCase for generic type parameter declaration.
- Destroy mutex in `Subsystem::drop`.
- Move `GroupOperationsVTable` struct definition next to implementation.
- Rebase on v6.14-rc1.
- Link to v1: https://lore.kernel.org/r/20250131-configfs-v1-0-87947611401c@kernel.org

---
Andreas Hindborg (3):
      rust: configfs: introduce rust support for configfs
      rust: configfs: add a sample demonstrating configfs usage
      MAINTAINERS: add configfs Rust abstractions

 MAINTAINERS                     |    2 +
 rust/bindings/bindings_helper.h |    1 +
 rust/helpers/mutex.c            |    5 +
 rust/kernel/configfs.rs         | 1048 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    2 +
 samples/rust/Kconfig            |   11 +
 samples/rust/Makefile           |    1 +
 samples/rust/rust_configfs.rs   |  192 +++++++
 8 files changed, 1262 insertions(+)
---
base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e
change-id: 20250131-configfs-b888cd82d84a

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

[PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

Add a Rust API for configfs, thus allowing Rust modules to use configfs for
configuration. Make the implementation a shim on top of the C configfs
implementation, allowing safe use of the C infrastructure from Rust.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

---

This patch is a direct dependency for `rnull`, the rust null block driver.
---
 rust/bindings/bindings_helper.h |    1 +
 rust/helpers/mutex.c            |    5 +
 rust/kernel/configfs.rs         | 1048 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    2 +
 4 files changed, 1056 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ab37e1d35c70..1a532b83a9af 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -10,6 +10,7 @@
 #include <linux/blk-mq.h>
 #include <linux/blk_types.h>
 #include <linux/blkdev.h>
+#include <linux/configfs.h>
 #include <linux/cpumask.h>
 #include <linux/cred.h>
 #include <linux/device/faux.h>
diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c
index 06575553eda5..3e9b910a88e9 100644
--- a/rust/helpers/mutex.c
+++ b/rust/helpers/mutex.c
@@ -17,3 +17,8 @@ void rust_helper_mutex_assert_is_held(struct mutex *mutex)
 {
 	lockdep_assert_held(mutex);
 }
+
+void rust_helper_mutex_destroy(struct mutex *lock)
+{
+	mutex_destroy(lock);
+}
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
new file mode 100644
index 000000000000..e338cbf94239
--- /dev/null
+++ b/rust/kernel/configfs.rs
@@ -0,0 +1,1048 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! `configfs` interface.
+//!
+//! `configfs` is an in-memory pseudo file system for configuration of kernel
+//! modules. Please see the [C documentation] for details and intended use of
+//! `configfs`.
+//!
+//! This module does not support the following `configfs` features:
+//!
+//! - Items. All group children are groups.
+//! - Symlink support.
+//! - `disconnect_notify` hook.
+//! - Default groups.
+//!
+//! See the [rust_configfs.rs] sample for a full example use of this module.
+//!
+//! C header: [`include/linux/configfs.h`](srctree/include/linux/configfs.h)
+//!
+//! # Example
+//!
+//! ```ignore
+//! use kernel::alloc::flags;
+//! use kernel::c_str;
+//! use kernel::configfs_attrs;
+//! use kernel::configfs;
+//! use kernel::new_mutex;
+//! use kernel::page::PAGE_SIZE;
+//! use kernel::sync::Mutex;
+//! use kernel::ThisModule;
+//!
+//! #[pin_data]
+//! struct RustConfigfs {
+//!     #[pin]
+//!     config: configfs::Subsystem<Configuration>,
+//! }
+//!
+//! impl kernel::InPlaceModule for RustConfigfs {
+//!     fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+//!         pr_info!("Rust configfs sample (init)\n");
+//!
+//!         let item_type = configfs_attrs! {
+//!             container: configfs::Subsystem<Configuration>,
+//!             data: Configuration,
+//!             attributes: [
+//!                 message: 0,
+//!                 bar: 1,
+//!             ],
+//!         };
+//!
+//!         try_pin_init!(Self {
+//!             config <- configfs::Subsystem::new(
+//!                 c_str!("rust_configfs"), item_type, Configuration::new()
+//!             ),
+//!         })
+//!     }
+//! }
+//!
+//! #[pin_data]
+//! struct Configuration {
+//!     message: &'static CStr,
+//!     #[pin]
+//!     bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
+//! }
+//!
+//! impl Configuration {
+//!     fn new() -> impl PinInit<Self, Error> {
+//!         try_pin_init!(Self {
+//!             message: c_str!("Hello World\n"),
+//!             bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
+//!         })
+//!     }
+//! }
+//!
+//! #[vtable]
+//! impl configfs::AttributeOperations<0> for Configuration {
+//!     type Data = Configuration;
+//!
+//!     fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+//!         pr_info!("Show message\n");
+//!         let data = container.message;
+//!         page[0..data.len()].copy_from_slice(data);
+//!         Ok(data.len())
+//!     }
+//! }
+//!
+//! #[vtable]
+//! impl configfs::AttributeOperations<1> for Configuration {
+//!     type Data = Configuration;
+//!
+//!     fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+//!         pr_info!("Show bar\n");
+//!         let guard = container.bar.lock();
+//!         let data = guard.0.as_slice();
+//!         let len = guard.1;
+//!         page[0..len].copy_from_slice(&data[0..len]);
+//!         Ok(len)
+//!     }
+//!
+//!     fn store(container: &Configuration, page: &[u8]) -> Result {
+//!         pr_info!("Store bar\n");
+//!         let mut guard = container.bar.lock();
+//!         guard.0[0..page.len()].copy_from_slice(page);
+//!         guard.1 = page.len();
+//!         Ok(())
+//!     }
+//! }
+//! ```
+//!
+//! [C documentation]: srctree/Documentation/filesystems/configfs.rst
+//! [rust_configfs.rs]: srctree/samples/rust/rust_configfs.rs
+
+use crate::alloc::flags;
+use crate::container_of;
+use crate::page::PAGE_SIZE;
+use crate::prelude::*;
+use crate::str::CString;
+use crate::sync::Arc;
+use crate::sync::ArcBorrow;
+use crate::types::Opaque;
+use core::cell::UnsafeCell;
+use core::marker::PhantomData;
+
+/// A `configfs` subsystem.
+///
+/// This is the top level entrypoint for a `configfs` hierarchy. To register
+/// with configfs, embed a field of this type into your kernel module struct.
+#[pin_data(PinnedDrop)]
+pub struct Subsystem<Data> {
+    #[pin]
+    subsystem: Opaque<bindings::configfs_subsystem>,
+    #[pin]
+    data: Data,
+}
+
+// SAFETY: We do not provide any operations on `Subsystem`.
+unsafe impl<Data> Sync for Subsystem<Data> {}
+
+// SAFETY: Ownership of `Subsystem` can safely be transferred to other threads.
+unsafe impl<Data> Send for Subsystem<Data> {}
+
+impl<Data> Subsystem<Data> {
+    /// Create an initializer for a [`Subsystem`].
+    ///
+    /// The subsystem will appear in configfs as a directory name given by
+    /// `name`. The attributes available in directory are specified by
+    /// `item_type`.
+    pub fn new(
+        name: &'static CStr,
+        item_type: &'static ItemType<Subsystem<Data>, Data>,
+        data: impl PinInit<Data, Error>,
+    ) -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {
+            subsystem <- pin_init::zeroed().chain(
+                |place: &mut Opaque<bindings::configfs_subsystem>| {
+                    // SAFETY: We initialized the required fields of `place.group` above.
+                    unsafe {
+                        bindings::config_group_init_type_name(
+                            &mut (*place.get()).su_group,
+                            name.as_ptr(),
+                            item_type.as_ptr(),
+                        )
+                    };
+
+                    // SAFETY: `place.su_mutex` is valid for use as a mutex.
+                    unsafe {
+                        bindings::__mutex_init(
+                            &mut (*place.get()).su_mutex,
+                            kernel::optional_name!().as_char_ptr(),
+                            kernel::static_lock_class!().as_ptr(),
+                        )
+                    }
+                    Ok(())
+                }
+            ),
+            data <- data,
+        })
+        .pin_chain(|this| {
+            crate::error::to_result(
+                // SAFETY: We initialized `this.subsystem` according to C API contract above.
+                unsafe { bindings::configfs_register_subsystem(this.subsystem.get()) },
+            )
+        })
+    }
+}
+
+#[pinned_drop]
+impl<Data> PinnedDrop for Subsystem<Data> {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: We registered `self.subsystem` in the initializer returned by `Self::new`.
+        unsafe { bindings::configfs_unregister_subsystem(self.subsystem.get()) };
+        // SAFETY: We initialized the mutex in `Subsystem::new`.
+        unsafe { bindings::mutex_destroy(&raw mut (*self.subsystem.get()).su_mutex) };
+    }
+}
+
+/// Trait that allows offset calculations for structs that embed a
+/// `bindings::config_group`.
+///
+/// Users of the `configfs` API should not need to implement this trait.
+///
+/// # Safety
+///
+/// - Implementers of this trait must embed a `bindings::config_group`.
+/// - Methods must be implemented according to method documentation.
+pub unsafe trait HasGroup<Data> {
+    /// Return the address of the `bindings::config_group` embedded in `Self`.
+    ///
+    /// # Safety
+    ///
+    /// - `this` must be a valid allocation of at least the size of `Self`.
+    unsafe fn group(this: *const Self) -> *const bindings::config_group;
+
+    /// Return the address of the `Self` that `group` is embedded in.
+    ///
+    /// # Safety
+    ///
+    /// - `group` must point to the `bindings::config_group` that is embedded in
+    ///   `Self`.
+    unsafe fn container_of(group: *const bindings::config_group) -> *const Self;
+}
+
+// SAFETY: `Subsystem<Data>` embeds a field of type `bindings::config_group`
+// within the `subsystem` field.
+unsafe impl<Data> HasGroup<Data> for Subsystem<Data> {
+    unsafe fn group(this: *const Self) -> *const bindings::config_group {
+        // SAFETY: By impl and function safety requirement this projection is in bounds.
+        unsafe { &raw const (*(*this).subsystem.get()).su_group }
+    }
+
+    unsafe fn container_of(group: *const bindings::config_group) -> *const Self {
+        // SAFETY: By impl and function safety requirement this projection is in bounds.
+        let c_subsys_ptr = unsafe { container_of!(group, bindings::configfs_subsystem, su_group) };
+        let opaque_ptr = c_subsys_ptr.cast::<Opaque<bindings::configfs_subsystem>>();
+        // SAFETY: By impl and function safety requirement, `opaque_ptr` and the
+        // pointer it returns, are within the same allocation.
+        unsafe { container_of!(opaque_ptr, Subsystem<Data>, subsystem) }
+    }
+}
+
+/// A `configfs` group.
+///
+/// To add a subgroup to `configfs`, pass this type as `ctype` to
+/// [`crate::configfs_attrs`] when creating a group in [`GroupOperations::make_group`].
+#[pin_data]
+pub struct Group<Data> {
+    #[pin]
+    group: Opaque<bindings::config_group>,
+    #[pin]
+    data: Data,
+}
+
+impl<Data> Group<Data> {
+    /// Create an initializer for a new group.
+    ///
+    /// When instantiated, the group will appear as a directory with the name
+    /// given by `name` and it will contain attributes specified by `item_type`.
+    pub fn new(
+        name: CString,
+        item_type: &'static ItemType<Group<Data>, Data>,
+        data: impl PinInit<Data, Error>,
+    ) -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {
+            group <- pin_init::zeroed().chain(|v: &mut Opaque<bindings::config_group>| {
+                let place = v.get();
+                let name = name.as_bytes_with_nul().as_ptr();
+                // SAFETY: It is safe to initialize a group once it has been zeroed.
+                unsafe {
+                    bindings::config_group_init_type_name(place, name.cast(), item_type.as_ptr())
+                };
+                Ok(())
+            }),
+            data <- data,
+        })
+    }
+}
+
+// SAFETY: `Group<Data>` embeds a field of type `bindings::config_group`
+// within the `group` field.
+unsafe impl<Data> HasGroup<Data> for Group<Data> {
+    unsafe fn group(this: *const Self) -> *const bindings::config_group {
+        Opaque::raw_get(
+            // SAFETY: By impl and function safety requirements this field
+            // projection is within bounds of the allocation.
+            unsafe { &raw const (*this).group },
+        )
+    }
+
+    unsafe fn container_of(group: *const bindings::config_group) -> *const Self {
+        let opaque_ptr = group.cast::<Opaque<bindings::config_group>>();
+        // SAFETY: By impl and function safety requirement, `opaque_ptr` and
+        // pointer it returns will be in the same allocation.
+        unsafe { container_of!(opaque_ptr, Self, group) }
+    }
+}
+
+/// # Safety
+///
+/// `this` must be a valid pointer.
+///
+/// If `this` does not represent the root group of a `configfs` subsystem,
+/// `this` must be a pointer to a `bindings::config_group` embedded in a
+/// `Group<Parent>`.
+///
+/// Otherwise, `this` must be a pointer to a `bindings::config_group` that
+/// is embedded in a `bindings::configfs_subsystem` that is embedded in a
+/// `Subsystem<Parent>`.
+unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent {
+    // SAFETY: `this` is a valid pointer.
+    let is_root = unsafe { (*this).cg_subsys.is_null() };
+
+    if !is_root {
+        // SAFETY: By C API contact,`this` was returned from a call to
+        // `make_group`. The pointer is known to be embedded within a
+        // `Group<Parent>`.
+        unsafe { &(*Group::<Parent>::container_of(this)).data }
+    } else {
+        // SAFETY: By C API contract, `this` is a pointer to the
+        // `bindings::config_group` field within a `Subsystem<Parent>`.
+        unsafe { &(*Subsystem::container_of(this)).data }
+    }
+}
+
+struct GroupOperationsVTable<Parent, Child>(PhantomData<(Parent, Child)>);
+
+impl<Parent, Child> GroupOperationsVTable<Parent, Child>
+where
+    Parent: GroupOperations<Child = Child>,
+    Child: 'static,
+{
+    /// # Safety
+    ///
+    /// `this` must be a valid pointer.
+    ///
+    /// If `this` does not represent the root group of a `configfs` subsystem,
+    /// `this` must be a pointer to a `bindings::config_group` embedded in a
+    /// `Group<Parent>`.
+    ///
+    /// Otherwise, `this` must be a pointer to a `bindings::config_group` that
+    /// is embedded in a `bindings::configfs_subsystem` that is embedded in a
+    /// `Subsystem<Parent>`.
+    ///
+    /// `name` must point to a null terminated string.
+    unsafe extern "C" fn make_group(
+        this: *mut bindings::config_group,
+        name: *const kernel::ffi::c_char,
+    ) -> *mut bindings::config_group {
+        // SAFETY: By function safety requirements of this function, this call
+        // is safe.
+        let parent_data = unsafe { get_group_data(this) };
+
+        let group_init = match Parent::make_group(
+            parent_data,
+            // SAFETY: By function safety requirements, name points to a null
+            // terminated string.
+            unsafe { CStr::from_char_ptr(name) },
+        ) {
+            Ok(init) => init,
+            Err(e) => return e.to_ptr(),
+        };
+
+        let child_group = <Arc<Group<Child>> as InPlaceInit<Group<Child>>>::try_pin_init(
+            group_init,
+            flags::GFP_KERNEL,
+        );
+
+        match child_group {
+            Ok(child_group) => {
+                let child_group_ptr = child_group.into_raw();
+                // SAFETY: We allocated the pointee of `child_ptr` above as a
+                // `Group<Child>`.
+                unsafe { Group::<Child>::group(child_group_ptr) }.cast_mut()
+            }
+            Err(e) => e.to_ptr(),
+        }
+    }
+
+    /// # Safety
+    ///
+    /// If `this` does not represent the root group of a `configfs` subsystem,
+    /// `this` must be a pointer to a `bindings::config_group` embedded in a
+    /// `Group<Parent>`.
+    ///
+    /// Otherwise, `this` must be a pointer to a `bindings::config_group` that
+    /// is embedded in a `bindings::configfs_subsystem` that is embedded in a
+    /// `Subsystem<Parent>`.
+    ///
+    /// `item` must point to a `bindings::config_item` within a
+    /// `bindings::config_group` within a `Group<Child>`.
+    unsafe extern "C" fn drop_item(
+        this: *mut bindings::config_group,
+        item: *mut bindings::config_item,
+    ) {
+        // SAFETY: By function safety requirements of this function, this call
+        // is safe.
+        let parent_data = unsafe { get_group_data(this) };
+
+        // SAFETY: By function safety requirements, `item` is embedded in a
+        // `config_group`.
+        let c_child_group_ptr = unsafe { container_of!(item, bindings::config_group, cg_item) };
+        // SAFETY: By function safety requirements, `c_child_group_ptr` is
+        // embedded within a `Group<Child>`.
+        let r_child_group_ptr = unsafe { Group::<Child>::container_of(c_child_group_ptr) };
+
+        if Parent::HAS_DROP_ITEM {
+            // SAFETY: We called `into_raw` to produce `r_child_group_ptr` in
+            // `make_group`.
+            let arc: Arc<Group<Child>> = unsafe { Arc::from_raw(r_child_group_ptr.cast_mut()) };
+
+            Parent::drop_item(parent_data, arc.as_arc_borrow());
+            arc.into_raw();
+        }
+
+        // SAFETY: By C API contract, we are required to drop a refcount on
+        // `item`.
+        unsafe { bindings::config_item_put(item) };
+    }
+
+    const VTABLE: bindings::configfs_group_operations = bindings::configfs_group_operations {
+        make_item: None,
+        make_group: Some(Self::make_group),
+        disconnect_notify: None,
+        drop_item: Some(Self::drop_item),
+        is_visible: None,
+        is_bin_visible: None,
+    };
+
+    const fn vtable_ptr() -> *const bindings::configfs_group_operations {
+        &Self::VTABLE as *const bindings::configfs_group_operations
+    }
+}
+
+struct ItemOperationsVTable<Container, Data>(PhantomData<(Container, Data)>);
+
+impl<Data> ItemOperationsVTable<Group<Data>, Data>
+where
+    Data: 'static,
+{
+    /// # Safety
+    ///
+    /// `this` must be a pointer to a `bindings::config_group` embedded in a
+    /// `Group<Parent>`.
+    ///
+    /// This function will destroy the pointee of `this`. The pointee of `this`
+    /// must not be accessed after the function returns.
+    unsafe extern "C" fn release(this: *mut bindings::config_item) {
+        // SAFETY: By function safety requirements, `this` is embedded in a
+        // `config_group`.
+        let c_group_ptr = unsafe { kernel::container_of!(this, bindings::config_group, cg_item) };
+        // SAFETY: By function safety requirements, `c_group_ptr` is
+        // embedded within a `Group<Data>`.
+        let r_group_ptr = unsafe { Group::<Data>::container_of(c_group_ptr) };
+
+        // SAFETY: We called `into_raw` on `r_group_ptr` in
+        // `make_group`.
+        let pin_self: Arc<Group<Data>> = unsafe { Arc::from_raw(r_group_ptr.cast_mut()) };
+        drop(pin_self);
+    }
+
+    const VTABLE: bindings::configfs_item_operations = bindings::configfs_item_operations {
+        release: Some(Self::release),
+        allow_link: None,
+        drop_link: None,
+    };
+
+    const fn vtable_ptr() -> *const bindings::configfs_item_operations {
+        &Self::VTABLE as *const bindings::configfs_item_operations
+    }
+}
+
+impl<Data> ItemOperationsVTable<Subsystem<Data>, Data> {
+    const VTABLE: bindings::configfs_item_operations = bindings::configfs_item_operations {
+        release: None,
+        allow_link: None,
+        drop_link: None,
+    };
+
+    const fn vtable_ptr() -> *const bindings::configfs_item_operations {
+        &Self::VTABLE as *const bindings::configfs_item_operations
+    }
+}
+
+/// Operations implemented by `configfs` groups that can create subgroups.
+///
+/// Implement this trait on structs that embed a [`Subsystem`] or a [`Group`].
+#[vtable]
+pub trait GroupOperations {
+    /// The child data object type.
+    ///
+    /// This group will create subgroups (subdirectories) backed by this kind of
+    /// object.
+    type Child: 'static;
+
+    /// Creates a new subgroup.
+    ///
+    /// The kernel will call this method in response to `mkdir(2)` in the
+    /// directory representing `this`.
+    ///
+    /// To accept the request to create a group, implementations should
+    /// return an initializer of a `Group<Self::Child>`. To prevent creation,
+    /// return a suitable error.
+    fn make_group(&self, name: &CStr) -> Result<impl PinInit<Group<Self::Child>, Error>>;
+
+    /// Prepares the group for removal from configfs.
+    ///
+    /// The kernel will call this method before the directory representing `_child` is removed from
+    /// `configfs`.
+    ///
+    /// Implementations can use this method to do house keeping before `configfs` drops its
+    /// reference to `Child`.
+    ///
+    /// NOTE: "drop" in the name of this function is not related to the Rust drop term. Rather, the
+    /// name is inherited from the callback name in the underlying C code.
+    fn drop_item(&self, _child: ArcBorrow<'_, Group<Self::Child>>) {
+        kernel::build_error!(kernel::error::VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// A `configfs` attribute.
+///
+/// An attribute appears as a file in configfs, inside a folder that represent
+/// the group that the attribute belongs to.
+#[repr(transparent)]
+pub struct Attribute<const ID: u64, O, Data> {
+    attribute: Opaque<bindings::configfs_attribute>,
+    _p: PhantomData<(O, Data)>,
+}
+
+// SAFETY: We do not provide any operations on `Attribute`.
+unsafe impl<const ID: u64, O, Data> Sync for Attribute<ID, O, Data> {}
+
+// SAFETY: Ownership of `Attribute` can safely be transferred to other threads.
+unsafe impl<const ID: u64, O, Data> Send for Attribute<ID, O, Data> {}
+
+impl<const ID: u64, O, Data> Attribute<ID, O, Data>
+where
+    O: AttributeOperations<ID, Data = Data>,
+{
+    /// # Safety
+    ///
+    /// `item` must be embedded in a `bindings::config_group`.
+    ///
+    /// If `item` does not represent the root group of a `configfs` subsystem,
+    /// the group must be embedded in a `Group<Data>`.
+    ///
+    /// Otherwise, the group must be a embedded in a
+    /// `bindings::configfs_subsystem` that is embedded in a `Subsystem<Data>`.
+    ///
+    /// `page` must point to a writable buffer of size at least [`PAGE_SIZE`].
+    unsafe extern "C" fn show(
+        item: *mut bindings::config_item,
+        page: *mut kernel::ffi::c_char,
+    ) -> isize {
+        let c_group: *mut bindings::config_group =
+        // SAFETY: By function safety requirements, `item` is embedded in a
+        // `config_group`.
+            unsafe { container_of!(item, bindings::config_group, cg_item) }.cast_mut();
+
+        // SAFETY: The function safety requirements for this function satisfy
+        // the conditions for this call.
+        let data: &Data = unsafe { get_group_data(c_group) };
+
+        // SAFETY: By function safety requirements, `page` is writable for `PAGE_SIZE`.
+        let ret = O::show(data, unsafe { &mut *(page as *mut [u8; PAGE_SIZE]) });
+
+        match ret {
+            Ok(size) => size as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `item` must be embedded in a `bindings::config_group`.
+    ///
+    /// If `item` does not represent the root group of a `configfs` subsystem,
+    /// the group must be embedded in a `Group<Data>`.
+    ///
+    /// Otherwise, the group must be a embedded in a
+    /// `bindings::configfs_subsystem` that is embedded in a `Subsystem<Data>`.
+    ///
+    /// `page` must point to a readable buffer of size at least `size`.
+    unsafe extern "C" fn store(
+        item: *mut bindings::config_item,
+        page: *const kernel::ffi::c_char,
+        size: usize,
+    ) -> isize {
+        let c_group: *mut bindings::config_group =
+        // SAFETY: By function safety requirements, `item` is embedded in a
+        // `config_group`.
+            unsafe { container_of!(item, bindings::config_group, cg_item) }.cast_mut();
+
+        // SAFETY: The function safety requirements for this function satisfy
+        // the conditions for this call.
+        let data: &Data = unsafe { get_group_data(c_group) };
+
+        let ret = O::store(
+            data,
+            // SAFETY: By function safety requirements, `page` is readable
+            // for at least `size`.
+            unsafe { core::slice::from_raw_parts(page.cast(), size) },
+        );
+
+        match ret {
+            Ok(()) => size as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
+    /// Create a new attribute.
+    ///
+    /// The attribute will appear as a file with name given by `name`.
+    pub const fn new(name: &'static CStr) -> Self {
+        Self {
+            attribute: Opaque::new(bindings::configfs_attribute {
+                ca_name: name.as_char_ptr(),
+                ca_owner: core::ptr::null_mut(),
+                ca_mode: 0o660,
+                show: Some(Self::show),
+                store: if O::HAS_STORE {
+                    Some(Self::store)
+                } else {
+                    None
+                },
+            }),
+            _p: PhantomData,
+        }
+    }
+}
+
+/// Operations supported by an attribute.
+///
+/// Implement this trait on type and pass that type as generic parameter when
+/// creating an [`Attribute`]. The type carrying the implementation serve no
+/// purpose other than specifying the attribute operations.
+///
+/// This trait must be implemented on the `Data` type of for types that
+/// implement `HasGroup<Data>`. The trait must be implemented once for each
+/// attribute of the group. The constant type parameter `ID` maps the
+/// implementation to a specific `Attribute`. `ID` must be passed when declaring
+/// attributes via the `configfs_attrs!` macro, to tie `AttributeOperations`
+/// implementations to concrete named attributes.
+#[vtable]
+pub trait AttributeOperations<const ID: u64 = 0> {
+    /// The type of the object that contains the field that is backing the
+    /// attribute for this operation.
+    type Data;
+
+    /// Renders the value of an attribute.
+    ///
+    /// This function is called by the kernel to read the value of an attribute.
+    ///
+    /// Implementations should write the rendering of the attribute to `page`
+    /// and return the number of bytes written.
+    fn show(data: &Self::Data, page: &mut [u8; PAGE_SIZE]) -> Result<usize>;
+
+    /// Stores the value of an attribute.
+    ///
+    /// This function is called by the kernel to update the value of an attribute.
+    ///
+    /// Implementations should parse the value from `page` and update internal
+    /// state to reflect the parsed value.
+    fn store(_data: &Self::Data, _page: &[u8]) -> Result {
+        kernel::build_error!(kernel::error::VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// A list of attributes.
+///
+/// This type is used to construct a new [`ItemType`]. It represents a list of
+/// [`Attribute`] that will appear in the directory representing a [`Group`].
+/// Users should not directly instantiate this type, rather they should use the
+/// [`kernel::configfs_attrs`] macro to declare a static set of attributes for a
+/// group.
+///
+/// # Note
+///
+/// This type is constructed statically at compile time and is by the
+/// [`kernel::configfs_attrs`] macro.
+#[repr(transparent)]
+pub struct AttributeList<const N: usize, Data>(
+    /// Null terminated Array of pointers to `Attribute`. The type is `c_void`
+    /// to conform to the C API.
+    UnsafeCell<[*mut kernel::ffi::c_void; N]>,
+    PhantomData<Data>,
+);
+
+// SAFETY: Ownership of `AttributeList` can safely be transferred to other threads.
+unsafe impl<const N: usize, Data> Send for AttributeList<N, Data> {}
+
+// SAFETY: We do not provide any operations on `AttributeList` that need synchronization.
+unsafe impl<const N: usize, Data> Sync for AttributeList<N, Data> {}
+
+impl<const N: usize, Data> AttributeList<N, Data> {
+    /// # Safety
+    ///
+    /// This function must only be called by the `configfs_attrs`
+    /// macro.
+    #[doc(hidden)]
+    pub const unsafe fn new() -> Self {
+        Self(UnsafeCell::new([core::ptr::null_mut(); N]), PhantomData)
+    }
+
+    /// # Safety
+    ///
+    /// This function can only be called by the `configfs_attrs`
+    /// macro.
+    #[doc(hidden)]
+    pub const unsafe fn add<const I: usize, const ID: u64, O>(
+        &'static self,
+        attribute: &'static Attribute<ID, O, Data>,
+    ) where
+        O: AttributeOperations<ID, Data = Data>,
+    {
+        // We need a space at the end of our list for a null terminator.
+        if I >= N - 1 {
+            kernel::build_error!("Invalid attribute index");
+        }
+
+        // SAFETY: This function is only called through the `configfs_attrs`
+        // macro. This ensures that we are evaluating the function in const
+        // context when initializing a static. As such, the reference created
+        // below will be exclusive.
+        unsafe {
+            (&mut *self.0.get())[I] = (attribute as *const Attribute<ID, O, Data>)
+                .cast_mut()
+                .cast()
+        };
+    }
+}
+
+/// A representation of the attributes that will appear in a [`Group`] or
+/// [`Subsystem`].
+///
+/// Users should not directly instantiate objects of this type. Rather, they
+/// should use the [`kernel::configfs_attrs`] macro to statically declare the
+/// shape of a [`Group`] or [`Subsystem`].
+#[pin_data]
+pub struct ItemType<Container, Data> {
+    #[pin]
+    item_type: Opaque<bindings::config_item_type>,
+    _p: PhantomData<(Container, Data)>,
+}
+
+// SAFETY: We do not provide any operations on `ItemType` that need synchronization.
+unsafe impl<Container, Data> Sync for ItemType<Container, Data> {}
+
+// SAFETY: Ownership of `ItemType` can safely be transferred to other threads.
+unsafe impl<Container, Data> Send for ItemType<Container, Data> {}
+
+macro_rules! impl_item_type {
+    ($tpe:ty) => {
+        impl<Data> ItemType<$tpe, Data> {
+            #[doc(hidden)]
+            pub const fn new_with_child_ctor<const N: usize, Child>(
+                owner: &'static ThisModule,
+                attributes: &'static AttributeList<N, Data>,
+            ) -> Self
+            where
+                Data: GroupOperations<Child = Child>,
+                Child: 'static,
+            {
+                Self {
+                    item_type: Opaque::new(bindings::config_item_type {
+                        ct_owner: owner.as_ptr(),
+                        ct_group_ops: GroupOperationsVTable::<Data, Child>::vtable_ptr().cast_mut(),
+                        ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(),
+                        ct_attrs: (attributes as *const AttributeList<N, Data>)
+                            .cast_mut()
+                            .cast(),
+                        ct_bin_attrs: core::ptr::null_mut(),
+                    }),
+                    _p: PhantomData,
+                }
+            }
+
+            #[doc(hidden)]
+            pub const fn new<const N: usize>(
+                owner: &'static ThisModule,
+                attributes: &'static AttributeList<N, Data>,
+            ) -> Self {
+                Self {
+                    item_type: Opaque::new(bindings::config_item_type {
+                        ct_owner: owner.as_ptr(),
+                        ct_group_ops: core::ptr::null_mut(),
+                        ct_item_ops: ItemOperationsVTable::<$tpe, Data>::vtable_ptr().cast_mut(),
+                        ct_attrs: (attributes as *const AttributeList<N, Data>)
+                            .cast_mut()
+                            .cast(),
+                        ct_bin_attrs: core::ptr::null_mut(),
+                    }),
+                    _p: PhantomData,
+                }
+            }
+        }
+    };
+}
+
+impl_item_type!(Subsystem<Data>);
+impl_item_type!(Group<Data>);
+
+impl<Container, Data> ItemType<Container, Data> {
+    fn as_ptr(&self) -> *const bindings::config_item_type {
+        self.item_type.get()
+    }
+}
+
+/// Define a list of configfs attributes statically.
+///
+/// Invoking the macro in the following manner:
+///
+/// ```ignore
+/// let item_type = configfs_attrs! {
+///     container: configfs::Subsystem<Configuration>,
+///     data: Configuration,
+///     child: Child,
+///     attributes: [
+///         message: 0,
+///         bar: 1,
+///     ],
+/// };
+/// ```
+///
+/// Expands the following output:
+///
+/// ```ignore
+/// let item_type = {
+///     static CONFIGURATION_MESSAGE_ATTR: kernel::configfs::Attribute<
+///         0,
+///         Configuration,
+///         Configuration,
+///     > = unsafe {
+///         kernel::configfs::Attribute::new({
+///             const S: &str = "message\u{0}";
+///             const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+///                 S.as_bytes()
+///             ) {
+///                 Ok(v) => v,
+///                 Err(_) => {
+///                     core::panicking::panic_fmt(core::const_format_args!(
+///                         "string contains interior NUL"
+///                     ));
+///                 }
+///             };
+///             C
+///         })
+///     };
+///
+///     static CONFIGURATION_BAR_ATTR: kernel::configfs::Attribute<
+///             1,
+///             Configuration,
+///             Configuration
+///     > = unsafe {
+///         kernel::configfs::Attribute::new({
+///             const S: &str = "bar\u{0}";
+///             const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+///                 S.as_bytes()
+///             ) {
+///                 Ok(v) => v,
+///                 Err(_) => {
+///                     core::panicking::panic_fmt(core::const_format_args!(
+///                         "string contains interior NUL"
+///                     ));
+///                 }
+///             };
+///             C
+///         })
+///     };
+///
+///     const N: usize = (1usize + (1usize + 0usize)) + 1usize;
+///
+///     static CONFIGURATION_ATTRS: kernel::configfs::AttributeList<N, Configuration> =
+///         unsafe { kernel::configfs::AttributeList::new() };
+///
+///     {
+///         const N: usize = 0usize;
+///         unsafe { CONFIGURATION_ATTRS.add::<N, 0, _>(&CONFIGURATION_MESSAGE_ATTR) };
+///     }
+///
+///     {
+///         const N: usize = (1usize + 0usize);
+///         unsafe { CONFIGURATION_ATTRS.add::<N, 1, _>(&CONFIGURATION_BAR_ATTR) };
+///     }
+///
+///     static CONFIGURATION_TPE:
+///       kernel::configfs::ItemType<configfs::Subsystem<Configuration> ,Configuration>
+///         = kernel::configfs::ItemType::<
+///                 configfs::Subsystem<Configuration>,
+///                 Configuration
+///                 >::new_with_child_ctor::<N,Child>(
+///             &THIS_MODULE,
+///             &CONFIGURATION_ATTRS
+///         );
+///
+///     &CONFIGURATION_TPE
+/// }
+/// ```
+#[macro_export]
+macro_rules! configfs_attrs {
+    (
+        container: $container:ty,
+        data: $data:ty,
+        attributes: [
+            $($name:ident: $attr:literal),* $(,)?
+        ] $(,)?
+    ) => {
+        $crate::configfs_attrs!(
+            count:
+            @container($container),
+            @data($data),
+            @child(),
+            @no_child(x),
+            @attrs($($name $attr)*),
+            @eat($($name $attr,)*),
+            @assign(),
+            @cnt(0usize),
+        )
+    };
+    (
+        container: $container:ty,
+        data: $data:ty,
+        child: $child:ty,
+        attributes: [
+            $($name:ident: $attr:literal),* $(,)?
+        ] $(,)?
+    ) => {
+        $crate::configfs_attrs!(
+            count:
+            @container($container),
+            @data($data),
+            @child($child),
+            @no_child(),
+            @attrs($($name $attr)*),
+            @eat($($name $attr,)*),
+            @assign(),
+            @cnt(0usize),
+        )
+    };
+    (count:
+     @container($container:ty),
+     @data($data:ty),
+     @child($($child:ty)?),
+     @no_child($($no_child:ident)?),
+     @attrs($($aname:ident $aattr:literal)*),
+     @eat($name:ident $attr:literal, $($rname:ident $rattr:literal,)*),
+     @assign($($assign:block)*),
+     @cnt($cnt:expr),
+    ) => {
+        $crate::configfs_attrs!(
+            count:
+            @container($container),
+            @data($data),
+            @child($($child)?),
+            @no_child($($no_child)?),
+            @attrs($($aname $aattr)*),
+            @eat($($rname $rattr,)*),
+            @assign($($assign)* {
+                const N: usize = $cnt;
+                // The following macro text expands to a call to `Attribute::add`.
+                // SAFETY: We are expanding `configfs_attrs`.
+                unsafe {
+                    $crate::macros::paste!(
+                        [< $data:upper _ATTRS >]
+                            .add::<N, $attr, _>(&[< $data:upper _ $name:upper _ATTR >])
+                    )
+                };
+            }),
+            @cnt(1usize + $cnt),
+        )
+    };
+    (count:
+     @container($container:ty),
+     @data($data:ty),
+     @child($($child:ty)?),
+     @no_child($($no_child:ident)?),
+     @attrs($($aname:ident $aattr:literal)*),
+     @eat(),
+     @assign($($assign:block)*),
+     @cnt($cnt:expr),
+    ) =>
+    {
+        $crate::configfs_attrs!(
+            final:
+            @container($container),
+            @data($data),
+            @child($($child)?),
+            @no_child($($no_child)?),
+            @attrs($($aname $aattr)*),
+            @assign($($assign)*),
+            @cnt($cnt),
+        )
+    };
+    (final:
+     @container($container:ty),
+     @data($data:ty),
+     @child($($child:ty)?),
+     @no_child($($no_child:ident)?),
+     @attrs($($name:ident $attr:literal)*),
+     @assign($($assign:block)*),
+     @cnt($cnt:expr),
+    ) =>
+    {
+        $crate::macros::paste!{
+            {
+                $(
+                    // SAFETY: We are expanding `configfs_attrs`.
+                    static [< $data:upper _ $name:upper _ATTR >]:
+                        $crate::configfs::Attribute<$attr, $data, $data> =
+                            unsafe {
+                                $crate::configfs::Attribute::new(c_str!(::core::stringify!($name)))
+                            };
+                )*
+
+
+                // We need space for a null terminator.
+                const N: usize = $cnt + 1usize;
+
+                // SAFETY: We are expanding `configfs_attrs`.
+                static [< $data:upper _ATTRS >]:
+                $crate::configfs::AttributeList<N, $data> =
+                    unsafe { $crate::configfs::AttributeList::new() };
+
+                $($assign)*
+
+                $(
+                    const [<$no_child:upper>]: bool = true;
+
+                    static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data>  =
+                        $crate::configfs::ItemType::<$container, $data>::new::<N>(
+                            &THIS_MODULE, &[<$ data:upper _ATTRS >]
+                        );
+                )?
+
+                $(
+                    static [< $data:upper _TPE >]:
+                        $crate::configfs::ItemType<$container, $data>  =
+                            $crate::configfs::ItemType::<$container, $data>::
+                            new_with_child_ctor::<N, $child>(
+                                &THIS_MODULE, &[<$ data:upper _ATTRS >]
+                            );
+                )?
+
+                & [< $data:upper _TPE >]
+            }
+        }
+    };
+
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5..bcbf5026449d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -42,6 +42,8 @@
 pub mod block;
 #[doc(hidden)]
 pub mod build_assert;
+#[cfg(CONFIG_CONFIGFS_FS)]
+pub mod configfs;
 pub mod cred;
 pub mod device;
 pub mod device_id;

-- 
2.47.2

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Thu, May 1, 2025 at 12:15 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> ---
>

Spurious newlines.

> This patch is a direct dependency for `rnull`, the rust null block driver.
> ---

By the way, you don't need this `---`.

> +//! `configfs` interface.

I don't know if configfs is supposed to be written in code spans, but
I appreciate you are trying to be throughout in your Markdown use ;)
It may be easier to read to not have it in code spans, since we have
many already and it is not code anyway.

By the way, you may want to mention somehow the title they use in
their docs: "Userspace-driven Kernel Object Configuration".

> +//! See the [rust_configfs.rs] sample for a full example use of this module.

Files are, though, like the C header below, so: [`rust_configfs.rs`]

> +/// with configfs, embed a field of this type into your kernel module struct.

Either with or without a code span, i.e. being consistent is best.

> +    /// Return the address of the `bindings::config_group` embedded in `Self`.

I think you may be able to use intra-doc links for [`Self`].

> +        let c_group: *mut bindings::config_group =
> +        // SAFETY: By function safety requirements, `item` is embedded in a
> +        // `config_group`.
> +            unsafe { container_of!(item, bindings::config_group, cg_item) }.cast_mut();

It doesn't work to put the safety comment on top? (We had issues
similar to this in the past, so if it is intentional, that is fine).

> +/// This type is constructed statically at compile time and is by the
> +/// [`kernel::configfs_attrs`] macro.

Sentence is missing something. Also, we never used `# Note` yet, but I
guess it is fine.

> +    /// Null terminated Array of pointers to `Attribute`. The type is `c_void`

Intar-doc link(s)?

> +        // We need a space at the end of our list for a null terminator.
> +        if I >= N - 1 {
> +            kernel::build_error!("Invalid attribute index");
> +        }

Would the following work instead?

    const { assert!(I < N - 1, "Invalid attribute index") };

(Please double-check it actually catches the cases you need)

Thanks!

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Thu, May 1, 2025 at 12:15 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> ---
>>
>
> Spurious newlines.

The one just before the cut?

>
>> This patch is a direct dependency for `rnull`, the rust null block driver.
>> ---
>
> By the way, you don't need this `---`.

b4 adds it 🤷

>
>> +//! `configfs` interface.
>
> I don't know if configfs is supposed to be written in code spans, but
> I appreciate you are trying to be throughout in your Markdown use ;)
> It may be easier to read to not have it in code spans, since we have
> many already and it is not code anyway.

OK

>
> By the way, you may want to mention somehow the title they use in
> their docs: "Userspace-driven Kernel Object Configuration".

Will do.

>
>> +//! See the [rust_configfs.rs] sample for a full example use of this module.
>
> Files are, though, like the C header below, so: [`rust_configfs.rs`]

OK

>
>> +/// with configfs, embed a field of this type into your kernel module struct.
>
> Either with or without a code span, i.e. being consistent is best.

I am! Consistently inconsistent. Very much so in this series. Will fix.

>
>> +    /// Return the address of the `bindings::config_group` embedded in `Self`.
>
> I think you may be able to use intra-doc links for [`Self`].

Thanks. Would be nice with a lint for missed intra-doc links.

>
>> +        let c_group: *mut bindings::config_group =
>> +        // SAFETY: By function safety requirements, `item` is embedded in a
>> +        // `config_group`.
>> +            unsafe { container_of!(item, bindings::config_group, cg_item) }.cast_mut();
>
> It doesn't work to put the safety comment on top? (We had issues
> similar to this in the past, so if it is intentional, that is fine).

Clippy gets mad if we move it up. Because rustfmt wants the unsafe block
to a new line:

  warning: unsafe block missing a safety comment
    --> rust/kernel/configfs.rs:557:13
      |
  557 |             unsafe { container_of!(item, bindings::config_group, cg_item) }.cast_mut();
      |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = help: consider adding a safety comment on the preceding line
      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
      = note: requested on the command line with `-W clippy::undocumented-unsafe-blocks`

>
>> +/// This type is constructed statically at compile time and is by the
>> +/// [`kernel::configfs_attrs`] macro.
>
> Sentence is missing something. Also, we never used `# Note` yet, but I
> guess it is fine.

Thanks, rephrased:

  # Note
  
  Instances of this type are constructed statically at compile by the
  [`kernel::configfs_attrs`] macro.

>
>> +    /// Null terminated Array of pointers to `Attribute`. The type is `c_void`
>
> Intar-doc link(s)?
>
>> +        // We need a space at the end of our list for a null terminator.
>> +        if I >= N - 1 {
>> +            kernel::build_error!("Invalid attribute index");
>> +        }
>
> Would the following work instead?
>
>     const { assert!(I < N - 1, "Invalid attribute index") };
>
> (Please double-check it actually catches the cases you need)

The reason I choose build_error is that if this should somehow end up
being evaluated in non-const context at some point, I want the build to
fail if the condition is not true. I don't think I get that with assert?


Best regards,
Andreas Hindborg

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Thu, May 1, 2025 at 1:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> The one just before the cut?

Both.

> Thanks. Would be nice with a lint for missed intra-doc links.

Definitely -- I filled it a while back:

    https://github.com/rust-lang/rust/issues/131510

> Clippy gets mad if we move it up. Because rustfmt wants the unsafe block
> to a new line:

Yeah, then it is one of the cases I was referring to. In that case, it
is fine, but please indent the safety comment to match the `unsafe`
block.

> The reason I choose build_error is that if this should somehow end up
> being evaluated in non-const context at some point, I want the build to
> fail if the condition is not true. I don't think I get that with assert?

I am not sure what you mean. My understanding is that `const` blocks,
if execution reaches them, are always evaluated at compile-time (they
are a const context):

    https://doc.rust-lang.org/reference/expressions/block-expr.html#const-blocks

e.g.

    https://godbolt.org/z/h36s3nqWK

We are lucky to have Gary with us, since he stabilized this particular
language feature, so he can correct us! :)

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Thu, May 1, 2025 at 1:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> The one just before the cut?
>
> Both.

But why does that matter? Anything in the commit message after the cut
is dropped when applying the patch, right?

>
>> Thanks. Would be nice with a lint for missed intra-doc links.
>
> Definitely -- I filled it a while back:
>
>     https://github.com/rust-lang/rust/issues/131510
>
>> Clippy gets mad if we move it up. Because rustfmt wants the unsafe block
>> to a new line:
>
> Yeah, then it is one of the cases I was referring to. In that case, it
> is fine, but please indent the safety comment to match the `unsafe`
> block.

OK. rustfmt does not seem to care about this though.

>
>> The reason I choose build_error is that if this should somehow end up
>> being evaluated in non-const context at some point, I want the build to
>> fail if the condition is not true. I don't think I get that with assert?
>
> I am not sure what you mean. My understanding is that `const` blocks,
> if execution reaches them, are always evaluated at compile-time (they
> are a const context):
>
>     https://doc.rust-lang.org/reference/expressions/block-expr.html#const-blocks
>
> e.g.
>
>     https://godbolt.org/z/h36s3nqWK
>
> We are lucky to have Gary with us, since he stabilized this particular
> language feature, so he can correct us! :)

I might not have the full picture, but it is my understanding that
while `const fn` are evaluated in const context when called from const
context, they _may_ be called from non-const context, and then they are
evaluated in non-const context if their arguments are not const [1].
They are not guaranteed to be evaluated in const context.

So my thinking is that down the road, refactoring of this code may cause
the `AttributeList::add` to be called in a way so that it is not
evaluated in const context, and then the `assert` would be evaluated at
run time. With `build_error` we would get an error during build.

But I should probably use `build_assert` instead of the conditional with
`build_error`.


Best regards,
Andreas Hindborg


[1] https://doc.rust-lang.org/reference/const_eval.html#r-const-eval.const-expr.list

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Thu, May 1, 2025 at 8:11 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> But why does that matter? Anything in the commit message after the cut
> is dropped when applying the patch, right?

Yes, but it is not common to add a newline there. I mentioned it
because it looked odd, no worries.

> I might not have the full picture, but it is my understanding that
> while `const fn` are evaluated in const context when called from const
> context, they _may_ be called from non-const context, and then they are
> evaluated in non-const context if their arguments are not const [1].
> They are not guaranteed to be evaluated in const context.
>
> So my thinking is that down the road, refactoring of this code may cause
> the `AttributeList::add` to be called in a way so that it is not
> evaluated in const context, and then the `assert` would be evaluated at
> run time. With `build_error` we would get an error during build.

No, it will always be evaluated at compile-time (if it is evaluated at all).

Please see again the links I provided. There is no `const fn` in the
example I provided, and yet it is a build error.

From your link, it is clear `const` blocks are a const context, which
are always evaluated at compile time:

    Certain forms of expressions, called constant expressions, can be
    evaluated at compile time.

    In const contexts, these are the only allowed expressions, and are
    always evaluated at compile time.

    A const context is one of the following: (...) A const block

And from mine, it mentions that it is guaranteed to be evaluated if
execution reaches that point and that such evaluation would happen at
compile time:

    A const block is a variant of a block expression whose body
    evaluates at compile-time instead of at runtime.

    If the const block expression is executed at runtime, then the
    constant is guaranteed to be evaluated, even if its return value is
    ignored:

        fn foo<T>() -> usize {
            // If this code ever gets executed, then the assertion has
            // definitely been evaluated at compile-time.
            const { assert!(std::mem::size_of::<T>() > 0); }
            // Here we can have unsafe code relying on the type being
            // non-zero-sized.
            /* ... */
            42
        }

That example they give is precisely about using a `const` block for
guaranteeing something that unsafe code relies on.

I hope that helps.

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Thu, May 1, 2025 at 8:11 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> But why does that matter? Anything in the commit message after the cut
>> is dropped when applying the patch, right?
>
> Yes, but it is not common to add a newline there. I mentioned it
> because it looked odd, no worries.
>
>> I might not have the full picture, but it is my understanding that
>> while `const fn` are evaluated in const context when called from const
>> context, they _may_ be called from non-const context, and then they are
>> evaluated in non-const context if their arguments are not const [1].
>> They are not guaranteed to be evaluated in const context.
>>
>> So my thinking is that down the road, refactoring of this code may cause
>> the `AttributeList::add` to be called in a way so that it is not
>> evaluated in const context, and then the `assert` would be evaluated at
>> run time. With `build_error` we would get an error during build.
>
> No, it will always be evaluated at compile-time (if it is evaluated at all).
>
> Please see again the links I provided. There is no `const fn` in the
> example I provided, and yet it is a build error.
>
> From your link, it is clear `const` blocks are a const context, which
> are always evaluated at compile time:
>
>     Certain forms of expressions, called constant expressions, can be
>     evaluated at compile time.
>
>     In const contexts, these are the only allowed expressions, and are
>     always evaluated at compile time.
>
>     A const context is one of the following: (...) A const block
>
> And from mine, it mentions that it is guaranteed to be evaluated if
> execution reaches that point and that such evaluation would happen at
> compile time:
>
>     A const block is a variant of a block expression whose body
>     evaluates at compile-time instead of at runtime.
>
>     If the const block expression is executed at runtime, then the
>     constant is guaranteed to be evaluated, even if its return value is
>     ignored:
>
>         fn foo<T>() -> usize {
>             // If this code ever gets executed, then the assertion has
>             // definitely been evaluated at compile-time.
>             const { assert!(std::mem::size_of::<T>() > 0); }
>             // Here we can have unsafe code relying on the type being
>             // non-zero-sized.
>             /* ... */
>             42
>         }
>
> That example they give is precisely about using a `const` block for
> guaranteeing something that unsafe code relies on.
>
> I hope that helps.

Ah - I understand. I missed that you wrapped the assert in a const
block, a quite important bit 🤦 Thanks for explaining!


Best regards,
Andreas Hindborg

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Thu, May 1, 2025 at 9:51 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Ah - I understand. I missed that you wrapped the assert in a const
> block, a quite important bit 🤦 Thanks for explaining!

Ah -- that explains it. Yeah, your calls to `add` were from const
contexts, which explains what you were trying to say.

You're welcome!

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Thu, May 1, 2025 at 11:28 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Ah -- that explains it. Yeah, your calls to `add` were from const
> contexts, which explains what you were trying to say.

No, wait, they are not from const context. So a "normal" `assert!`
there would have never worked.

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Thu, May 1, 2025 at 11:28 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> Ah -- that explains it. Yeah, your calls to `add` were from const
>> contexts, which explains what you were trying to say.
>
> No, wait, they are not from const context. So a "normal" `assert!`
> there would have never worked.

Right, they are not from const context, but they are inside a `let`
statement, so if all the captured variables are constant (which they
are), the let statement will be evaluated in const context [1], right?

Best regards,
Andreas Hindborg


[1] https://doc.rust-lang.org/reference/const_eval.html#r-const-eval.const-context.init

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Fri, May 2, 2025 at 8:57 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Right, they are not from const context, but they are inside a `let`
> statement, so if all the captured variables are constant (which they
> are), the let statement will be evaluated in const context [1], right?

No, I don't think so. Though I don't know what you mean by "captured
variables" here or why you point to [const-eval.const-context.init].

The way I read the reference is that Rust only guarantees evaluation
at compile-time within const contexts, and a `let` statement is not a
const context and its initializer is not one of the ones listed in
[const-eval.const-context.init]. `const fn` isn't a const context
either.

Which makes sense -- the `let` initializers are just "normal"
expressions, i.e. you don't want to limit the kinds of code you can
run there.

For instance, here there is a panic at runtime trying to mimic a bit the patch:

    https://godbolt.org/z/v5qdK9vve

Similarly, if I take your patch and put there an `assert!(false)` in
`add` -- I see no build error and `objdump` shows the panic call from
the sample.

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Fri, May 2, 2025 at 8:57 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Right, they are not from const context, but they are inside a `let`
>> statement, so if all the captured variables are constant (which they
>> are), the let statement will be evaluated in const context [1], right?
>
> No, I don't think so. Though I don't know what you mean by "captured
> variables" here or why you point to [const-eval.const-context.init].

I see. it is not guaranteed, but it _may_ be.

Regarding captured variables, I was referring to "any operands". From
[const-eval.const-context.init]:

  Const context is one of the following:
  ...
  The initializer of statics.

From [const-eval.const-expr.list]:

  The following expressions are constant expressions, so long as any
  operands are also constant expressions and do not cause any Drop::drop
  calls to be run.

  ...

[const-eval.const-expr.block]:

  ...

  let statements

So I was thinking that because I am initializing a static with a let
statement, it would run in const context. But I see that it is not
actually guaranteed.

> The way I read the reference is that Rust only guarantees evaluation
> at compile-time within const contexts, and a `let` statement is not a
> const context and its initializer is not one of the ones listed in
> [const-eval.const-context.init]. `const fn` isn't a const context
> either.

I agree. No guarantees, [const-eval.const-expr.runtime-context]:

  In other places, such as let statements, constant expressions may be,
  but are not guaranteed to be, evaluated at compile time.

> Which makes sense -- the `let` initializers are just "normal"
> expressions, i.e. you don't want to limit the kinds of code you can
> run there.
>
> For instance, here there is a panic at runtime trying to mimic a bit the patch:
>
>     https://godbolt.org/z/v5qdK9vve
>
> Similarly, if I take your patch and put there an `assert!(false)` in
> `add` -- I see no build error and `objdump` shows the panic call from
> the sample.

Right. Which is why I opted for `build_error`. But with the `const`
block solution you suggested is better.


Best regards,
Andreas Hindborg

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Miguel Ojeda]

On Mon, May 5, 2025 at 9:51 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> So I was thinking that because I am initializing a static with a let
> statement, it would run in const context. But I see that it is not
> actually guaranteed.

No, that is actually guaranteed, i.e. when initializing a static. But
you aren't initializing a static here, no? Which static are you
referring to? If you were, then the "normal" `assert!` would work,
because it would be a const context.

The `add` calls I see are just in the `let` statement, not
initializing any static:

            {
                const N: usize = 0usize;
                unsafe { CONFIGURATION_ATTRS.add::<N, 0,
_>(&CONFIGURATION_MESSAGE_ATTR) };
            }

So it also means this comment is wrong:

+        // SAFETY: This function is only called through the `configfs_attrs`
+        // macro. This ensures that we are evaluating the function in const
+        // context when initializing a static. As such, the reference created
+        // below will be exclusive.

Please double-check all this... :)

> Right. Which is why I opted for `build_error`. But with the `const`
> block solution you suggested is better.

I thought you opted for that because you thought the `assert!` would
only work if not refactored. What I tried to point out was that the
`assert!` wouldn't have worked even before the refactoring.

I hope that helps.

Cheers,
Miguel

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Mon, May 5, 2025 at 9:51 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> So I was thinking that because I am initializing a static with a let
>> statement, it would run in const context. But I see that it is not
>> actually guaranteed.
>
> No, that is actually guaranteed, i.e. when initializing a static. But
> you aren't initializing a static here, no? Which static are you
> referring to? If you were, then the "normal" `assert!` would work,
> because it would be a const context.
>
> The `add` calls I see are just in the `let` statement, not
> initializing any static:
>
>             {
>                 const N: usize = 0usize;
>                 unsafe { CONFIGURATION_ATTRS.add::<N, 0,
> _>(&CONFIGURATION_MESSAGE_ATTR) };
>             }
>
> So it also means this comment is wrong:
>
> +        // SAFETY: This function is only called through the `configfs_attrs`
> +        // macro. This ensures that we are evaluating the function in const
> +        // context when initializing a static. As such, the reference created
> +        // below will be exclusive.
>
> Please double-check all this... :)

Oops.

>
>> Right. Which is why I opted for `build_error`. But with the `const`
>> block solution you suggested is better.
>
> I thought you opted for that because you thought the `assert!` would
> only work if not refactored. What I tried to point out was that the
> `assert!` wouldn't have worked even before the refactoring.

I made a mistake in thinking this was in const context. I'll see if I
can fix that.



Best regards,
Andreas Hindborg

Re: [PATCH v6 1/3] rust: configfs: introduce rust support for configfs

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:
>
>> On Mon, May 5, 2025 at 9:51 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>> So I was thinking that because I am initializing a static with a let
>>> statement, it would run in const context. But I see that it is not
>>> actually guaranteed.
>>
>> No, that is actually guaranteed, i.e. when initializing a static. But
>> you aren't initializing a static here, no? Which static are you
>> referring to? If you were, then the "normal" `assert!` would work,
>> because it would be a const context.
>>
>> The `add` calls I see are just in the `let` statement, not
>> initializing any static:
>>
>>             {
>>                 const N: usize = 0usize;
>>                 unsafe { CONFIGURATION_ATTRS.add::<N, 0,
>> _>(&CONFIGURATION_MESSAGE_ATTR) };
>>             }
>>
>> So it also means this comment is wrong:
>>
>> +        // SAFETY: This function is only called through the `configfs_attrs`
>> +        // macro. This ensures that we are evaluating the function in const
>> +        // context when initializing a static. As such, the reference created
>> +        // below will be exclusive.
>>
>> Please double-check all this... :)
>
> Oops.
>
>>
>>> Right. Which is why I opted for `build_error`. But with the `const`
>>> block solution you suggested is better.
>>
>> I thought you opted for that because you thought the `assert!` would
>> only work if not refactored. What I tried to point out was that the
>> `assert!` wouldn't have worked even before the refactoring.
>
> I made a mistake in thinking this was in const context. I'll see if I
> can fix that.


I think I can fix it like this:


modified   rust/kernel/configfs.rs
@@ -703,8 +703,8 @@ impl<const N: usize, Data> AttributeList<N, Data> {
 
     /// # Safety
     ///
-    /// This function can only be called by the [`kernel::configfs_attrs`]
-    /// macro.
+    /// The caller must ensure that there are no other concurrent accesses to
+    /// `self`. That is, the caller has exclusive access to `self.`
     #[doc(hidden)]
     pub const unsafe fn add<const I: usize, const ID: u64, O>(
         &'static self,
@@ -715,10 +715,8 @@ impl<const N: usize, Data> AttributeList<N, Data> {
         // We need a space at the end of our list for a null terminator.
         const { assert!(I < N - 1, "Invalid attribute index") };
 
-        // SAFETY: This function is only called through the
-        // [kernel::`configfs_attrs`] macro. This ensures that we are evaluating
-        // the function in const context when initializing a static. As such,
-        // the reference created below will be exclusive.
+        // SAFETY: By function safety requirements, we have exclusive access to
+        // `self` and the reference created below will be exclusive.
         unsafe {
             (&mut *self.0.get())[I] = (attribute as *const Attribute<ID, O, Data>)
                 .cast_mut()
@@ -955,7 +953,12 @@ macro_rules! configfs_attrs {
             @assign($($assign)* {
                 const N: usize = $cnt;
                 // The following macro text expands to a call to `Attribute::add`.
-                // SAFETY: We are expanding [`kernel::configfs_attrs`].
+
+                // SAFETY: By design of this macro, the name of the variable we
+                // invoke the `add` method on below, is not visible outside of
+                // the macro expansion. The macro does not operate concurrently
+                // on this variable, and thus we have exclusive access to the
+                // variable.
                 unsafe {
                     $crate::macros::paste!(
                         [< $data:upper _ATTRS >]


Best regards,
Andreas Hindborg

[PATCH v6 2/3] rust: configfs: add a sample demonstrating configfs usage

[Andreas Hindborg]

Add a sample to the samples folder, demonstrating the intended use of the
Rust configfs API.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 samples/rust/Kconfig          |  11 +++
 samples/rust/Makefile         |   1 +
 samples/rust/rust_configfs.rs | 192 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 204 insertions(+)

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index cad52b7120b5..be491ad9b3af 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -10,6 +10,17 @@ menuconfig SAMPLES_RUST
 
 if SAMPLES_RUST
 
+config SAMPLE_RUST_CONFIGFS
+	tristate "Configfs sample"
+	depends on CONFIGFS_FS
+	help
+	  This option builds the Rust configfs sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_configfs.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_MINIMAL
 	tristate "Minimal"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index c6a2479f7d9c..b3c9178d654a 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
+obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
 
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
new file mode 100644
index 000000000000..9c0989072a8f
--- /dev/null
+++ b/samples/rust/rust_configfs.rs
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust configfs sample.
+
+use kernel::alloc::flags;
+use kernel::c_str;
+use kernel::configfs;
+use kernel::configfs_attrs;
+use kernel::new_mutex;
+use kernel::page::PAGE_SIZE;
+use kernel::prelude::*;
+use kernel::sync::Mutex;
+
+module! {
+    type: RustConfigfs,
+    name: "rust_configfs",
+    author: "Rust for Linux Contributors",
+    description: "Rust configfs sample",
+    license: "GPL",
+}
+
+#[pin_data]
+struct RustConfigfs {
+    #[pin]
+    config: configfs::Subsystem<Configuration>,
+}
+
+#[pin_data]
+struct Configuration {
+    message: &'static CStr,
+    #[pin]
+    bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
+}
+
+impl Configuration {
+    fn new() -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {
+            message: c_str!("Hello World\n"),
+            bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
+        })
+    }
+}
+
+impl kernel::InPlaceModule for RustConfigfs {
+    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
+        pr_info!("Rust configfs sample (init)\n");
+
+        // Define a subsystem with the data type `Configuration`, two
+        // attributes, `message` and `bar` and child group type `Child`. `mkdir`
+        // in the directory representing this subsystem will create directories
+        // backed by the `Child` type.
+        let item_type = configfs_attrs! {
+            container: configfs::Subsystem<Configuration>,
+            data: Configuration,
+            child: Child,
+            attributes: [
+                message: 0,
+                bar: 1,
+            ],
+        };
+
+        try_pin_init!(Self {
+            config <- configfs::Subsystem::new(
+                c_str!("rust_configfs"), item_type, Configuration::new()
+            ),
+        })
+    }
+}
+
+#[vtable]
+impl configfs::GroupOperations for Configuration {
+    type Child = Child;
+
+    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
+        // Define a group with data type `Child`, one attribute `baz` and child
+        // group type `GrandChild`. `mkdir` in the directory representing this
+        // group will create directories backed by the `GrandChild` type.
+        let tpe = configfs_attrs! {
+            container: configfs::Group<Child>,
+            data: Child,
+            child: GrandChild,
+            attributes: [
+                baz: 0,
+            ],
+        };
+
+        Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<0> for Configuration {
+    type Data = Configuration;
+
+    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        pr_info!("Show message\n");
+        let data = container.message;
+        page[0..data.len()].copy_from_slice(data);
+        Ok(data.len())
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<1> for Configuration {
+    type Data = Configuration;
+
+    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        pr_info!("Show bar\n");
+        let guard = container.bar.lock();
+        let data = guard.0.as_slice();
+        let len = guard.1;
+        page[0..len].copy_from_slice(&data[0..len]);
+        Ok(len)
+    }
+
+    fn store(container: &Configuration, page: &[u8]) -> Result {
+        pr_info!("Store bar\n");
+        let mut guard = container.bar.lock();
+        guard.0[0..page.len()].copy_from_slice(page);
+        guard.1 = page.len();
+        Ok(())
+    }
+}
+
+// `pin_data` cannot handle structs without braces.
+#[pin_data]
+struct Child {}
+
+impl Child {
+    fn new() -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {})
+    }
+}
+
+#[vtable]
+impl configfs::GroupOperations for Child {
+    type Child = GrandChild;
+
+    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
+        // Define a group with data type `GrandChild`, one attribute `gz`. As no
+        // child type is specified, it will not be possible to create subgroups
+        // in this group, and `mkdir`in the directory representing this group
+        // will return an error.
+        let tpe = configfs_attrs! {
+            container: configfs::Group<GrandChild>,
+            data: GrandChild,
+            attributes: [
+                gc: 0,
+            ],
+        };
+
+        Ok(configfs::Group::new(
+            name.try_into()?,
+            tpe,
+            GrandChild::new(),
+        ))
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<0> for Child {
+    type Data = Child;
+
+    fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        pr_info!("Show baz\n");
+        let data = c"Hello Baz\n".to_bytes();
+        page[0..data.len()].copy_from_slice(data);
+        Ok(data.len())
+    }
+}
+
+// `pin_data` cannot handle structs without braces.
+#[pin_data]
+struct GrandChild {}
+
+impl GrandChild {
+    fn new() -> impl PinInit<Self, Error> {
+        try_pin_init!(Self {})
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<0> for GrandChild {
+    type Data = GrandChild;
+
+    fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        pr_info!("Show grand child\n");
+        let data = c"Hello GC\n".to_bytes();
+        page[0..data.len()].copy_from_slice(data);
+        Ok(data.len())
+    }
+}

-- 
2.47.2

Re: [PATCH v6 2/3] rust: configfs: add a sample demonstrating configfs usage

[Charalampos Mitrodimas]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Add a sample to the samples folder, demonstrating the intended use of the
> Rust configfs API.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  samples/rust/Kconfig          |  11 +++
>  samples/rust/Makefile         |   1 +
>  samples/rust/rust_configfs.rs | 192 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 204 insertions(+)
>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index cad52b7120b5..be491ad9b3af 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -10,6 +10,17 @@ menuconfig SAMPLES_RUST
>  
>  if SAMPLES_RUST
>  
> +config SAMPLE_RUST_CONFIGFS
> +	tristate "Configfs sample"
> +	depends on CONFIGFS_FS
> +	help
> +	  This option builds the Rust configfs sample.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_configfs.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_MINIMAL
>  	tristate "Minimal"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index c6a2479f7d9c..b3c9178d654a 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
> +obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
>  
>  rust_print-y := rust_print_main.o rust_print_events.o
>  
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> new file mode 100644
> index 000000000000..9c0989072a8f
> --- /dev/null
> +++ b/samples/rust/rust_configfs.rs
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust configfs sample.
> +
> +use kernel::alloc::flags;
> +use kernel::c_str;
> +use kernel::configfs;
> +use kernel::configfs_attrs;
> +use kernel::new_mutex;
> +use kernel::page::PAGE_SIZE;
> +use kernel::prelude::*;
> +use kernel::sync::Mutex;
> +
> +module! {
> +    type: RustConfigfs,
> +    name: "rust_configfs",
> +    author: "Rust for Linux Contributors",
> +    description: "Rust configfs sample",
> +    license: "GPL",
> +}
> +
> +#[pin_data]
> +struct RustConfigfs {
> +    #[pin]
> +    config: configfs::Subsystem<Configuration>,
> +}
> +
> +#[pin_data]
> +struct Configuration {
> +    message: &'static CStr,
> +    #[pin]
> +    bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
> +}
> +
> +impl Configuration {
> +    fn new() -> impl PinInit<Self, Error> {
> +        try_pin_init!(Self {
> +            message: c_str!("Hello World\n"),
> +            bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
> +        })
> +    }
> +}
> +
> +impl kernel::InPlaceModule for RustConfigfs {
> +    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> +        pr_info!("Rust configfs sample (init)\n");
> +
> +        // Define a subsystem with the data type `Configuration`, two
> +        // attributes, `message` and `bar` and child group type `Child`. `mkdir`
> +        // in the directory representing this subsystem will create directories
> +        // backed by the `Child` type.
> +        let item_type = configfs_attrs! {
> +            container: configfs::Subsystem<Configuration>,
> +            data: Configuration,
> +            child: Child,
> +            attributes: [
> +                message: 0,
> +                bar: 1,
> +            ],
> +        };
> +
> +        try_pin_init!(Self {
> +            config <- configfs::Subsystem::new(
> +                c_str!("rust_configfs"), item_type, Configuration::new()
> +            ),
> +        })
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Configuration {
> +    type Child = Child;
> +
> +    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
> +        // Define a group with data type `Child`, one attribute `baz` and child
> +        // group type `GrandChild`. `mkdir` in the directory representing this
> +        // group will create directories backed by the `GrandChild` type.
> +        let tpe = configfs_attrs! {
> +            container: configfs::Group<Child>,
> +            data: Child,
> +            child: GrandChild,
> +            attributes: [
> +                baz: 0,
> +            ],
> +        };
> +
> +        Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for Configuration {
> +    type Data = Configuration;
> +
> +    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        pr_info!("Show message\n");
> +        let data = container.message;
> +        page[0..data.len()].copy_from_slice(data);
> +        Ok(data.len())
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<1> for Configuration {
> +    type Data = Configuration;
> +
> +    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        pr_info!("Show bar\n");
> +        let guard = container.bar.lock();
> +        let data = guard.0.as_slice();
> +        let len = guard.1;
> +        page[0..len].copy_from_slice(&data[0..len]);
> +        Ok(len)
> +    }
> +
> +    fn store(container: &Configuration, page: &[u8]) -> Result {
> +        pr_info!("Store bar\n");
> +        let mut guard = container.bar.lock();
> +        guard.0[0..page.len()].copy_from_slice(page);
> +        guard.1 = page.len();
> +        Ok(())
> +    }
> +}
> +
> +// `pin_data` cannot handle structs without braces.
> +#[pin_data]
> +struct Child {}
> +
> +impl Child {
> +    fn new() -> impl PinInit<Self, Error> {
> +        try_pin_init!(Self {})
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Child {
> +    type Child = GrandChild;
> +
> +    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
> +        // Define a group with data type `GrandChild`, one attribute `gz`. As no

Hi Andreas,

Maybe you meant "one attribute `gc`" here?

C. Mitrodimas

> +        // child type is specified, it will not be possible to create subgroups
> +        // in this group, and `mkdir`in the directory representing this group
> +        // will return an error.
> +        let tpe = configfs_attrs! {
> +            container: configfs::Group<GrandChild>,
> +            data: GrandChild,
> +            attributes: [
> +                gc: 0,
> +            ],
> +        };
> +
> +        Ok(configfs::Group::new(
> +            name.try_into()?,
> +            tpe,
> +            GrandChild::new(),
> +        ))
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for Child {
> +    type Data = Child;
> +
> +    fn show(_container: &Child, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        pr_info!("Show baz\n");
> +        let data = c"Hello Baz\n".to_bytes();
> +        page[0..data.len()].copy_from_slice(data);
> +        Ok(data.len())
> +    }
> +}
> +
> +// `pin_data` cannot handle structs without braces.
> +#[pin_data]
> +struct GrandChild {}
> +
> +impl GrandChild {
> +    fn new() -> impl PinInit<Self, Error> {
> +        try_pin_init!(Self {})
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for GrandChild {
> +    type Data = GrandChild;
> +
> +    fn show(_container: &GrandChild, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        pr_info!("Show grand child\n");
> +        let data = c"Hello GC\n".to_bytes();
> +        page[0..data.len()].copy_from_slice(data);
> +        Ok(data.len())
> +    }
> +}

Re: [PATCH v6 2/3] rust: configfs: add a sample demonstrating configfs usage

[Andreas Hindborg]

"Charalampos Mitrodimas" <charmitro@posteo.net> writes:

> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> Add a sample to the samples folder, demonstrating the intended use of the
>> Rust configfs API.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  samples/rust/Kconfig          |  11 +++
>>  samples/rust/Makefile         |   1 +
>>  samples/rust/rust_configfs.rs | 192 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 204 insertions(+)
>>
>> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
>> index cad52b7120b5..be491ad9b3af 100644
>> --- a/samples/rust/Kconfig
>> +++ b/samples/rust/Kconfig
>> @@ -10,6 +10,17 @@ menuconfig SAMPLES_RUST
>>
>>  if SAMPLES_RUST
>>
>> +config SAMPLE_RUST_CONFIGFS
>> +	tristate "Configfs sample"
>> +	depends on CONFIGFS_FS
>> +	help
>> +	  This option builds the Rust configfs sample.
>> +
>> +	  To compile this as a module, choose M here:
>> +	  the module will be called rust_configfs.
>> +
>> +	  If unsure, say N.
>> +
>>  config SAMPLE_RUST_MINIMAL
>>  	tristate "Minimal"
>>  	help
>> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
>> index c6a2479f7d9c..b3c9178d654a 100644
>> --- a/samples/rust/Makefile
>> +++ b/samples/rust/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
>>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
>>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
>>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
>> +obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
>>
>>  rust_print-y := rust_print_main.o rust_print_events.o
>>
>> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
>> new file mode 100644
>> index 000000000000..9c0989072a8f
>> --- /dev/null
>> +++ b/samples/rust/rust_configfs.rs
>> @@ -0,0 +1,192 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Rust configfs sample.
>> +
>> +use kernel::alloc::flags;
>> +use kernel::c_str;
>> +use kernel::configfs;
>> +use kernel::configfs_attrs;
>> +use kernel::new_mutex;
>> +use kernel::page::PAGE_SIZE;
>> +use kernel::prelude::*;
>> +use kernel::sync::Mutex;
>> +
>> +module! {
>> +    type: RustConfigfs,
>> +    name: "rust_configfs",
>> +    author: "Rust for Linux Contributors",
>> +    description: "Rust configfs sample",
>> +    license: "GPL",
>> +}
>> +
>> +#[pin_data]
>> +struct RustConfigfs {
>> +    #[pin]
>> +    config: configfs::Subsystem<Configuration>,
>> +}
>> +
>> +#[pin_data]
>> +struct Configuration {
>> +    message: &'static CStr,
>> +    #[pin]
>> +    bar: Mutex<(KBox<[u8; PAGE_SIZE]>, usize)>,
>> +}
>> +
>> +impl Configuration {
>> +    fn new() -> impl PinInit<Self, Error> {
>> +        try_pin_init!(Self {
>> +            message: c_str!("Hello World\n"),
>> +            bar <- new_mutex!((KBox::new([0; PAGE_SIZE], flags::GFP_KERNEL)?, 0)),
>> +        })
>> +    }
>> +}
>> +
>> +impl kernel::InPlaceModule for RustConfigfs {
>> +    fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>> +        pr_info!("Rust configfs sample (init)\n");
>> +
>> +        // Define a subsystem with the data type `Configuration`, two
>> +        // attributes, `message` and `bar` and child group type `Child`. `mkdir`
>> +        // in the directory representing this subsystem will create directories
>> +        // backed by the `Child` type.
>> +        let item_type = configfs_attrs! {
>> +            container: configfs::Subsystem<Configuration>,
>> +            data: Configuration,
>> +            child: Child,
>> +            attributes: [
>> +                message: 0,
>> +                bar: 1,
>> +            ],
>> +        };
>> +
>> +        try_pin_init!(Self {
>> +            config <- configfs::Subsystem::new(
>> +                c_str!("rust_configfs"), item_type, Configuration::new()
>> +            ),
>> +        })
>> +    }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::GroupOperations for Configuration {
>> +    type Child = Child;
>> +
>> +    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<Child>, Error>> {
>> +        // Define a group with data type `Child`, one attribute `baz` and child
>> +        // group type `GrandChild`. `mkdir` in the directory representing this
>> +        // group will create directories backed by the `GrandChild` type.
>> +        let tpe = configfs_attrs! {
>> +            container: configfs::Group<Child>,
>> +            data: Child,
>> +            child: GrandChild,
>> +            attributes: [
>> +                baz: 0,
>> +            ],
>> +        };
>> +
>> +        Ok(configfs::Group::new(name.try_into()?, tpe, Child::new()))
>> +    }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<0> for Configuration {
>> +    type Data = Configuration;
>> +
>> +    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> +        pr_info!("Show message\n");
>> +        let data = container.message;
>> +        page[0..data.len()].copy_from_slice(data);
>> +        Ok(data.len())
>> +    }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::AttributeOperations<1> for Configuration {
>> +    type Data = Configuration;
>> +
>> +    fn show(container: &Configuration, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>> +        pr_info!("Show bar\n");
>> +        let guard = container.bar.lock();
>> +        let data = guard.0.as_slice();
>> +        let len = guard.1;
>> +        page[0..len].copy_from_slice(&data[0..len]);
>> +        Ok(len)
>> +    }
>> +
>> +    fn store(container: &Configuration, page: &[u8]) -> Result {
>> +        pr_info!("Store bar\n");
>> +        let mut guard = container.bar.lock();
>> +        guard.0[0..page.len()].copy_from_slice(page);
>> +        guard.1 = page.len();
>> +        Ok(())
>> +    }
>> +}
>> +
>> +// `pin_data` cannot handle structs without braces.
>> +#[pin_data]
>> +struct Child {}
>> +
>> +impl Child {
>> +    fn new() -> impl PinInit<Self, Error> {
>> +        try_pin_init!(Self {})
>> +    }
>> +}
>> +
>> +#[vtable]
>> +impl configfs::GroupOperations for Child {
>> +    type Child = GrandChild;
>> +
>> +    fn make_group(&self, name: &CStr) -> Result<impl PinInit<configfs::Group<GrandChild>, Error>> {
>> +        // Define a group with data type `GrandChild`, one attribute `gz`. As no
>
> Hi Andreas,
>
> Maybe you meant "one attribute `gc`" here?

Yes, thanks!


Best regards,
Andreas Hindborg

[PATCH v6 3/3] MAINTAINERS: add configfs Rust abstractions

[Andreas Hindborg]

Add rust files to configfs MAINTAINERS entry.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3cbf9ac0d83f..5ccf04e1eb1f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5982,7 +5982,9 @@ S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git configfs-next
 F:	fs/configfs/
 F:	include/linux/configfs.h
+F:	rust/kernel/configfs.rs
 F:	samples/configfs/
+F:	samples/rust/rust_configfs.rs
 
 CONGATEC BOARD CONTROLLER MFD DRIVER
 M:	Thomas Richard <thomas.richard@bootlin.com>

-- 
2.47.2