Re: [PATCH] configfs: rust: add an API for adding default groups from C

[Andreas Hindborg <a.hindborg@kernel.org>]

Alice Ryhl <aliceryhl@google.com> writes:

> On Sun, Feb 15, 2026 at 09:33:47PM +0100, Andreas Hindborg wrote:
>> Some C subsystems provide a feature to add configfs default groups to the
>> configfs hierarchy of other drivers or subsystems. Rust abstractions for
>> these subsystems will want a way to add these default groups via the
>> configfs Rust API. So add infrastructure to make this possible.
>> 
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  drivers/block/rnull/configfs.rs |  1 +
>>  rust/helpers/configfs.c         | 16 ++++++++++++++++
>>  rust/helpers/helpers.c          |  1 +
>>  rust/kernel/configfs.rs         | 33 ++++++++++++++++++++++++++++++++-
>>  samples/rust/rust_configfs.rs   |  8 +++++++-
>>  5 files changed, 57 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
>> index 6713a6d92391d..ea38b27a9011c 100644
>> --- a/drivers/block/rnull/configfs.rs
>> +++ b/drivers/block/rnull/configfs.rs
>> @@ -78,6 +78,7 @@ fn make_group(
>>                      name: name.try_into()?,
>>                  }),
>>              }),
>> +            core::iter::empty(),
>>          ))
>>      }
>>  }
>> diff --git a/rust/helpers/configfs.c b/rust/helpers/configfs.c
>> new file mode 100644
>> index 0000000000000..7cec8ffcb093d
>> --- /dev/null
>> +++ b/rust/helpers/configfs.c
>> @@ -0,0 +1,16 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/configfs.h>
>> +
>> +__rust_helper void
>> +rust_helper_configfs_add_default_group(struct config_group *new_group,
>> +				       struct config_group *group)
>> +{
>> +	configfs_add_default_group(new_group, group);
>> +}
>> +
>> +__rust_helper void
>> +__rust_helper_configfs_remove_default_groups(struct config_group *group)
>
> Only the annotation has the two __ prefix, not the symbol name.

Hmm, odd that the bindings generate correct anyway.

>
>> +#[pinned_drop]
>> +impl<Data> PinnedDrop for Group<Data> {
>> +    fn drop(self: Pin<&mut Self>) {
>> +        // SAFETY: We have exclusive access to `self` and we know the default groups are alive
>> +        // because we reference them through `self.default_groups`.
>> +        unsafe { bindings::configfs_remove_default_groups(self.group.get()) };
>
> Why isn't this here already?

What do you mean. The drop of default groups is not there already,
because it was not previously possible to add default groups. So there
would be nothing to drop.

>
>> +    }
>>  }
>>  
>>  impl<Data> Group<Data> {
>> @@ -259,7 +269,13 @@ pub fn new(
>>          name: CString,
>>          item_type: &'static ItemType<Group<Data>, Data>,
>>          data: impl PinInit<Data, Error>,
>> +        default_groups: impl IntoIterator<Item = Arc<dyn CDefaultGroup>>,
>
> Honestly, I'd just take a KVec here.

I was trying to be fancy. Is it not more flexible to take an iterator?
What are the pros/cons?

>
>>      ) -> impl PinInit<Self, Error> {
>> +        let mut dg = KVec::new();
>> +        for group in default_groups {
>> +            dg.push(group, GFP_KERNEL).unwrap();
>> +        }
>> +
>>          try_pin_init!(Self {
>>              group <- pin_init::init_zeroed().chain(|v: &mut Opaque<bindings::config_group>| {
>>                  let place = v.get();
>> @@ -268,13 +284,28 @@ pub fn new(
>>                  unsafe {
>>                      bindings::config_group_init_type_name(place, name.cast(), item_type.as_ptr())
>>                  };
>> +
>> +                for default_group in &dg {
>> +                    // SAFETY: We keep the default groups alive until `Self` is dropped.
>> +                    unsafe { bindings::configfs_add_default_group(default_group.group_ptr(), place) }
>> +                }
>
> Do these not need to be removed in drop?

Yes, I added the drop logic above. It is the hunk you asked why is it not
already here?


Best regards,
Andreas Hindborg