Defining configfs attributes in rust is a bit verbose at the moment. Add some macros to make the attribute definition less verbose. The configfs Rust abstractions should eventually provide procedural macros for this task. When we get more users of the configfs Rust abstractions, we shall consider this task. Signed-off-by: Andreas Hindborg --- drivers/block/rnull/configfs.rs | 134 +++++++++------------------------ drivers/block/rnull/configfs/macros.rs | 128 +++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 98 deletions(-) diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs index ea38b27a9011c..eafa71dfc40d3 100644 --- a/drivers/block/rnull/configfs.rs +++ b/drivers/block/rnull/configfs.rs @@ -1,20 +1,41 @@ // SPDX-License-Identifier: GPL-2.0 -use super::{NullBlkDevice, THIS_MODULE}; +use super::{ + NullBlkDevice, + THIS_MODULE, // +}; use kernel::{ - block::mq::gen_disk::{GenDisk, GenDiskBuilder}, + block::mq::gen_disk::{ + GenDisk, + GenDiskBuilder, // + }, c_str, - configfs::{self, AttributeOperations}, + configfs::{ + self, + AttributeOperations, // + }, configfs_attrs, - fmt::{self, Write as _}, + fmt::{ + self, + Write as _, // + }, new_mutex, page::PAGE_SIZE, prelude::*, - str::{kstrtobool_bytes, CString}, - sync::Mutex, + str::{ + kstrtobool_bytes, + CString, // + }, + sync::Mutex, // +}; +use macros::{ + configfs_simple_bool_field, + configfs_simple_field, // }; use pin_init::PinInit; +mod macros; + pub(crate) fn subsystem() -> impl PinInit, Error> { let item_type = configfs_attrs! { container: configfs::Subsystem, @@ -166,99 +187,16 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result { } } -#[vtable] -impl configfs::AttributeOperations<1> for DeviceConfig { - type Data = DeviceConfig; - - fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result { - let mut writer = kernel::str::Formatter::new(page); - writer.write_fmt(fmt!("{}\n", this.data.lock().block_size))?; - Ok(writer.bytes_written()) - } - - fn store(this: &DeviceConfig, page: &[u8]) -> Result { - if this.data.lock().powered { - return Err(EBUSY); - } - - let text = core::str::from_utf8(page)?.trim(); - let value = text.parse::().map_err(|_| EINVAL)?; - - GenDiskBuilder::validate_block_size(value)?; - this.data.lock().block_size = value; - Ok(()) - } -} - -#[vtable] -impl configfs::AttributeOperations<2> for DeviceConfig { - type Data = DeviceConfig; - - fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result { - let mut writer = kernel::str::Formatter::new(page); - - if this.data.lock().rotational { - writer.write_str("1\n")?; - } else { - writer.write_str("0\n")?; - } - - Ok(writer.bytes_written()) - } - - fn store(this: &DeviceConfig, page: &[u8]) -> Result { - if this.data.lock().powered { - return Err(EBUSY); - } - - this.data.lock().rotational = kstrtobool_bytes(page)?; - - Ok(()) - } -} - -#[vtable] -impl configfs::AttributeOperations<3> for DeviceConfig { - type Data = DeviceConfig; - - fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result { - let mut writer = kernel::str::Formatter::new(page); - writer.write_fmt(fmt!("{}\n", this.data.lock().capacity_mib))?; - Ok(writer.bytes_written()) - } - - fn store(this: &DeviceConfig, page: &[u8]) -> Result { - if this.data.lock().powered { - return Err(EBUSY); - } - - let text = core::str::from_utf8(page)?.trim(); - let value = text.parse::().map_err(|_| EINVAL)?; - - this.data.lock().capacity_mib = value; - Ok(()) - } -} - -#[vtable] -impl configfs::AttributeOperations<4> for DeviceConfig { - type Data = DeviceConfig; - - fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result { - let mut writer = kernel::str::Formatter::new(page); - writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?; - Ok(writer.bytes_written()) - } +configfs_simple_field!(DeviceConfig, 1, block_size, u32, check GenDiskBuilder::validate_block_size); +configfs_simple_bool_field!(DeviceConfig, 2, rotational); +configfs_simple_field!(DeviceConfig, 3, capacity_mib, u64); +configfs_simple_field!(DeviceConfig, 4, irq_mode, IRQMode); - fn store(this: &DeviceConfig, page: &[u8]) -> Result { - if this.data.lock().powered { - return Err(EBUSY); - } +impl core::str::FromStr for IRQMode { + type Err = Error; - let text = core::str::from_utf8(page)?.trim(); - let value = text.parse::().map_err(|_| EINVAL)?; - - this.data.lock().irq_mode = IRQMode::try_from(value)?; - Ok(()) + fn from_str(s: &str) -> Result { + let value: u8 = s.parse().map_err(|_| EINVAL)?; + value.try_into() } } diff --git a/drivers/block/rnull/configfs/macros.rs b/drivers/block/rnull/configfs/macros.rs new file mode 100644 index 0000000000000..53ce9d5dbdc8a --- /dev/null +++ b/drivers/block/rnull/configfs/macros.rs @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: GPL-2.0 + +use super::DeviceConfig; +use core::{fmt::Write, str::FromStr}; +use kernel::{page::PAGE_SIZE, prelude::*}; + +pub(crate) fn show_field(value: T, page: &mut [u8; PAGE_SIZE]) -> Result { + let mut writer = kernel::str::Formatter::new(page); + writer.write_fmt(fmt!("{}\n", value))?; + Ok(writer.bytes_written()) +} + +pub(crate) fn store_with_power_check(this: &DeviceConfig, page: &[u8], store_fn: F) -> Result +where + F: FnOnce(&DeviceConfig, &[u8]) -> Result, +{ + if this.data.lock().powered { + return Err(EBUSY); + } + store_fn(this, page) +} + +pub(crate) fn store_number_with_power_check( + this: &DeviceConfig, + page: &[u8], + store_fn: F, +) -> Result +where + F: FnOnce(&DeviceConfig, T) -> Result, + T: FromStr, +{ + if this.data.lock().powered { + return Err(EBUSY); + } + + let text = core::str::from_utf8(page)?.trim(); + let value = text.parse::().map_err(|_| EINVAL)?; + + store_fn(this, value) +} + +macro_rules! configfs_attribute { + ( + $type:ty, + $id:literal, + show: |$show_this:ident, $show_page:ident| $show_block:expr, + store: |$store_this:ident, $store_page:ident| $store_block:expr + $(,)? + ) => { + #[vtable] + impl configfs::AttributeOperations<$id> for $type { + type Data = $type; + + fn show($show_this: &$type, $show_page: &mut [u8; PAGE_SIZE]) -> Result { + $show_block + } + + fn store($store_this: &$type, $store_page: &[u8]) -> Result { + $store_block + } + } + }; +} +pub(crate) use configfs_attribute; + +// Specialized macro for simple boolean fields that just store kstrtobool_bytes result. +macro_rules! configfs_simple_bool_field { + ($type:ty, $id:literal, $field:ident) => { + crate::configfs::macros::configfs_attribute!($type, $id, + show: |this, page| crate::configfs::macros::show_field(this.data.lock().$field, page), + store: |this, page| + crate::configfs::macros::store_with_power_check(this, page, |this, page| { + this.data.lock().$field = kstrtobool_bytes(page)?; + Ok(()) + }) + ); + }; +} +pub(crate) use configfs_simple_bool_field; + +// Specialized macro for simple numeric fields that just parse and assign +macro_rules! configfs_simple_field { + // Simple direct assignment + ($type:ty, $id:literal, $field:ident, $field_type:ty) => { + crate::configfs::macros::configfs_attribute!($type, $id, + show: |this, page| crate::configfs::macros::show_field(this.data.lock().$field, page), + store: |this, page| crate::configfs::macros::store_number_with_power_check( + this, + page, + |this, value: $field_type| { + this.data.lock().$field = value; + Ok(()) + } + ) + ); + }; + // With infallible conversion expression (direct value) + ($type:ty, $id:literal, $field:ident, $field_type:ty, into $convert:expr) => { + crate::configfs::macros::configfs_attribute!($type, $id, + show: |this, page| + crate::configfs::macros::show_field(this.data.lock().$field, page), + store: |this, page| crate::configfs::macros::store_number_with_power_check( + this, + page, + |this, value: $field_type| { + this.data.lock().$field = $convert(value); + Ok(()) + } + ) + ); + }; + // With check, no conversion + ($type:ty, $id:literal, $field:ident, $field_type:ty, check $check:expr) => { + crate::configfs::macros::configfs_attribute!($type, $id, + show: |this, page| crate::configfs::macros::show_field(this.data.lock().$field, page), + store: |this, page| crate::configfs::macros::store_number_with_power_check( + this, + page, + |this, value: $field_type| { + $check(value)?; + this.data.lock().$field = value; + Ok(()) + } + ) + ); + }; +} +pub(crate) use configfs_simple_field; -- 2.51.2