* [PATCH v3] rust: add procedural macro for declaring configfs attributes
@ 2026-06-12 13:29 Malte Wechter
2026-06-12 13:41 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Malte Wechter @ 2026-06-12 13:29 UTC (permalink / raw)
To: Andreas Hindborg, Breno Leitao, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jens Axboe, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, Aaron Tomlin
Cc: linux-kernel, rust-for-linux, linux-block, linux-modules,
Malte Wechter
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 v3:
- Remove 'make_static_ident' function, make names for static variables simpler
- Move 'parse_ordered_fields' macro from module.rs into helpers
- Use 'parse_ordered_fields' macro for parsing instead of doing it ad-hoc
- Link to v2: https://lore.kernel.org/r/20260603-configfs-syn-v2-1-cb58489c2647@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 | 135 +++++++++++++++++++++
rust/macros/helpers.rs | 139 ++++++++++++++++++++++
rust/macros/lib.rs | 85 ++++++++++++++
rust/macros/module.rs | 137 ----------------------
samples/rust/rust_configfs.rs | 2 +-
8 files changed, 362 insertions(+), 390 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16..45f7a1ec93b4 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 7c2eb5c0b722..f28ec69d7984 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/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325..7a91e36677f5 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -791,254 +791,3 @@ 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: 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 >]
- .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(
- $crate::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 >]
- }
- }
- };
-
-}
-
-pub use crate::configfs_attrs;
diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
new file mode 100644
index 000000000000..81037bc38188
--- /dev/null
+++ b/rust/macros/configfs_attrs.rs
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use quote::{
+ format_ident,
+ quote, //
+};
+
+use syn::{
+ bracketed,
+ ext::IdentExt,
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ punctuated::Punctuated,
+ spanned::Spanned,
+ Error,
+ Ident,
+ LitInt,
+ Token,
+ Type, //
+};
+
+use crate::helpers::parse_ordered_fields;
+
+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_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
+ let attr_stream;
+ let _bracket = bracketed!(attr_stream in stream);
+ let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
+ &attr_stream,
+ parse_attribute_field,
+ )?;
+ Ok(attributes.into_iter().collect::<Vec<_>>())
+}
+
+impl Parse for ConfigfsAttrs {
+ fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+ parse_ordered_fields!(
+ from input;
+ container [required] => (input.parse::<Type>())?,
+ data [required] => (input.parse::<Type>())?,
+ child => (input.parse::<Type>())?,
+ attributes [required] => parse_attributes(input)?,
+ );
+
+ Ok(ConfigfsAttrs {
+ container,
+ data,
+ child,
+ attributes,
+ })
+ }
+}
+
+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 = Ident::new("DATA_TPE", cfs_attrs.data.span());
+ let data_attr_ident = Ident::new("DATA_ATTR_LIST", cfs_attrs.data.span());
+
+ 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 = format_ident!("{}_ATTR_{}", name.to_string().to_uppercase(), attr_idx);
+
+ 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();
+ }
+ };
+
+ 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 d18fbf4daa0a..df524749631a 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -58,3 +58,142 @@ pub(crate) fn file() -> String {
pub(crate) fn gather_cfg_attrs(attr: &[Attribute]) -> impl Iterator<Item = &Attribute> + '_ {
attr.iter().filter(|a| a.path().is_ident("cfg"))
}
+
+/// Parse fields that are required to use a specific order.
+///
+/// As fields must follow a specific order, we *could* just parse fields one by one by peeking.
+/// However the error message generated when implementing that way is not very friendly.
+///
+/// So instead we parse fields in an arbitrary order, but only enforce the ordering after parsing,
+/// and if the wrong order is used, the proper order is communicated to the user with error message.
+///
+/// Usage looks like this:
+/// ```ignore
+/// parse_ordered_fields! {
+/// from input;
+///
+/// // This will extract "foo: <field>" into a variable named "foo".
+/// // The variable will have type `Option<_>`.
+/// foo => <expression that parses the field>,
+///
+/// // If you need the variable name to be different than the key name.
+/// // This extracts "baz: <field>" into a variable named "bar".
+/// // You might want this if "baz" is a keyword.
+/// baz as bar => <expression that parse the field>,
+///
+/// // You can mark a key as required, and the variable will no longer be `Option`.
+/// // foobar will be of type `Expr` instead of `Option<Expr>`.
+/// foobar [required] => input.parse::<Expr>()?,
+/// }
+/// ```
+macro_rules! parse_ordered_fields {
+ (@gen
+ [$input:expr]
+ [$([$name:ident; $key:ident; $parser:expr])*]
+ [$([$req_name:ident; $req_key:ident])*]
+ ) => {
+ $(let mut $name = None;)*
+
+ const EXPECTED_KEYS: &[&str] = &[$(stringify!($key),)*];
+ const REQUIRED_KEYS: &[&str] = &[$(stringify!($req_key),)*];
+
+ let span = $input.span();
+ let mut seen_keys = Vec::new();
+
+ while !$input.is_empty() {
+ let key = $input.call(Ident::parse_any)?;
+
+ if seen_keys.contains(&key) {
+ Err(Error::new_spanned(
+ &key,
+ format!(r#"duplicated key "{key}". Keys can only be specified once."#),
+ ))?
+ }
+
+ $input.parse::<Token![:]>()?;
+
+ match &*key.to_string() {
+ $(
+ stringify!($key) => $name = Some($parser),
+ )*
+ _ => {
+ Err(Error::new_spanned(
+ &key,
+ format!(r#"unknown key "{key}". Valid keys are: {EXPECTED_KEYS:?}."#),
+ ))?
+ }
+ }
+
+ $input.parse::<Token![,]>()?;
+ seen_keys.push(key);
+ }
+
+ for key in REQUIRED_KEYS {
+ if !seen_keys.iter().any(|e| e == key) {
+ Err(Error::new(span, format!(r#"missing required key "{key}""#)))?
+ }
+ }
+
+ let mut ordered_keys: Vec<&str> = Vec::new();
+ for key in EXPECTED_KEYS {
+ if seen_keys.iter().any(|e| e == key) {
+ ordered_keys.push(key);
+ }
+ }
+
+ if seen_keys != ordered_keys {
+ Err(Error::new(
+ span,
+ format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
+ ))?
+ }
+
+ $(let $req_name = $req_name.expect("required field");)*
+ };
+
+ // Handle required fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)* [$name; $key]] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident [required] => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)* [$name; $name]] $($rest)*
+ )
+ };
+
+ // Handle optional fields.
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $key:ident as $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)*] $($rest)*
+ )
+ };
+ (@gen
+ [$input:expr] [$($tok:tt)*] [$($req:tt)*]
+ $name:ident => $parser:expr,
+ $($rest:tt)*
+ ) => {
+ parse_ordered_fields!(
+ @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)*] $($rest)*
+ )
+ };
+
+ (from $input:expr; $($tok:tt)*) => {
+ parse_ordered_fields!(@gen [$input] [] [] $($tok)*)
+ }
+}
+
+pub(crate) use parse_ordered_fields;
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 2cfd59e0f9e7..be04d94d0bc5 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 DATA_ATTR_LIST: kernel::configfs::AttributeList<
+/// 3usize,
+/// Configuration,
+/// > = unsafe { kernel::configfs::AttributeList::new() };
+/// static MESSAGE_ATTR_0: 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 { DATA_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR_0) }
+/// static BAR_ATTR_1: 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 { DATA_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR_1) }
+/// {
+/// static DATA_TPE: kernel::configfs::ItemType<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// > = kernel::configfs::ItemType::<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &DATA_ATTR_LIST);
+/// &DATA_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/rust/macros/module.rs b/rust/macros/module.rs
index 06c18e207508..7ff6ad09b1a2 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -196,143 +196,6 @@ fn param_ops_path(param_type: &str) -> Path {
}
}
-/// Parse fields that are required to use a specific order.
-///
-/// As fields must follow a specific order, we *could* just parse fields one by one by peeking.
-/// However the error message generated when implementing that way is not very friendly.
-///
-/// So instead we parse fields in an arbitrary order, but only enforce the ordering after parsing,
-/// and if the wrong order is used, the proper order is communicated to the user with error message.
-///
-/// Usage looks like this:
-/// ```ignore
-/// parse_ordered_fields! {
-/// from input;
-///
-/// // This will extract "foo: <field>" into a variable named "foo".
-/// // The variable will have type `Option<_>`.
-/// foo => <expression that parses the field>,
-///
-/// // If you need the variable name to be different than the key name.
-/// // This extracts "baz: <field>" into a variable named "bar".
-/// // You might want this if "baz" is a keyword.
-/// baz as bar => <expression that parse the field>,
-///
-/// // You can mark a key as required, and the variable will no longer be `Option`.
-/// // foobar will be of type `Expr` instead of `Option<Expr>`.
-/// foobar [required] => input.parse::<Expr>()?,
-/// }
-/// ```
-macro_rules! parse_ordered_fields {
- (@gen
- [$input:expr]
- [$([$name:ident; $key:ident; $parser:expr])*]
- [$([$req_name:ident; $req_key:ident])*]
- ) => {
- $(let mut $name = None;)*
-
- const EXPECTED_KEYS: &[&str] = &[$(stringify!($key),)*];
- const REQUIRED_KEYS: &[&str] = &[$(stringify!($req_key),)*];
-
- let span = $input.span();
- let mut seen_keys = Vec::new();
-
- while !$input.is_empty() {
- let key = $input.call(Ident::parse_any)?;
-
- if seen_keys.contains(&key) {
- Err(Error::new_spanned(
- &key,
- format!(r#"duplicated key "{key}". Keys can only be specified once."#),
- ))?
- }
-
- $input.parse::<Token![:]>()?;
-
- match &*key.to_string() {
- $(
- stringify!($key) => $name = Some($parser),
- )*
- _ => {
- Err(Error::new_spanned(
- &key,
- format!(r#"unknown key "{key}". Valid keys are: {EXPECTED_KEYS:?}."#),
- ))?
- }
- }
-
- $input.parse::<Token![,]>()?;
- seen_keys.push(key);
- }
-
- for key in REQUIRED_KEYS {
- if !seen_keys.iter().any(|e| e == key) {
- Err(Error::new(span, format!(r#"missing required key "{key}""#)))?
- }
- }
-
- let mut ordered_keys: Vec<&str> = Vec::new();
- for key in EXPECTED_KEYS {
- if seen_keys.iter().any(|e| e == key) {
- ordered_keys.push(key);
- }
- }
-
- if seen_keys != ordered_keys {
- Err(Error::new(
- span,
- format!(r#"keys are not ordered as expected. Order them like: {ordered_keys:?}."#),
- ))?
- }
-
- $(let $req_name = $req_name.expect("required field");)*
- };
-
- // Handle required fields.
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $key:ident as $name:ident [required] => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)* [$name; $key]] $($rest)*
- )
- };
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $name:ident [required] => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)* [$name; $name]] $($rest)*
- )
- };
-
- // Handle optional fields.
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $key:ident as $name:ident => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $key; $parser]] [$($req)*] $($rest)*
- )
- };
- (@gen
- [$input:expr] [$($tok:tt)*] [$($req:tt)*]
- $name:ident => $parser:expr,
- $($rest:tt)*
- ) => {
- parse_ordered_fields!(
- @gen [$input] [$($tok)* [$name; $name; $parser]] [$($req)*] $($rest)*
- )
- };
-
- (from $input:expr; $($tok:tt)*) => {
- parse_ordered_fields!(@gen [$input] [] [] $($tok)*)
- }
-}
-
struct Parameter {
name: Ident,
ptype: Ident,
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index a1bd9db6010d..876462f7789d 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,
--
Malte Wechter <maltewechter@gmail.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] rust: add procedural macro for declaring configfs attributes
2026-06-12 13:29 [PATCH v3] rust: add procedural macro for declaring configfs attributes Malte Wechter
@ 2026-06-12 13:41 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-12 13:41 UTC (permalink / raw)
To: Malte Wechter; +Cc: ojeda, linux-modules
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent execution of the generated `configfs_attrs!` block causes a data race on a shared `static` array.
--
commit d9a7547ab4c8957df6d5328eba0e3a712923192f
Author: Malte Wechter <maltewechter@gmail.com>
rust: add procedural macro for declaring configfs attributes
This commit replaces the old declarative configfs_attrs macro with a new
procedural macro to improve readability and maintainability.
> diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
> new file mode 100644
> index 0000000000000..81037bc38188b
> --- /dev/null
> +++ b/rust/macros/configfs_attrs.rs
[ ... ]
> + let mut attrs = Vec::new();
> + for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
> + let name_with_attr = format_ident!("{}_ATTR_{}", name.to_string().to_uppercase(), attr_idx);
[ ... ]
> + 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) }
[Severity: High]
This is a pre-existing issue, but does this generated code cause a data race
on the static attribute list array?
Because the generated attribute list is a static variable, its state is shared
across all invocations of the block. If the macro is used inside a callback
like make_group, every time a new directory is created via mkdir, the add
method is executed at runtime, mutating the globally shared UnsafeCell-backed
array via a raw pointer.
While make_group itself is serialized per-subsystem by su_mutex, concurrent
VFS reads on attributes of previously created directories do not hold this
mutex. Could make_group mutate the static array at the exact same time a
concurrent attribute read accesses it?
Could this array be fully initialized statically at compile time instead?
> + });
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-configfs-syn-v3-1-3292fbc5cc32@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-12 13:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 13:29 [PATCH v3] rust: add procedural macro for declaring configfs attributes Malte Wechter
2026-06-12 13:41 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox