Rust for Linux List
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Malte Wechter" <maltewechter@gmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Breno Leitao" <leitao@debian.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Jens Axboe" <axboe@kernel.dk>
Cc: <linux-kernel@vger.kernel.org>, <rust-for-linux@vger.kernel.org>,
	<linux-block@vger.kernel.org>
Subject: Re: [PATCH v2] rust: add procedural macro for declaring configfs attributes
Date: Wed, 03 Jun 2026 15:32:49 +0100	[thread overview]
Message-ID: <DIZHKJSZXQ4Z.6SPXJVGDV9ZW@garyguo.net> (raw)
In-Reply-To: <20260603-configfs-syn-v2-1-cb58489c2647@gmail.com>

On Wed Jun 3, 2026 at 3:08 PM BST, Malte Wechter wrote:
> Implement `configfs_attrs!` as a procedural macro using `syn`, this
> improves readability and maintainability. Remove the old macro and
> replace all uses with the new macro. Add the new macro implementation
> file to MAINTAINERS.
>
> Signed-off-by: Malte Wechter <maltewechter@gmail.com>
> ---
> Changes in v2:
> - Add a try_parse helper function to macros/helpers.rs
> - Fix bug where 'child' configuration gets dropped if trailing comma is missing (sashiko)
> - Link to v1: https://lore.kernel.org/r/20260520-configfs-syn-v1-1-6c5b80a9cef2@gmail.com
> ---
>  MAINTAINERS                     |   1 +
>  drivers/block/rnull/configfs.rs |   2 +-
>  rust/kernel/configfs.rs         | 251 ----------------------------------------
>  rust/macros/configfs_attrs.rs   | 182 +++++++++++++++++++++++++++++
>  rust/macros/helpers.rs          |  15 +++
>  rust/macros/lib.rs              |  85 ++++++++++++++
>  samples/rust/rust_configfs.rs   |   2 +-
>  7 files changed, 285 insertions(+), 253 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2fb1c75afd163..45f7a1ec93b45 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6464,6 +6464,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git config
>  F:	fs/configfs/
>  F:	include/linux/configfs.h
>  F:	rust/kernel/configfs.rs
> +F:	rust/macros/configfs_attrs.rs
>  F:	samples/configfs/
>  F:	samples/rust/rust_configfs.rs
>  
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 7c2eb5c0b7228..f28ec69d79846 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -4,8 +4,8 @@
>  use kernel::{
>      block::mq::gen_disk::{GenDisk, GenDiskBuilder},
>      configfs::{self, AttributeOperations},
> -    configfs_attrs,
>      fmt::{self, Write as _},
> +    macros::configfs_attrs,
>      new_mutex,
>      page::PAGE_SIZE,
>      prelude::*,
> diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
> new file mode 100644
> index 0000000000000..a7fc75cdebcc0
> --- /dev/null
> +++ b/rust/macros/configfs_attrs.rs
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use quote::{
> +    quote, //
> +    ToTokens,
> +};
> +
> +use proc_macro2::{
> +    Span, //
> +};
> +
> +use syn::{
> +    bracketed,
> +    parse::{
> +        Parse,
> +        ParseStream, //
> +    },
> +    punctuated::Punctuated,
> +    spanned::Spanned,
> +    Ident,
> +    LitInt,
> +    Token,
> +    Type, //
> +};
> +
> +use crate::helpers::try_parse;
> +
> +pub(crate) struct ConfigfsAttrs {
> +    container: Type,
> +    data: Type,
> +    child: Option<Type>,
> +    attributes: Vec<(Ident, LitInt)>,
> +}
> +
> +fn parse_attribute_field(stream: ParseStream<'_>) -> syn::Result<(Ident, LitInt)> {
> +    let id = stream.parse::<syn::Ident>()?;
> +    let _colon = stream.parse::<Token![:]>()?;
> +    let v = stream.parse::<LitInt>()?;
> +    Ok((id, v))
> +}
> +
> +fn parse_named_field(stream: ParseStream<'_>, name: &str) -> syn::Result<Type> {
> +    let name_id = stream.parse::<Ident>()?;
> +    if name_id != name {
> +        return Err(parse_field_error(name_id.span(), &name_id, name));
> +    }
> +    let _colon = stream.parse::<Token![:]>()?;
> +    let v = stream.parse::<Type>()?;
> +    stream.parse::<Token![,]>()?;
> +    Ok(v)
> +}
> +
> +fn parse_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
> +    let attribute_id = stream.parse::<Ident>()?;
> +    if attribute_id != "attributes" {
> +        return Err(syn::Error::new(
> +            attribute_id.span(),
> +            format!(
> +                "unexpected identifier: {}, expected: 'attributes'",
> +                attribute_id
> +            ),
> +        ));
> +    }
> +    stream.parse::<Token![:]>()?;
> +    let attr_stream;
> +    let _bracket = bracketed!(attr_stream in stream);
> +    let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
> +        &attr_stream,
> +        parse_attribute_field,
> +    )?;
> +    let attributes = attributes.into_iter().collect::<Vec<_>>();
> +
> +    stream.parse::<Option<Token![,]>>()?;
> +    Ok(attributes)
> +}
> +
> +fn parse_field_error(span: Span, name: &Ident, expected_name: &str) -> syn::Error {
> +    syn::Error::new(
> +        span,
> +        format!("Unexpected field name, got: {name} expected: {expected_name}"),
> +    )
> +}
> +
> +impl Parse for ConfigfsAttrs {
> +    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
> +        let container = try_parse(input, |s| parse_named_field(s, "container"))?;
> +        let data = try_parse(input, |s| parse_named_field(s, "data"))?;
> +        let child = try_parse(input, |s| parse_named_field(s, "child")).ok();
> +        let attributes = parse_attributes(input)?;

I have a `parse_ordered_fields!()` macro in module.rs, please extract it to
helpers and use it instead of implementing the parsing in an ad-hoc manner.

> +
> +        Ok(ConfigfsAttrs {
> +            container,
> +            data,
> +            child,
> +            attributes,
> +        })
> +    }
> +}
> +
> +fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
> +    let raw_id = quote! { #ty }.to_string();
> +
> +    // Sanitizing syn::Type::Path, this is safe since it is
> +    // only used as the identifier.
> +    let normalized = raw_id
> +        .split("::")
> +        .map(|s| String::from(s.trim()))
> +        .reduce(|a, b| format!("{a}_{b}"))
> +        .expect("Cannot be empty")
> +        .to_uppercase()
> +        .replace(|c: char| !c.is_alphanumeric(), "_");
> +
> +    Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
> +}
> +
> +pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
> +    let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
> +
> +    let data_tp_ident = make_static_ident(data_ty, "TPE");
> +    let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");

Instead of creating identifers like this, just scope them properly so that
a fixed identifier is used without colliding.

> +
> +    let n = cfs_attrs.attributes.len() + 1;
> +
> +    let attr_list = quote! {
> +        static #data_attr_ident: kernel::configfs::AttributeList<#n, #data_ty> =
> +            // SAFETY: We are expanding `configfs_attrs`.
> +            unsafe { kernel::configfs::AttributeList::new() };
> +    };
> +
> +    let mut attrs = Vec::new();
> +    for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
> +        let name_with_attr = make_static_ident(name, "ATTR");
> +
> +        let id: u64 = match id.base10_parse::<u64>() {
> +            Ok(v) => v,
> +            Err(_) => {
> +                return syn::Error::new(id.span(), "Could not parse attribute ID as a u64")
> +                    .to_compile_error();
> +            }
> +        };

Why parsing? The ID looks like it's just substituted as is.

Best,
Gary

> +
> +        attrs.push(quote! {
> +        static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
> +            // SAFETY: We are expanding `configfs_attrs`.
> +            unsafe {
> +              kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
> +            };
> +
> +          // 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 { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
> +        });
> +    }
> +
> +    let has_child_code = if let Some(child) = cfs_attrs.child {
> +        quote! { new_with_child_ctor::<#n, #child>}
> +    } else {
> +        quote! { new::<#n> }
> +    };
> +
> +    let data_type = quote! {
> +        {
> +            static #data_tp_ident:
> +            kernel::configfs::ItemType<#container_ty, #data_ty> =
> +                kernel::configfs::ItemType::<#container_ty, #data_ty>::#has_child_code(
> +                    &THIS_MODULE, &#data_attr_ident
> +                );
> +            &#data_tp_ident
> +        }
> +    };
> +
> +    quote! {
> +        {
> +            #attr_list
> +            #(#attrs)*
> +            #data_type
> +        }
> +    }
> +}
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index d18fbf4daa0a5..fdab8804e1ba9 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -4,6 +4,7 @@
>  use quote::ToTokens;
>  use syn::{
>      parse::{
> +        discouraged::Speculative,
>          Parse,
>          ParseStream, //
>      },
> @@ -54,6 +55,20 @@ pub(crate) fn file() -> String {
>      }
>  }
>  
> +pub(crate) fn try_parse<T>(
> +    input: ParseStream<'_>,
> +    parser: impl FnOnce(ParseStream<'_>) -> Result<T>,
> +) -> Result<T> {
> +    let fork = input.fork();
> +    match parser(&fork) {
> +        Ok(value) => {
> +            input.advance_to(&fork);
> +            Ok(value)
> +        }
> +        Err(e) => Err(e),
> +    }
> +}
> +
>  /// Obtain all `#[cfg]` attributes.
>  pub(crate) fn gather_cfg_attrs(attr: &[Attribute]) -> impl Iterator<Item = &Attribute> + '_ {
>      attr.iter().filter(|a| a.path().is_ident("cfg"))
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 2cfd59e0f9e7c..745ba83c828b9 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -15,6 +15,8 @@
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
>  
>  mod concat_idents;
> +#[cfg(CONFIG_CONFIGFS_FS)]
> +mod configfs_attrs;
>  mod export;
>  mod fmt;
>  mod helpers;
> @@ -489,3 +491,86 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream {
>          .unwrap_or_else(|e| e.into_compile_error())
>          .into()
>  }
> +
> +/// Define a list of configfs attributes statically.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// let item_type = configfs_attrs! {
> +///     container: configfs::Subsystem<Configuration>,
> +///     data: Configuration,
> +///     child: Child,
> +///     attributes: [
> +///         message: 0,
> +///         bar: 1,
> +///     ],
> +/// };
> +///```
> +///
> +/// Expands the following output:
> +///    let item_type = {
> +///         static CONFIGURATION_ATTR_LIST: kernel::configfs::AttributeList<
> +///             3usize,
> +///             Configuration,
> +///         > = unsafe { kernel::configfs::AttributeList::new() };
> +///         static MESSAGE_ATTR: kernel::configfs::Attribute<
> +///             0u64,
> +///             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(
> +///                             format_args!("string contains interior NUL"),
> +///                         );
> +///                     }
> +///                 };
> +///                 C
> +///             })
> +///         };
> +///         unsafe { CONFIGURATION_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR) }
> +///         static BAR_ATTR: kernel::configfs::Attribute<
> +///             1u64,
> +///             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(
> +///                             format_args!("string contains interior NUL"),
> +///                         );
> +///                     }
> +///                 };
> +///                 C
> +///             })
> +///         };
> +///         unsafe { CONFIGURATION_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR) }
> +///         {
> +///             static CONFIGURATION_TPE: kernel::configfs::ItemType<
> +///                 Subsystem<Configuration>,
> +///                 Configuration,
> +///             > = kernel::configfs::ItemType::<
> +///                 Subsystem<Configuration>,
> +///                 Configuration,
> +///             >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &CONFIGURATION_ATTR_LIST);
> +///             &CONFIGURATION_TPE
> +///         }
> +///     };
> +///
> +#[cfg(CONFIG_CONFIGFS_FS)]
> +#[proc_macro]
> +pub fn configfs_attrs(input: TokenStream) -> TokenStream {
> +    configfs_attrs::configfs_attrs(parse_macro_input!(input as configfs_attrs::ConfigfsAttrs))
> +        .into()
> +}
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> index a1bd9db6010da..876462f7789d1 100644
> --- a/samples/rust/rust_configfs.rs
> +++ b/samples/rust/rust_configfs.rs
> @@ -4,7 +4,7 @@
>  
>  use kernel::alloc::flags;
>  use kernel::configfs;
> -use kernel::configfs::configfs_attrs;
> +use kernel::macros::configfs_attrs;
>  use kernel::new_mutex;
>  use kernel::page::PAGE_SIZE;
>  use kernel::prelude::*;
>
> ---
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> change-id: 20260417-configfs-syn-191e07130027
>
> Best regards,



  reply	other threads:[~2026-06-03 14:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03 14:08 [PATCH v2] rust: add procedural macro for declaring configfs attributes Malte Wechter
2026-06-03 14:32 ` Gary Guo [this message]
2026-06-10  9:15   ` Malte Wechter
2026-06-10 10:05     ` Gary Guo
2026-06-10 14:48       ` Malte Wechter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DIZHKJSZXQ4Z.6SPXJVGDV9ZW@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=axboe@kernel.dk \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=maltewechter@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox