All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] Rust device mapper abstractions
@ 2023-06-25 12:16 Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

Hello!

This is a version of device mapper abstractions. Based on
these, we also implement a linear target as a PoC.
Any suggestions are welcomed, thanks!

Additionally, there are some dummy codes used to wrap the block
layer structs, i.e., `bio` and `request`, which seems being
in the review process, so I just place it in the same file.

Qingsong Chen (8):
  rust: kernel: add basic abstractions for device-mapper
  rust: kernel: add request related TargetOperations
  rust: kernel: add some hook TargetOperations
  rust: kernel: add some info handler TargetOperations
  rust: kernel: add underlying device related TargetOperations
  rust: kernel: add DAX related TargetOperations
  rust: kernel: prepare to implement dm target in Rust
  samples: rust: add a device mapper linear target

 rust/bindings/bindings_helper.h |    2 +
 rust/helpers.c                  |    7 +
 rust/kernel/device_mapper.rs    | 1279 +++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |    1 +
 samples/rust/Kconfig            |   10 +
 samples/rust/Makefile           |    1 +
 samples/rust/rust_dm_linear.rs  |  257 +++++++
 7 files changed, 1557 insertions(+)
 create mode 100644 rust/kernel/device_mapper.rs
 create mode 100644 samples/rust/rust_dm_linear.rs

-- 
2.40.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 12:25   ` Greg KH
  2023-07-14 15:48   ` Benno Lossin
  2023-06-25 12:16 ` [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations Qingsong Chen
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Martin Rodriguez Reboredo, Alice Ryhl, Vincenzo Palazzo,
	Asahi Lina, rust-for-linux

- Declare `vtable` for basic target operations.
- Wrap `target_type` to register/unregister target.
- Wrap `dm_target`/`dm_dev` to handle io request.
- Add a dummy `bio` wrapper.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/device_mapper.rs    | 382 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 3 files changed, 384 insertions(+)
 create mode 100644 rust/kernel/device_mapper.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3e601ce2548d..807fc9cf41b8 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -11,6 +11,7 @@
 #include <linux/refcount.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
+#include <linux/device-mapper.h>
 
 /* `bindgen` gets confused at certain things. */
 const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
new file mode 100644
index 000000000000..28286b373b59
--- /dev/null
+++ b/rust/kernel/device_mapper.rs
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Device Mapper.
+//!
+//! C header: [`include/linux/device-mapper.h`](../../../../include/linux/device-mapper.h)
+
+use core::marker::PhantomData;
+use core::ops::Index;
+use core::ptr::{addr_of, NonNull};
+
+use crate::error::to_result;
+use crate::prelude::*;
+use crate::str::CStr;
+use crate::types::Opaque;
+
+/// Declare operations that a device mapper target can do.
+#[vtable]
+pub trait TargetOperations: Sized {
+    /// Persist user data.
+    type Private: Sync;
+
+    /// Constructor. The target will already have the table, type, begin and
+    /// len fields filled in. A `Private` struct can be returned to persist
+    /// its own context.
+    fn ctr(t: &mut Target<Self>, args: Args) -> Result<Box<Self::Private>>;
+
+    /// Destructor. The target could clean up anything hidden in `Private`,
+    /// and `Private` itself can be dropped automatically.
+    fn dtr(t: &mut Target<Self>);
+
+    /// Map block IOs. Return [`MapState`] to indicate how to handle the `bio`
+    /// later (end or resubmit).
+    fn map(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> MapState;
+}
+
+/// Wrap the kernel struct `target_type`.
+///
+/// It contains a struct `list_head` for internal device-mapper use, so it
+/// should be pinned. Users can use this struct to register/unregister their
+/// own device mapper target.
+#[pin_data(PinnedDrop)]
+pub struct TargetType {
+    #[pin]
+    opaque: Opaque<bindings::target_type>,
+}
+
+/// Define target feature type.
+pub type Feature = u64;
+
+// SAFETY: It's OK to access `TargetType` from multiple threads. The
+// `dm_register_target` and `dm_unregister_target` provides its own
+// synchronization.
+unsafe impl Sync for TargetType {}
+
+macro_rules! check_target_operations {
+    ($tt:expr, $(($op:ident, $filed:ident, $func:ident),)+) => {$(
+        if <T as TargetOperations>::$op {
+            (*$tt).$filed = Some(TargetType::$func::<T>);
+        }
+    )+};
+}
+
+impl TargetType {
+    /// Provide an in-place constructor to register a new device mapper target.
+    pub fn register<T: TargetOperations>(
+        module: &'static ThisModule,
+        name: &'static CStr,
+        version: [u32; 3],
+        feature: Feature,
+    ) -> impl PinInit<Self, Error> {
+        // SAFETY: `slot` is valid while the closure is called.
+        unsafe {
+            init::pin_init_from_closure(move |slot: *mut Self| {
+                // `slot` contains uninit memory, avoid creating a reference.
+                let opaque = addr_of!((*slot).opaque);
+                let tt = Opaque::raw_get(opaque);
+
+                (*tt).module = module.0;
+                (*tt).name = name.as_char_ptr();
+                (*tt).version = version;
+                (*tt).features = feature;
+
+                check_target_operations!(
+                    tt,
+                    (HAS_CTR, ctr, dm_ctr_fn),
+                    (HAS_DTR, dtr, dm_dtr_fn),
+                    (HAS_MAP, map, dm_map_fn),
+                );
+
+                to_result(bindings::dm_register_target(tt))
+            })
+        }
+    }
+}
+
+#[pinned_drop]
+impl PinnedDrop for TargetType {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: `self.opaque` are initialized by the `register` constructor,
+        // so it's valid.
+        unsafe {
+            bindings::dm_unregister_target(self.opaque.get());
+        }
+    }
+}
+
+impl TargetType {
+    unsafe extern "C" fn dm_ctr_fn<T: TargetOperations>(
+        target: *mut bindings::dm_target,
+        argc: core::ffi::c_uint,
+        argv: *mut *mut core::ffi::c_char,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel splits arguments by `dm_split_args`, then pass
+        // suitable `argc` and `argv` to `dm_ctr_fn`. If `argc` is not zero,
+        // `argv` is non-null and valid.
+        let args = unsafe { Args::new(argc, argv) };
+
+        // SAFETY: the kernel should pass a valid `dm_target`.
+        let t = unsafe { Target::borrow_mut(target) };
+        T::ctr(t, args).map_or_else(
+            |e| e.to_errno(),
+            // SAFETY: the kernel should pass a valid `dm_target`.
+            |p| unsafe {
+                (*target).private = Box::into_raw(p) as _;
+                0
+            },
+        )
+    }
+    unsafe extern "C" fn dm_dtr_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
+        // SAFETY: the kernel should pass a valid `dm_target`.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::dtr(t);
+        // SAFETY: `private` is constructed in `dm_ctr_fn`, and we drop it here.
+        unsafe {
+            let ptr = (*ti).private as *mut T::Private;
+            drop(Box::from_raw(ptr));
+            (*ti).private = core::ptr::null_mut();
+        }
+    }
+    unsafe extern "C" fn dm_map_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        bio: *mut bindings::bio,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass a valid `dm_target` and `bio`.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let bio = Bio::from_raw(bio);
+            T::map(t, bio) as _
+        }
+    }
+}
+
+/// Wrap the kernel struct `dm_target`.
+///
+/// This struct represents a device mapper target. And the device mapper
+/// core will alloc/free `dm_target` instances, so we just `borrow` it.
+/// It also holds a `Private` struct, which is used to persist user's data,
+/// and can be accessed by the `private` method.
+pub struct Target<T: TargetOperations + Sized> {
+    opaque: Opaque<bindings::dm_target>,
+    _p: PhantomData<*mut T::Private>,
+}
+
+impl<T: TargetOperations> Target<T> {
+    unsafe fn borrow<'a>(ptr: *const bindings::dm_target) -> &'a Self {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { &*(ptr as *const Self) }
+    }
+
+    unsafe fn borrow_mut<'a>(ptr: *mut bindings::dm_target) -> &'a mut Self {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { &mut *(ptr as *mut Self) }
+    }
+
+    /// Access user's private data.
+    pub fn private(&mut self) -> Option<Pin<&mut T::Private>> {
+        let t = self.opaque.get();
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        // And 'p' is non-null and assigned in `dm_ctr_fn`, so it's valid.
+        unsafe {
+            ((*t).private as *mut T::Private)
+                .as_mut()
+                .map(|p| Pin::new_unchecked(p))
+        }
+    }
+
+    /// Return the target name.
+    pub fn name(&self) -> &CStr {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            let name = (*(*self.opaque.get()).type_).name;
+            CStr::from_char_ptr(name)
+        }
+    }
+
+    /// Return the target version.
+    pub fn version(&self) -> [u32; 3] {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*(*self.opaque.get()).type_).version }
+    }
+
+    /// Return the target begin sector.
+    pub fn begin_sector(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).begin as _ }
+    }
+
+    /// Return the target total sectors.
+    pub fn total_sectors(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).len as _ }
+    }
+
+    /// Get the underlying device by `path`. The `dm_put_device` will be called when
+    /// [`TargetDevice`] is dropped.
+    pub fn get_device(&mut self, path: &CStr) -> Result<TargetDevice<T>> {
+        let mut dd = core::ptr::null_mut();
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            let mode = bindings::dm_table_get_mode((*self.opaque.get()).table);
+            match bindings::dm_get_device(self.opaque.get(), path.as_char_ptr(), mode, &mut dd) {
+                0 => {
+                    let ti = self.opaque.get();
+                    Ok(TargetDevice::from_raw(ti, dd))
+                }
+                err => Err(Error::from_errno(err)),
+            }
+        }
+    }
+}
+
+/// Represent an underlying device of a device mapper target.
+///
+/// We also holds a pointer to `dm_target`, so that we can call
+/// `dm_put_device` in `drop`, to close the device correctly.
+pub struct TargetDevice<T: TargetOperations + Sized> {
+    ti: NonNull<bindings::dm_target>,
+    dev: NonNull<bindings::dm_dev>,
+    _p: PhantomData<*mut Target<T>>,
+}
+
+impl<T: TargetOperations> TargetDevice<T> {
+    unsafe fn from_raw(ti: *mut bindings::dm_target, dd: *mut bindings::dm_dev) -> Self {
+        // SAFETY: the caller should pass valid `dm_target` and `dm_dev`.
+        unsafe {
+            Self {
+                ti: NonNull::new_unchecked(ti),
+                dev: NonNull::new_unchecked(dd),
+                _p: PhantomData,
+            }
+        }
+    }
+
+    /// Borrow the device mapper target.
+    pub fn target(&self) -> &Target<T> {
+        // SAFETY: the `from_raw` caller should pass valid `ti` pointer.
+        unsafe { Target::borrow(self.ti.as_ptr()) }
+    }
+
+    /// Borrow the device mapper target mutably.
+    pub fn mut_target(&mut self) -> &mut Target<T> {
+        // SAFETY: the `from_raw` caller should pass valid `ti` pointer.
+        unsafe { Target::borrow_mut(self.ti.as_ptr()) }
+    }
+
+    /// Return the device name.
+    pub fn device_name(&self) -> &CStr {
+        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
+        unsafe {
+            let name = (*self.dev.as_ptr()).name.as_ptr();
+            CStr::from_char_ptr(name)
+        }
+    }
+
+    /// Return the total device sectors.
+    pub fn device_sectors(&self) -> usize {
+        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
+        unsafe { (*(*self.dev.as_ptr()).bdev).bd_nr_sectors as _ }
+    }
+}
+
+impl<T: TargetOperations> Drop for TargetDevice<T> {
+    fn drop(&mut self) {
+        // SAFETY: the `from_raw` caller should pass valid `ti` and `dev`.
+        unsafe { bindings::dm_put_device(self.ti.as_ptr(), self.dev.as_ptr()) }
+    }
+}
+
+/// The return values of target map function, i.e., [`TargetOperations::map`].
+#[repr(u32)]
+pub enum MapState {
+    /// The target will handle the io by resubmitting it later.
+    Submitted = bindings::DM_MAPIO_SUBMITTED,
+
+    /// Simple remap complete.
+    Remapped = bindings::DM_MAPIO_REMAPPED,
+
+    /// The target wants to requeue the io.
+    Requeue = bindings::DM_MAPIO_REQUEUE,
+
+    /// The target wants to requeue the io after a delay.
+    DelayRequeue = bindings::DM_MAPIO_DELAY_REQUEUE,
+
+    /// The target wants to complete the io.
+    Kill = bindings::DM_MAPIO_KILL,
+}
+
+/// Wrap the `c_char` arguments, which yields `CStr`.
+pub struct Args {
+    argc: core::ffi::c_uint,
+    argv: *mut *mut core::ffi::c_char,
+}
+
+impl Args {
+    /// The caller should ensure that the number of valid `argv` pointers
+    /// should be exactly `argc`.
+    unsafe fn new(argc: core::ffi::c_uint, argv: *mut *mut core::ffi::c_char) -> Self {
+        Self { argc, argv }
+    }
+
+    /// Return the number of arguments.
+    pub fn len(&self) -> usize {
+        self.argc as _
+    }
+
+    /// Return the `nth` (from zero) argument.
+    ///
+    /// If the index is out of bounds, return `None`.
+    pub fn get(&self, index: usize) -> Option<&CStr> {
+        if self.argc == 0 || index >= self.argc as _ {
+            None
+        } else {
+            // SAFETY: the `new` caller should ensure the number of valid `argv`.
+            unsafe { Some(CStr::from_char_ptr(*self.argv.add(index))) }
+        }
+    }
+}
+
+impl Index<usize> for Args {
+    type Output = CStr;
+
+    /// When using the indexing operator(`[]`), the caller should check the
+    /// length of [`Args`]. If the index is out of bounds, this will [`panic`].
+    fn index(&self, index: usize) -> &Self::Output {
+        if self.argc == 0 || index >= self.argc as _ {
+            panic!(
+                "Index out of bounds: the length is {} but the index is {}.",
+                self.argc, index
+            )
+        } else {
+            // SAFETY: the `new` caller should ensure the number of valid `argv`.
+            unsafe { CStr::from_char_ptr(*self.argv.add(index)) }
+        }
+    }
+}
+
+/// Wrap the kernel struct `bio`.
+///
+/// Dummy.
+#[pin_data]
+pub struct Bio {
+    #[pin]
+    opaque: Opaque<bindings::bio>,
+}
+
+impl Bio {
+    unsafe fn from_raw<'a>(ptr: *mut bindings::bio) -> Pin<&'a mut Self> {
+        // SAFETY: the caller should pass a valid `bio` pointer.
+        unsafe { Pin::new(&mut *(ptr as *mut Self)) }
+    }
+
+    fn op_and_flags(&self) -> u32 {
+        // SAFETY: the `from_raw` caller should pass valid `bio`, so
+        // `self.opaque` is valid too.
+        unsafe { (*self.opaque.get()).bi_opf }
+    }
+
+    /// Return `true` if the bio request is write.
+    pub fn is_write(&self) -> bool {
+        self.op_and_flags() & bindings::req_op_REQ_OP_WRITE != 0
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 85b261209977..3dc19f4642f0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -31,6 +31,7 @@
 #[cfg(not(testlib))]
 mod allocator;
 mod build_assert;
+pub mod device_mapper;
 pub mod error;
 pub mod init;
 pub mod ioctl;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-07-14 15:49   ` Benno Lossin
  2023-06-25 12:16 ` [RFC PATCH 3/8] rust: kernel: add some hook TargetOperations Qingsong Chen
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

- Add `request` related and end_io TargetOperations.
- Add a dummy `request` wrapper and `blk_status_t`.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/kernel/device_mapper.rs | 199 +++++++++++++++++++++++++++++++++++
 1 file changed, 199 insertions(+)

diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index 28286b373b59..153bae3aad79 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -31,6 +31,39 @@ pub trait TargetOperations: Sized {
     /// Map block IOs. Return [`MapState`] to indicate how to handle the `bio`
     /// later (end or resubmit).
     fn map(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> MapState;
+
+    /// Map `request`. Return [`MapState`] and the optional cloned `request`.
+    #[allow(unused)]
+    fn clone_and_map_rq<'a>(
+        t: &mut Target<Self>,
+        rq: Pin<&mut Request>,
+        map_ctx: &mut MapInfo,
+    ) -> (MapState, Option<Pin<&'a mut Request>>) {
+        unimplemented!()
+    }
+
+    /// Release the cloned `request`.
+    #[allow(unused)]
+    fn release_clone_rq(rq: Pin<&mut Request>, map_ctx: &mut MapInfo) {
+        unimplemented!()
+    }
+
+    /// End the `bio`. Return [`EndState`] and [`BlkStatus`].
+    #[allow(unused)]
+    fn end_io(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> (EndState, BlkStatus) {
+        unimplemented!()
+    }
+
+    /// End the `request`. Return [`EndState`].
+    #[allow(unused)]
+    fn rq_end_io(
+        t: &mut Target<Self>,
+        rq: Pin<&mut Request>,
+        map_ctx: &mut MapInfo,
+        err: BlkStatus,
+    ) -> EndState {
+        unimplemented!()
+    }
 }
 
 /// Wrap the kernel struct `target_type`.
@@ -85,6 +118,18 @@ pub fn register<T: TargetOperations>(
                     (HAS_CTR, ctr, dm_ctr_fn),
                     (HAS_DTR, dtr, dm_dtr_fn),
                     (HAS_MAP, map, dm_map_fn),
+                    (
+                        HAS_CLONE_AND_MAP_RQ,
+                        clone_and_map_rq,
+                        dm_clone_and_map_request_fn
+                    ),
+                    (
+                        HAS_RELEASE_CLONE_RQ,
+                        release_clone_rq,
+                        dm_release_clone_request_fn
+                    ),
+                    (HAS_END_IO, end_io, dm_endio_fn),
+                    (HAS_RQ_END_IO, rq_end_io, dm_request_endio_fn),
                 );
 
                 to_result(bindings::dm_register_target(tt))
@@ -148,6 +193,64 @@ impl TargetType {
             T::map(t, bio) as _
         }
     }
+    unsafe extern "C" fn dm_clone_and_map_request_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        rq: *mut bindings::request,
+        map_context: *mut bindings::map_info,
+        clone: *mut *mut bindings::request,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target`, `request`
+        // and `map_info` pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let rq = Request::from_raw(rq);
+            let map_ctx = MapInfo::borrow_mut(map_context);
+            let (map_state, rq_cloned) = T::clone_and_map_rq(t, rq, map_ctx);
+            *clone = rq_cloned.map_or(core::ptr::null_mut(), |rq| rq.opaque.get());
+            map_state as _
+        }
+    }
+    unsafe extern "C" fn dm_release_clone_request_fn<T: TargetOperations>(
+        clone: *mut bindings::request,
+        map_context: *mut bindings::map_info,
+    ) {
+        // SAFETY: the kernel should pass valid `request` and `map_info` pointers.
+        unsafe {
+            let rq = Request::from_raw(clone);
+            let map_ctx = MapInfo::borrow_mut(map_context);
+            T::release_clone_rq(rq, map_ctx);
+        }
+    }
+    unsafe extern "C" fn dm_endio_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        bio: *mut bindings::bio,
+        error: *mut bindings::blk_status_t,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target`, `bio` and
+        // `error` pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let bio = Bio::from_raw(bio);
+            let (end_state, blk_status) = T::end_io(t, bio);
+            *error = blk_status as _;
+            end_state as _
+        }
+    }
+    unsafe extern "C" fn dm_request_endio_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        clone: *mut bindings::request,
+        error: bindings::blk_status_t,
+        map_context: *mut bindings::map_info,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target`, `request`
+        // and `map_info` pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let rq = Request::from_raw(clone);
+            let map_ctx = MapInfo::borrow_mut(map_context);
+            T::rq_end_io(t, rq, map_ctx, (error as u32).into()) as _
+        }
+    }
 }
 
 /// Wrap the kernel struct `dm_target`.
@@ -305,6 +408,33 @@ pub enum MapState {
     Kill = bindings::DM_MAPIO_KILL,
 }
 
+/// The return values of target end_io function.
+#[repr(u32)]
+pub enum EndState {
+    /// Ended successfully.
+    Done = bindings::DM_ENDIO_DONE,
+
+    /// The io has still not completed (eg, multipath target might
+    /// want to requeue a failed io).
+    Incomplete = bindings::DM_ENDIO_INCOMPLETE,
+
+    /// The target wants to requeue the io.
+    Requeue = bindings::DM_ENDIO_REQUEUE,
+
+    /// The target wants to requeue the io after a delay.
+    DelayRequeue = bindings::DM_ENDIO_DELAY_REQUEUE,
+}
+
+/// Wrap the c struct `map_info`.
+pub struct MapInfo(Opaque<bindings::map_info>);
+
+impl MapInfo {
+    unsafe fn borrow_mut<'a>(ptr: *mut bindings::map_info) -> &'a mut Self {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { &mut *(ptr as *mut Self) }
+    }
+}
+
 /// Wrap the `c_char` arguments, which yields `CStr`.
 pub struct Args {
     argc: core::ffi::c_uint,
@@ -380,3 +510,72 @@ pub fn is_write(&self) -> bool {
         self.op_and_flags() & bindings::req_op_REQ_OP_WRITE != 0
     }
 }
+
+/// Wrap the kernel struct `request`.
+///
+/// Dummy.
+#[pin_data]
+pub struct Request {
+    #[pin]
+    opaque: Opaque<bindings::request>,
+}
+
+impl Request {
+    unsafe fn from_raw<'a>(ptr: *mut bindings::request) -> Pin<&'a mut Self> {
+        // SAFETY: the caller should pass a valid `request` pointer.
+        unsafe { Pin::new_unchecked(&mut *(ptr as *mut Self)) }
+    }
+}
+
+/// Wrap the block error status values (see [blk_status_t]).
+///
+/// [`blk_status_t`]: ../../../../include/linux/blk_types.h
+#[repr(u32)]
+#[allow(missing_docs)]
+pub enum BlkStatus {
+    Ok,
+    NotSupp,
+    TimeOut,
+    NoSpc,
+    Transport,
+    Target,
+    Nexus,
+    Medium,
+    Protection,
+    Resource,
+    IoErr,
+    DmRequeue,
+    Again,
+    DevResource,
+    ZoneResource,
+    ZoneOpenResource,
+    ZoneActiveResource,
+    Offline,
+    Undefined,
+}
+
+impl From<u32> for BlkStatus {
+    fn from(value: u32) -> Self {
+        match value {
+            0 => Self::Ok,
+            1 => Self::NotSupp,
+            2 => Self::TimeOut,
+            3 => Self::NoSpc,
+            4 => Self::Transport,
+            5 => Self::Target,
+            6 => Self::Nexus,
+            7 => Self::Medium,
+            8 => Self::Protection,
+            9 => Self::Resource,
+            10 => Self::IoErr,
+            11 => Self::DmRequeue,
+            12 => Self::Again,
+            13 => Self::DevResource,
+            14 => Self::ZoneResource,
+            15 => Self::ZoneOpenResource,
+            16 => Self::ZoneActiveResource,
+            17 => Self::Offline,
+            _ => Self::Undefined,
+        }
+    }
+}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 3/8] rust: kernel: add some hook TargetOperations
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations Qingsong Chen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

- Add hooks when suspend or resume a target.
- Add operation to check if a target is busy.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/kernel/device_mapper.rs | 76 ++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index 153bae3aad79..aeda12245171 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -64,6 +64,42 @@ fn rq_end_io(
     ) -> EndState {
         unimplemented!()
     }
+
+    /// Hook presuspend.
+    #[allow(unused)]
+    fn presuspend(t: &mut Target<Self>) {
+        unimplemented!()
+    }
+
+    /// Hook presuspend_undo.
+    #[allow(unused)]
+    fn presuspend_undo(t: &mut Target<Self>) {
+        unimplemented!()
+    }
+
+    /// Hook postsuspend.
+    #[allow(unused)]
+    fn postsuspend(t: &mut Target<Self>) {
+        unimplemented!()
+    }
+
+    /// Hook preresume.
+    #[allow(unused)]
+    fn preresume(t: &mut Target<Self>) -> Result {
+        unimplemented!()
+    }
+
+    /// Hook resume.
+    #[allow(unused)]
+    fn resume(t: &mut Target<Self>) {
+        unimplemented!()
+    }
+
+    /// Check if target is busy.
+    #[allow(unused)]
+    fn busy(t: &mut Target<Self>) -> bool {
+        unimplemented!()
+    }
 }
 
 /// Wrap the kernel struct `target_type`.
@@ -130,6 +166,12 @@ pub fn register<T: TargetOperations>(
                     ),
                     (HAS_END_IO, end_io, dm_endio_fn),
                     (HAS_RQ_END_IO, rq_end_io, dm_request_endio_fn),
+                    (HAS_PRESUSPEND, presuspend, dm_presuspend_fn),
+                    (HAS_PRESUSPEND_UNDO, presuspend_undo, dm_presuspend_undo_fn),
+                    (HAS_POSTSUSPEND, postsuspend, dm_postsuspend_fn),
+                    (HAS_PRERESUME, preresume, dm_preresume_fn),
+                    (HAS_RESUME, resume, dm_resume_fn),
+                    (HAS_BUSY, busy, dm_busy_fn),
                 );
 
                 to_result(bindings::dm_register_target(tt))
@@ -251,6 +293,40 @@ impl TargetType {
             T::rq_end_io(t, rq, map_ctx, (error as u32).into()) as _
         }
     }
+    unsafe extern "C" fn dm_presuspend_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::presuspend(t);
+    }
+    unsafe extern "C" fn dm_presuspend_undo_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::presuspend_undo(t);
+    }
+    unsafe extern "C" fn dm_postsuspend_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::postsuspend(t);
+    }
+    unsafe extern "C" fn dm_preresume_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::preresume(t).map_or_else(|e| e.to_errno(), |_| 0)
+    }
+    unsafe extern "C" fn dm_resume_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::resume(t);
+    }
+    unsafe extern "C" fn dm_busy_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::busy(t) as _
+    }
 }
 
 /// Wrap the kernel struct `dm_target`.
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
                   ` (2 preceding siblings ...)
  2023-06-25 12:16 ` [RFC PATCH 3/8] rust: kernel: add some hook TargetOperations Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 16:58   ` kernel test robot
  2023-06-25 12:16 ` [RFC PATCH 5/8] rust: kernel: add underlying device related TargetOperations Qingsong Chen
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

- Add `status`/`message` to retrieve info of targets.
- Add `report_zones` to propagate report zones command
  to underlying devices.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/kernel/device_mapper.rs | 149 ++++++++++++++++++++++++++++++++++-
 1 file changed, 148 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index aeda12245171..9a62208fda56 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -10,7 +10,7 @@
 
 use crate::error::to_result;
 use crate::prelude::*;
-use crate::str::CStr;
+use crate::str::{CStr, CString};
 use crate::types::Opaque;
 
 /// Declare operations that a device mapper target can do.
@@ -100,6 +100,24 @@ fn resume(t: &mut Target<Self>) {
     fn busy(t: &mut Target<Self>) -> bool {
         unimplemented!()
     }
+
+    /// Return the target status.
+    #[allow(unused)]
+    fn status(t: &mut Target<Self>, type_: StatusType, flags: StatusFlags) -> Option<CString> {
+        unimplemented!()
+    }
+
+    /// Handle messages to the target.
+    #[allow(unused)]
+    fn message(t: &mut Target<Self>, args: Args) -> Result<CString> {
+        unimplemented!()
+    }
+
+    /// Propagate report zones command to underlying devices.
+    #[allow(unused)]
+    fn report_zones(t: &mut Target<Self>, args: &mut [ReportZonesArgs]) -> Result {
+        unimplemented!()
+    }
 }
 
 /// Wrap the kernel struct `target_type`.
@@ -172,6 +190,9 @@ pub fn register<T: TargetOperations>(
                     (HAS_PRERESUME, preresume, dm_preresume_fn),
                     (HAS_RESUME, resume, dm_resume_fn),
                     (HAS_BUSY, busy, dm_busy_fn),
+                    (HAS_STATUS, status, dm_status_fn),
+                    (HAS_MESSAGE, message, dm_message_fn),
+                    (HAS_REPORT_ZONES, report_zones, dm_report_zones_fn),
                 );
 
                 to_result(bindings::dm_register_target(tt))
@@ -327,6 +348,69 @@ impl TargetType {
         let t = unsafe { Target::borrow_mut(ti) };
         T::busy(t) as _
     }
+    unsafe extern "C" fn dm_status_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        status_type: bindings::status_type_t,
+        status_flags: core::ffi::c_uint,
+        result: *mut core::ffi::c_char,
+        maxlen: core::ffi::c_uint,
+    ) {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        if let Some(s) = T::status(t, status_type.into(), status_flags.into()) {
+            let count = s.len_with_nul().min(maxlen as _);
+            // SAFETY: the kernel should pass valid `result` pointer, and
+            // `count` is not greater than `maxlen`.
+            unsafe {
+                core::ptr::copy(s.as_char_ptr(), result, count);
+            }
+        }
+    }
+    unsafe extern "C" fn dm_message_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        argc: core::ffi::c_uint,
+        argv: *mut *mut core::ffi::c_char,
+        result: *mut core::ffi::c_char,
+        maxlen: core::ffi::c_uint,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel splits arguments by `dm_split_args`, then pass
+        // suitable `argc` and `argv` to `dm_ctr_fn`. If `argc` is not zero,
+        // `argv` is non-null and valid.
+        let args = unsafe { Args::new(argc, argv) };
+
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        T::message(t, args).map_or_else(
+            |e| e.to_errno(),
+            // SAFETY: the kernel should pass valid `result` pointer, and
+            // `count` is not greater than `maxlen`.
+            |ret| unsafe {
+                let count = ret.len_with_nul().min(maxlen as _);
+                core::ptr::copy(ret.as_char_ptr(), result, count);
+                0
+            },
+        )
+    }
+    unsafe extern "C" fn dm_report_zones_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        args: *mut bindings::dm_report_zones_args,
+        nr_zones: core::ffi::c_uint,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+
+        let args = if nr_zones > 0 {
+            // SAFETY: the kernel should pass valid `args`, if `nr_zones`
+            // is not zero.
+            unsafe {
+                let first = ReportZonesArgs::borrow_mut(args);
+                core::slice::from_raw_parts_mut(first as _, nr_zones as _)
+            }
+        } else {
+            &mut []
+        };
+        T::report_zones(t, args).map_or_else(|e| e.to_errno(), |_| 0)
+    }
 }
 
 /// Wrap the kernel struct `dm_target`.
@@ -511,6 +595,69 @@ unsafe fn borrow_mut<'a>(ptr: *mut bindings::map_info) -> &'a mut Self {
     }
 }
 
+/// Define target status types.
+#[repr(u32)]
+pub enum StatusType {
+    /// Request for some general info.
+    Info,
+
+    /// Request for more detailed info.
+    Table,
+
+    /// Request for some integrity info.
+    Ima,
+
+    /// Undefined.
+    Undefined,
+}
+
+impl From<u32> for StatusType {
+    fn from(value: u32) -> Self {
+        match value {
+            bindings::status_type_t_STATUSTYPE_INFO => Self::Info,
+            bindings::status_type_t_STATUSTYPE_TABLE => Self::Table,
+            bindings::status_type_t_STATUSTYPE_IMA => Self::Ima,
+            _ => Self::Undefined,
+        }
+    }
+}
+
+/// Define target status flags.
+#[repr(u32)]
+pub enum StatusFlags {
+    /// See `DM_STATUS_NOFLUSH_FLAG`.
+    NoFlush = 1 << 0,
+
+    /// Undefined.
+    Undefined,
+}
+
+impl From<u32> for StatusFlags {
+    fn from(value: u32) -> Self {
+        match value {
+            0 => Self::NoFlush,
+            _ => Self::Undefined,
+        }
+    }
+}
+
+/// Wrap the kernel struct `dm_report_zones_args`.
+pub struct ReportZonesArgs(Opaque<bindings::dm_report_zones_args>);
+
+impl ReportZonesArgs {
+    unsafe fn borrow_mut<'a>(ptr: *mut bindings::dm_report_zones_args) -> &'a mut Self {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { &mut *(ptr as *mut Self) }
+    }
+
+    /// Return the next sector.
+    pub fn next_sector(&self) -> usize {
+        // SAFETY: `self.0` is borrowed form foreign pointer,
+        // should be valid.
+        unsafe { (*self.0.get()).next_sector as _ }
+    }
+}
+
 /// Wrap the `c_char` arguments, which yields `CStr`.
 pub struct Args {
     argc: core::ffi::c_uint,
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 5/8] rust: kernel: add underlying device related TargetOperations
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
                   ` (3 preceding siblings ...)
  2023-06-25 12:16 ` [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 6/8] rust: kernel: add DAX " Qingsong Chen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

- Add `prepare_ioctl` to pass ioctls to underlying device.
- Add `iterate_devices` to iterate underlying devices.
- Add `io_hints` to setup target request queue limits.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/kernel/device_mapper.rs | 101 +++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index 9a62208fda56..ba13294f2d0b 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -118,6 +118,24 @@ fn message(t: &mut Target<Self>, args: Args) -> Result<CString> {
     fn report_zones(t: &mut Target<Self>, args: &mut [ReportZonesArgs]) -> Result {
         unimplemented!()
     }
+
+    /// Pass on ioctl to the underlying device.
+    #[allow(unused)]
+    fn prepare_ioctl(t: &mut Target<Self>) -> (i32, Option<NonNull<TargetDevice<Self>>>) {
+        unimplemented!()
+    }
+
+    /// Iterate the underlying devices.
+    #[allow(unused)]
+    fn iterate_devices(t: &mut Target<Self>) -> Result<Box<dyn Iterator<Item = IterDevice<Self>>>> {
+        unimplemented!()
+    }
+
+    /// Setup target request queue limits.
+    #[allow(unused)]
+    fn io_hints(t: &mut Target<Self>, limits: &mut QueueLimits) {
+        unimplemented!()
+    }
 }
 
 /// Wrap the kernel struct `target_type`.
@@ -193,6 +211,9 @@ pub fn register<T: TargetOperations>(
                     (HAS_STATUS, status, dm_status_fn),
                     (HAS_MESSAGE, message, dm_message_fn),
                     (HAS_REPORT_ZONES, report_zones, dm_report_zones_fn),
+                    (HAS_PREPARE_IOCTL, prepare_ioctl, dm_prepare_ioctl_fn),
+                    (HAS_ITERATE_DEVICES, iterate_devices, dm_iterate_devices_fn),
+                    (HAS_IO_HINTS, io_hints, dm_io_hints_fn),
                 );
 
                 to_result(bindings::dm_register_target(tt))
@@ -411,6 +432,68 @@ impl TargetType {
         };
         T::report_zones(t, args).map_or_else(|e| e.to_errno(), |_| 0)
     }
+    unsafe extern "C" fn dm_prepare_ioctl_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        bdev: *mut *mut bindings::block_device,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+
+        match T::prepare_ioctl(t) {
+            (err, None) => err,
+            // SAFETY: `td` and `dev` is `NonNull`, both of them are valid.
+            (ret, Some(td)) => unsafe {
+                let dm_dev = td.as_ref().dev.as_ptr();
+                let block_dev = (*dm_dev).bdev;
+                *bdev = block_dev;
+                ret
+            },
+        }
+    }
+    unsafe extern "C" fn dm_iterate_devices_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        fn_: bindings::iterate_devices_callout_fn,
+        data: *mut core::ffi::c_void,
+    ) -> core::ffi::c_int {
+        let Some(fn_) = fn_ else {
+            pr_warn!("Invalid iterate_devices_callout_fn, skipped!");
+            return 0;
+        };
+
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        let t = unsafe { Target::borrow_mut(ti) };
+        let devices = match T::iterate_devices(t) {
+            Err(e) => return e.to_errno(),
+            Ok(devices) => devices,
+        };
+
+        let mut ret = 0;
+        for (target_device, start, len) in devices {
+            // SAFETY: `target_device` is `NonNull`, it is also valid.
+            // `fn_` is checked above, it is non-null, and the kernel
+            // should pass valid `iterate_devices_callout_fn`.
+            unsafe {
+                let dev = target_device.as_ref().dev.as_ptr();
+                ret = fn_(ti, dev, start as _, len as _, data);
+                if ret != 0 {
+                    break;
+                }
+            }
+        }
+        ret
+    }
+    unsafe extern "C" fn dm_io_hints_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        limits: *mut bindings::queue_limits,
+    ) {
+        // SAFETY: the kernel should pass valid `dm_target` and `queue_limits`
+        // pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let limits = QueueLimits::borrow_mut(limits);
+            T::io_hints(t, limits);
+        }
+    }
 }
 
 /// Wrap the kernel struct `dm_target`.
@@ -549,6 +632,12 @@ fn drop(&mut self) {
     }
 }
 
+/// Gather info about underlying device of target.
+///
+/// The first is the [`TargetDevice`], the second is the start sector of
+/// the device, and the third is the length (in sectors) of the device.
+pub type IterDevice<T> = (NonNull<TargetDevice<T>>, usize, usize);
+
 /// The return values of target map function, i.e., [`TargetOperations::map`].
 #[repr(u32)]
 pub enum MapState {
@@ -802,3 +891,15 @@ fn from(value: u32) -> Self {
         }
     }
 }
+
+/// Wrap the kernel struct `queue_limits`.
+///
+/// Dummy.
+pub struct QueueLimits(Opaque<bindings::queue_limits>);
+
+impl QueueLimits {
+    unsafe fn borrow_mut<'a>(ptr: *mut bindings::queue_limits) -> &'a mut Self {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { &mut *(ptr as *mut Self) }
+    }
+}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 6/8] rust: kernel: add DAX related TargetOperations
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
                   ` (4 preceding siblings ...)
  2023-06-25 12:16 ` [RFC PATCH 5/8] rust: kernel: add underlying device related TargetOperations Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 7/8] rust: kernel: prepare to implement dm target in Rust Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 8/8] samples: rust: add a device mapper linear target Qingsong Chen
  7 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Martin Rodriguez Reboredo, Alice Ryhl, rust-for-linux

- Add `direct_access` to translate `pgoff` to `pfn`.
- Add `dax_zero_page_range` to zero page range.
- Add `dax_recovery_write` to recover a poisoned range.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/device_mapper.rs    | 139 +++++++++++++++++++++++++++++++-
 2 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 807fc9cf41b8..edda3a9173dd 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -12,6 +12,7 @@
 #include <linux/wait.h>
 #include <linux/sched.h>
 #include <linux/device-mapper.h>
+#include <linux/dax.h>
 
 /* `bindgen` gets confused at certain things. */
 const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index ba13294f2d0b..d2c9d0a5bcd5 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -5,7 +5,7 @@
 //! C header: [`include/linux/device-mapper.h`](../../../../include/linux/device-mapper.h)
 
 use core::marker::PhantomData;
-use core::ops::Index;
+use core::ops::{Index, Range};
 use core::ptr::{addr_of, NonNull};
 
 use crate::error::to_result;
@@ -136,6 +136,39 @@ fn iterate_devices(t: &mut Target<Self>) -> Result<Box<dyn Iterator<Item = IterD
     fn io_hints(t: &mut Target<Self>, limits: &mut QueueLimits) {
         unimplemented!()
     }
+
+    /// Translate a device-relative logical-page-offset into an
+    /// absolute physical pfn.
+    ///
+    /// Return the `addr` and the `pages` available for `DAX` at
+    /// that pfn, if success.
+    #[allow(unused)]
+    fn direct_access(
+        t: &mut Target<Self>,
+        pgoff: usize,
+        nr_pages: usize,
+        mode: DaxMode,
+    ) -> Result<(usize, Range<usize>)> {
+        unimplemented!()
+    }
+
+    /// Zero page range.
+    #[allow(unused)]
+    fn dax_zero_page_range(t: &mut Target<Self>, pgoff: usize, nr_pages: usize) -> Result {
+        unimplemented!()
+    }
+
+    /// Recover a poisoned range by DAX device driver capable of
+    /// clearing poison.
+    #[allow(unused)]
+    fn dax_recovery_write(
+        t: &mut Target<Self>,
+        iov_iter: Pin<&mut IovIter>,
+        pgoff: usize,
+        region: Range<usize>,
+    ) -> usize {
+        unimplemented!()
+    }
 }
 
 /// Wrap the kernel struct `target_type`.
@@ -214,6 +247,17 @@ pub fn register<T: TargetOperations>(
                     (HAS_PREPARE_IOCTL, prepare_ioctl, dm_prepare_ioctl_fn),
                     (HAS_ITERATE_DEVICES, iterate_devices, dm_iterate_devices_fn),
                     (HAS_IO_HINTS, io_hints, dm_io_hints_fn),
+                    (HAS_DIRECT_ACCESS, direct_access, dm_dax_direct_access_fn),
+                    (
+                        HAS_DAX_ZERO_PAGE_RANGE,
+                        dax_zero_page_range,
+                        dm_dax_zero_page_range_fn
+                    ),
+                    (
+                        HAS_DAX_RECOVERY_WRITE,
+                        dax_recovery_write,
+                        dm_dax_recovery_write_fn
+                    ),
                 );
 
                 to_result(bindings::dm_register_target(tt))
@@ -494,6 +538,60 @@ impl TargetType {
             T::io_hints(t, limits);
         }
     }
+    unsafe extern "C" fn dm_dax_direct_access_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        pgoff: core::ffi::c_ulong,
+        nr_pages: core::ffi::c_long,
+        mode: bindings::dax_access_mode,
+        kaddr: *mut *mut core::ffi::c_void,
+        pfn: *mut bindings::pfn_t,
+    ) -> core::ffi::c_long {
+        // SAFETY: the kernel should pass valid `dm_target`, `kaddr` and
+        // `pfn` pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            match T::direct_access(t, pgoff as _, nr_pages as _, mode.into()) {
+                Ok((addr, pages)) => {
+                    *kaddr = addr as _;
+                    (*pfn).val = pages.start as _;
+                    pages.len() as _
+                }
+                Err(e) => e.to_errno() as _,
+            }
+        }
+    }
+    unsafe extern "C" fn dm_dax_zero_page_range_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        pgoff: core::ffi::c_ulong,
+        nr_pages: usize,
+    ) -> core::ffi::c_int {
+        // SAFETY: the kernel should pass valid `dm_target` pointer.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            T::dax_zero_page_range(t, pgoff as _, nr_pages as _)
+                .map_or_else(|e| e.to_errno(), |_| 0)
+        }
+    }
+    unsafe extern "C" fn dm_dax_recovery_write_fn<T: TargetOperations>(
+        ti: *mut bindings::dm_target,
+        pgoff: core::ffi::c_ulong,
+        addr: *mut core::ffi::c_void,
+        bytes: usize,
+        i: *mut bindings::iov_iter,
+    ) -> usize {
+        let region = Range {
+            start: addr as usize,
+            end: (addr as usize) + bytes,
+        };
+
+        // SAFETY: the kernel should pass valid `dm_target` and `iov_iter`
+        // pointers.
+        unsafe {
+            let t = Target::borrow_mut(ti);
+            let iov_iter = IovIter::from_raw(i);
+            T::dax_recovery_write(t, iov_iter, pgoff as _, region)
+        }
+    }
 }
 
 /// Wrap the kernel struct `dm_target`.
@@ -903,3 +1001,42 @@ unsafe fn borrow_mut<'a>(ptr: *mut bindings::queue_limits) -> &'a mut Self {
         unsafe { &mut *(ptr as *mut Self) }
     }
 }
+
+/// Define dax direct_access mode.
+pub enum DaxMode {
+    /// Normal dax access.
+    Access,
+
+    /// Recovery write.
+    RecoveryWrite,
+
+    /// Undefined.
+    Undefined,
+}
+
+impl From<i32> for DaxMode {
+    fn from(value: i32) -> Self {
+        match value {
+            bindings::dax_access_mode_DAX_ACCESS => Self::Access,
+            bindings::dax_access_mode_DAX_RECOVERY_WRITE => Self::RecoveryWrite,
+            _ => Self::Undefined,
+        }
+    }
+}
+
+/// Wrap the kernel struct `iov_iter`.
+///
+/// Dummy.
+#[allow(dead_code)]
+#[pin_data]
+pub struct IovIter {
+    #[pin]
+    opaque: Opaque<bindings::iov_iter>,
+}
+
+impl IovIter {
+    unsafe fn from_raw<'a>(ptr: *mut bindings::iov_iter) -> Pin<&'a mut Self> {
+        // SAFETY: the caller should pass a valid `ptr`.
+        unsafe { Pin::new_unchecked(&mut *(ptr as *mut Self)) }
+    }
+}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 7/8] rust: kernel: prepare to implement dm target in Rust
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
                   ` (5 preceding siblings ...)
  2023-06-25 12:16 ` [RFC PATCH 6/8] rust: kernel: add DAX " Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-25 12:16 ` [RFC PATCH 8/8] samples: rust: add a device mapper linear target Qingsong Chen
  7 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

- Add some methods to set fields in `dm_target`.
- Add some methods to `TargetDevice` to operate on
  the underlying devices.
- Add some methods to dummy `bio`, for remapping.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 rust/helpers.c               |   7 ++
 rust/kernel/device_mapper.rs | 237 +++++++++++++++++++++++++++++++++++
 2 files changed, 244 insertions(+)

diff --git a/rust/helpers.c b/rust/helpers.c
index bb594da56137..706541b58432 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -27,6 +27,7 @@
 #include <linux/spinlock.h>
 #include <linux/sched/signal.h>
 #include <linux/wait.h>
+#include <linux/bio.h>
 
 __noreturn void rust_helper_BUG(void)
 {
@@ -135,6 +136,12 @@ void rust_helper_put_task_struct(struct task_struct *t)
 }
 EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
 
+void rust_helper_bio_set_dev(struct bio *bio, struct block_device *bdev)
+{
+	bio_set_dev(bio, bdev);
+}
+EXPORT_SYMBOL_GPL(rust_helper_bio_set_dev);
+
 /*
  * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
  * as the Rust `usize` type, so we can use it in contexts where Rust
diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
index d2c9d0a5bcd5..fa9b33d529b7 100644
--- a/rust/kernel/device_mapper.rs
+++ b/rust/kernel/device_mapper.rs
@@ -671,6 +671,124 @@ pub fn get_device(&mut self, path: &CStr) -> Result<TargetDevice<T>> {
             }
         }
     }
+
+    /// Return maximum size (in sectors) of I/O submitted to the target.
+    pub fn max_io_len(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).max_io_len as _ }
+    }
+
+    /// Set maximum size (in sectors) of I/O submitted to the target.
+    pub fn set_max_io_len(&mut self, len: usize) -> Result {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            to_result(bindings::dm_set_target_max_io_len(
+                self.opaque.get(),
+                len as _,
+            ))
+        }
+    }
+
+    /// Return the number of zero-length barrier bios that will be submitted
+    /// to the target for the purpose of flushing cache.
+    pub fn num_flush_bios(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).num_flush_bios as _ }
+    }
+
+    /// Set the number of zero-length barrier bios that will be submitted
+    /// to the target for the purpose of flushing cache.
+    pub fn set_num_flush_bios(&mut self, num: usize) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).num_flush_bios = num as _;
+        }
+    }
+
+    /// Return the number of discard bios.
+    pub fn num_discard_bios(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).num_discard_bios as _ }
+    }
+
+    /// Set the number of discard bios.
+    pub fn set_num_discard_bios(&mut self, num: usize) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).num_discard_bios = num as _;
+        }
+    }
+
+    /// Return the number of secure erase bios.
+    pub fn num_secure_erase_bios(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).num_secure_erase_bios as _ }
+    }
+
+    /// Set the number of secure erase bios.
+    pub fn set_num_secure_erase_bios(&mut self, num: usize) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).num_secure_erase_bios = num as _;
+        }
+    }
+
+    /// Return the number of WRITE ZEROES bios.
+    pub fn num_write_zeroes_bios(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).num_write_zeroes_bios as _ }
+    }
+
+    /// Set the number of WRITE ZEROES bios.
+    pub fn set_num_write_zeroes_bios(&mut self, num: usize) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).num_write_zeroes_bios = num as _;
+        }
+    }
+
+    /// Return the minimum number of extra bytes allocated in each io
+    /// for the target to use.
+    pub fn per_io_data_size(&self) -> usize {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { (*self.opaque.get()).per_io_data_size as _ }
+    }
+
+    /// Set the minimum number of extra bytes allocated in each io
+    /// for the target to use.
+    pub fn set_per_io_data_size(&mut self, size: usize) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).per_io_data_size = size as _;
+        }
+    }
+
+    /// Set an error string for the target, could be used
+    /// by [`TargetOperations::ctr`].
+    pub fn set_error(&mut self, err: &CStr) {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe {
+            (*self.opaque.get()).error = err.as_char_ptr() as _;
+        }
+    }
+
+    /// Check if the target is suspended.
+    pub fn suspended(&self) -> bool {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { bindings::dm_suspended(self.opaque.get()) != 0 }
+    }
+
+    /// Check if the target is post_suspending.
+    pub fn post_suspending(&self) -> bool {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { bindings::dm_post_suspending(self.opaque.get()) != 0 }
+    }
+
+    /// Check if the target is noflush_suspending.
+    pub fn noflush_suspending(&self) -> bool {
+        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
+        unsafe { bindings::dm_noflush_suspending(self.opaque.get()) != 0 }
+    }
 }
 
 /// Represent an underlying device of a device mapper target.
@@ -721,6 +839,100 @@ pub fn device_sectors(&self) -> usize {
         // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
         unsafe { (*(*self.dev.as_ptr()).bdev).bd_nr_sectors as _ }
     }
+
+    /// Return [NonNull<TargetDevice>].
+    pub fn as_ptr(&mut self) -> NonNull<Self> {
+        // SAFETY: `self` is non-null and valid.
+        unsafe { NonNull::new_unchecked(self as *mut Self) }
+    }
+
+    /// Propagate report zones command to underlying devices.
+    #[cfg(CONFIG_BLK_DEV_ZONED)]
+    pub fn report_zones(
+        &self,
+        start: usize,
+        sector: usize,
+        args: &mut [ReportZonesArgs],
+    ) -> Result {
+        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
+        unsafe {
+            let bdev = (*self.dev.as_ptr()).bdev;
+            let ptr = args.as_mut_ptr() as *mut bindings::dm_report_zones_args;
+            to_result(bindings::dm_report_zones(
+                bdev,
+                start as _,
+                sector as _,
+                ptr,
+                args.len() as _,
+            ))
+        }
+    }
+
+    /// Translate a device-relative logical-page-offset into an
+    /// absolute physical pfn.
+    ///
+    /// Return the `addr` and the `pages` available for `DAX` at
+    /// that pfn, if success.
+    #[cfg(CONFIG_FS_DAX)]
+    pub fn dax_direct_access(
+        &mut self,
+        pgoff: usize,
+        nr_pages: usize,
+        mode: DaxMode,
+    ) -> Result<(usize, Range<usize>)> {
+        let mut kaddr = core::ptr::null_mut();
+        let mut pfn = bindings::pfn_t::default();
+
+        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
+        let count = unsafe {
+            let dax_dev = (*self.dev.as_ptr()).dax_dev;
+            bindings::dax_direct_access(
+                dax_dev,
+                pgoff as _,
+                nr_pages as _,
+                mode as _,
+                &mut kaddr,
+                &mut pfn,
+            )
+        };
+
+        if count < 0 {
+            Err(Error::from_errno(count as _))
+        } else {
+            let pages = Range {
+                start: pfn.val as usize,
+                end: (pfn.val as usize) + (count as usize),
+            };
+            Ok((kaddr as _, pages))
+        }
+    }
+
+    /// Zero page range.
+    #[cfg(CONFIG_FS_DAX)]
+    pub fn dax_zero_page_range(&mut self, pgoff: usize, nr_pages: usize) -> Result {
+        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
+        unsafe {
+            let dax_dev = (*self.dev.as_ptr()).dax_dev;
+            to_result(bindings::dax_zero_page_range(dax_dev, pgoff as _, nr_pages))
+        }
+    }
+
+    /// Recover a poisoned range by DAX device driver capable of
+    /// clearing poison.
+    #[cfg(CONFIG_FS_DAX)]
+    pub fn dax_recovery_write(
+        &mut self,
+        iov_iter: Pin<&mut IovIter>,
+        pgoff: usize,
+        region: Range<usize>,
+    ) -> usize {
+        // SAFETY: the `from_raw` caller should pass `dev` pointer.
+        unsafe {
+            let dax_dev = (*self.dev.as_ptr()).dax_dev;
+            let i = iov_iter.opaque.get();
+            bindings::dax_recovery_write(dax_dev, pgoff as _, region.start as _, region.len(), i)
+        }
+    }
 }
 
 impl<T: TargetOperations> Drop for TargetDevice<T> {
@@ -919,6 +1131,31 @@ fn op_and_flags(&self) -> u32 {
     pub fn is_write(&self) -> bool {
         self.op_and_flags() & bindings::req_op_REQ_OP_WRITE != 0
     }
+
+    /// Return the start sector of bio.
+    pub fn sector(&self) -> usize {
+        // SAFETY: the `from_raw` caller should pass valid `bio`, so
+        // `self.opaque` is valid too.
+        unsafe { (*self.opaque.get()).bi_iter.bi_sector as _ }
+    }
+
+    /// Set the start sector of bio.
+    pub fn set_sector(&mut self, sector: usize) {
+        // SAFETY: the `from_raw` caller should pass valid `bio`, so
+        // `self.opaque` is valid too.
+        unsafe {
+            (*self.opaque.get()).bi_iter.bi_sector = sector as _;
+        }
+    }
+
+    /// Set the block device of bio.
+    pub fn set_dev<T: TargetOperations>(&mut self, td: &TargetDevice<T>) {
+        // SAFETY: the `from_raw` caller should pass valid `bio`, so
+        // `self.opaque` is valid too.
+        unsafe {
+            bindings::bio_set_dev(self.opaque.get(), (*td.dev.as_ptr()).bdev);
+        }
+    }
 }
 
 /// Wrap the kernel struct `request`.
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 8/8] samples: rust: add a device mapper linear target
  2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
                   ` (6 preceding siblings ...)
  2023-06-25 12:16 ` [RFC PATCH 7/8] rust: kernel: prepare to implement dm target in Rust Qingsong Chen
@ 2023-06-25 12:16 ` Qingsong Chen
  2023-06-30  7:15   ` kernel test robot
  7 siblings, 1 reply; 18+ messages in thread
From: Qingsong Chen @ 2023-06-25 12:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: 田洪亮,
	Qingsong Chen, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	rust-for-linux

Add a device mapper linear target sample, based on the
rust device_mapper abstractions.

Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
---
 samples/rust/Kconfig           |  10 ++
 samples/rust/Makefile          |   1 +
 samples/rust/rust_dm_linear.rs | 257 +++++++++++++++++++++++++++++++++
 3 files changed, 268 insertions(+)
 create mode 100644 samples/rust/rust_dm_linear.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..0d532c995f9a 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -30,6 +30,16 @@ config SAMPLE_RUST_PRINT
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DM_LINEAR
+	tristate "Device mapper linear target"
+	help
+	  This option builds the Rust device mapper linear sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_dm_linear.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..cf4bf82312d8 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,6 @@
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_DM_LINEAR)		+= rust_dm_linear.o
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_dm_linear.rs b/samples/rust/rust_dm_linear.rs
new file mode 100644
index 000000000000..1f338e6b227f
--- /dev/null
+++ b/samples/rust/rust_dm_linear.rs
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust device mapper linear target sample.
+//!
+//! C version: drivers/md/dm-linear.c
+
+use core::ops::Range;
+use core::ptr::NonNull;
+use kernel::bindings::PAGE_SECTORS_SHIFT;
+use kernel::device_mapper::*;
+use kernel::prelude::*;
+use kernel::{c_str, fmt, str::CString};
+
+module! {
+    type: RustDmLinear,
+    name: "rust_dm_linear",
+    author: "Rust for Linux Contributors",
+    description: "Rust device mapper linear target sample",
+    license: "GPL",
+}
+
+struct Linear {
+    dev: TargetDevice<Self>,
+    start: usize,
+}
+
+impl Linear {
+    fn new(dev: TargetDevice<Self>, start: usize) -> impl Init<Self> {
+        init!(Self { dev, start })
+    }
+
+    fn map_sector(&self, sector: usize, base: usize) -> usize {
+        sector - base + self.start
+    }
+}
+
+// SAFETY: `Linear` could be used from all threads.
+unsafe impl Sync for Linear {}
+
+#[vtable]
+impl TargetOperations for Linear {
+    type Private = Linear;
+
+    fn ctr(t: &mut Target<Self>, args: Args) -> Result<Box<Linear>> {
+        if args.len() != 2 {
+            t.set_error(c_str!("Invalid argument count"));
+            return Err(EINVAL);
+        }
+
+        let dev = match t.get_device(&args[0]) {
+            Ok(dev) => dev,
+            Err(e) => {
+                t.set_error(c_str!("Device lookup failed"));
+                return Err(e);
+            }
+        };
+
+        let start = &args[1].to_str().map_err(|_| EINVAL)?;
+        let start = usize::from_str_radix(start, 10).map_err(|_| {
+            t.set_error(c_str!("Invalid device sector"));
+            EINVAL
+        })?;
+
+        t.set_num_flush_bios(1);
+        t.set_num_discard_bios(1);
+        t.set_num_secure_erase_bios(1);
+        t.set_num_write_zeroes_bios(1);
+
+        Box::init(Linear::new(dev, start))
+    }
+
+    fn dtr(_: &mut Target<Self>) {}
+
+    fn map(t: &mut Target<Self>, mut bio: Pin<&mut Bio>) -> MapState {
+        let Some(linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return MapState::Kill;
+        };
+
+        let offset = bio.sector() - linear.dev.target().begin_sector();
+        bio.set_dev(&linear.dev);
+        bio.set_sector(linear.start + offset);
+        MapState::Remapped
+    }
+
+    fn status(t: &mut Target<Self>, type_: StatusType, _: StatusFlags) -> Option<CString> {
+        let Some(linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return None;
+        };
+
+        match type_ {
+            StatusType::Info => None,
+            StatusType::Table => {
+                CString::try_from_fmt(fmt!("{} {}", linear.dev.device_name(), linear.start)).ok()
+            }
+            StatusType::Ima => {
+                let version = linear.dev.target().version();
+                CString::try_from_fmt(fmt!(
+                    "target_name={},target_version={}.{}.{},device_name={},start={};",
+                    linear.dev.target().name(),
+                    version[0],
+                    version[1],
+                    version[2],
+                    linear.dev.device_name(),
+                    linear.start
+                ))
+                .ok()
+            }
+            _ => {
+                pr_warn!("Unsupported dm_status_type\n");
+                None
+            }
+        }
+    }
+
+    fn prepare_ioctl(t: &mut Target<Self>) -> (i32, Option<NonNull<TargetDevice<Self>>>) {
+        let Some(mut linear) = t.private() else {
+                pr_warn!("Error, found no rust_linear\n");
+                return (EINVAL.to_errno(), None);
+            };
+
+        let mut ret = 0;
+        if linear.start > 0 || linear.dev.target().total_sectors() != linear.dev.device_sectors() {
+            ret = 1;
+        }
+        (ret, Some(linear.dev.as_ptr()))
+    }
+
+    fn iterate_devices(t: &mut Target<Self>) -> Result<Box<dyn Iterator<Item = IterDevice<Self>>>> {
+        let Some(mut linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return Err(EINVAL);
+        };
+
+        Ok(Box::init(IterLinear::new(
+            linear.dev.as_ptr(),
+            linear.start,
+            t.total_sectors(),
+        ))?)
+    }
+
+    #[cfg(CONFIG_BLK_DEV_ZONED)]
+    fn report_zones(t: &mut Target<Self>, args: &mut [ReportZonesArgs]) -> Result {
+        if args.len() == 0 {
+            pr_warn!("Invalid report_zones_args\n");
+            return Err(EINVAL);
+        }
+
+        let Some(linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return Err(EINVAL);
+        };
+
+        let sector = linear.map_sector(args[0].next_sector(), linear.dev.target().begin_sector());
+        linear.dev.report_zones(linear.start, sector, args)
+    }
+
+    #[cfg(CONFIG_FS_DAX)]
+    fn direct_access(
+        t: &mut Target<Self>,
+        pgoff: usize,
+        nr_pages: usize,
+        mode: DaxMode,
+    ) -> Result<(usize, Range<usize>)> {
+        let base = t.begin_sector();
+        let Some(mut linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return Err(EINVAL);
+        };
+
+        let sector = linear.map_sector(pgoff << PAGE_SECTORS_SHIFT, base);
+        let offset = (linear.dev.device_sectors() + sector) >> PAGE_SECTORS_SHIFT;
+        linear.dev.dax_direct_access(offset, nr_pages, mode)
+    }
+
+    #[cfg(CONFIF_FS_DAX)]
+    fn dax_zero_page_range(t: &mut Target<Self>, pgoff: usize, nr_pages: usize) -> Result {
+        let base = t.begin_sector();
+        let Some(mut linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return Err(EINVAL);
+        };
+
+        let sector = linear.map_sector(pgoff << PAGE_SECTORS_SHIFT, base);
+        let offset = (linear.dev.device_sectors() + sector) >> PAGE_SECTORS_SHIFT;
+        linear.dev.dax_zero_page_range(offset, nr_pages)
+    }
+
+    #[cfg(CONFIF_FS_DAX)]
+    fn dax_recovery_write(
+        t: &mut Target<Self>,
+        iov_iter: Pin<&mut IovIter>,
+        pgoff: usize,
+        region: Range<usize>,
+    ) -> usize {
+        let base = t.begin_sector();
+        let Some(mut linear) = t.private() else {
+            pr_warn!("Error, found no rust_linear\n");
+            return 0;
+        };
+
+        let sector = linear.map_sector(pgoff << PAGE_SECTORS_SHIFT, base);
+        let offset = (linear.dev.device_sectors() + sector) >> PAGE_SECTORS_SHIFT;
+        linear.dev.dax_recovery_write(iov_iter, offset, region)
+    }
+}
+
+struct IterLinear {
+    item: Option<IterDevice<Linear>>,
+}
+
+impl IterLinear {
+    fn new(dev: NonNull<TargetDevice<Linear>>, start: usize, len: usize) -> impl Init<Self> {
+        init!(Self {
+            item: Some((dev, start, len))
+        })
+    }
+}
+
+impl Iterator for IterLinear {
+    type Item = IterDevice<Linear>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.item.take()
+    }
+}
+
+struct RustDmLinear(Pin<Box<TargetType>>);
+
+impl kernel::Module for RustDmLinear {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("Rust device mapper linear target sample (init)\n");
+
+        let target = Box::pin_init(TargetType::register::<Linear>(
+            _module,
+            c_str!("rust_linear"),
+            [0, 0, 1],
+            0,
+        ));
+
+        let target = match target {
+            Ok(target) => target,
+            Err(e) => {
+                pr_warn!("Target register failed, errno: {}\n", e.to_errno());
+                return Err(e);
+            }
+        };
+        Ok(RustDmLinear(target))
+    }
+}
+
+impl Drop for RustDmLinear {
+    fn drop(&mut self) {
+        pr_info!("Rust device mapper linear target sample (exit)\n");
+    }
+}
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
@ 2023-06-25 12:25   ` Greg KH
  2023-06-26  3:26     ` Qingsong Chen
  2023-07-14 15:48   ` Benno Lossin
  1 sibling, 1 reply; 18+ messages in thread
From: Greg KH @ 2023-06-25 12:25 UTC (permalink / raw)
  To: Qingsong Chen
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin,
	Martin Rodriguez Reboredo, Alice Ryhl, Vincenzo Palazzo,
	Asahi Lina, rust-for-linux

On Sun, Jun 25, 2023 at 08:16:50PM +0800, Qingsong Chen wrote:
> - Declare `vtable` for basic target operations.
> - Wrap `target_type` to register/unregister target.
> - Wrap `dm_target`/`dm_dev` to handle io request.
> - Add a dummy `bio` wrapper.

Shouldn't this be all different commits, and different files?  A bio
shouldn't be defined in the device_mapper.rs file, as that would prevent
anyone else from accessing it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations
  2023-06-25 12:16 ` [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations Qingsong Chen
@ 2023-06-25 16:58   ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2023-06-25 16:58 UTC (permalink / raw)
  To: Qingsong Chen; +Cc: llvm, oe-kbuild-all

Hi Qingsong,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on rust/rust-next]
[also build test ERROR on linus/master v6.4-rc7 next-20230623]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qingsong-Chen/rust-kernel-add-basic-abstractions-for-device-mapper/20230625-201850
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20230625121657.3631109-5-changxian.cqs%40antgroup.com
patch subject: [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230626/202306260042.e8UKaBKy-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230626/202306260042.e8UKaBKy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306260042.e8UKaBKy-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0308]: mismatched types
   --> rust/kernel/device_mapper.rs:145:34
   |
   145 |               (*$tt).$filed = Some(TargetType::$func::<T>);
   |                               ---- ^^^^^^^^^^^^^^^^^^^^^^ incorrect number of function parameters
   |                               |
   |                               arguments to this enum variant are incorrect
   ...
   170 | /                 check_target_operations!(
   171 | |                     tt,
   172 | |                     (HAS_CTR, ctr, dm_ctr_fn),
   173 | |                     (HAS_DTR, dtr, dm_dtr_fn),
   ...   |
   195 | |                     (HAS_REPORT_ZONES, report_zones, dm_report_zones_fn),
   196 | |                 );
   | |_________________- in this macro invocation
   |
   = note: expected fn pointer `unsafe extern "C" fn(*mut dm_target) -> _`
   found fn item `unsafe extern "C" fn(*mut dm_target, *mut dm_report_zones_args, u32) -> _ {TargetType::dm_report_zones_fn::<T>}`
   help: the type constructed contains `unsafe extern "C" fn(*mut dm_target, *mut dm_report_zones_args, u32) -> i32 {TargetType::dm_report_zones_fn::<T>}` due to the type of the argument passed
   --> rust/kernel/device_mapper.rs:145:29
   |
   145 |               (*$tt).$filed = Some(TargetType::$func::<T>);
   |                               ^^^^^----------------------^
   |                                    |
   |                                    this argument influences the type of `Some`
   ...
   170 | /                 check_target_operations!(
   171 | |                     tt,
   172 | |                     (HAS_CTR, ctr, dm_ctr_fn),
   173 | |                     (HAS_DTR, dtr, dm_dtr_fn),
   ...   |
   195 | |                     (HAS_REPORT_ZONES, report_zones, dm_report_zones_fn),
   196 | |                 );
   | |_________________- in this macro invocation
   note: tuple variant defined here
   --> /opt/cross/rustc-1.68.2-bindgen-0.56.0/rustup/toolchains/1.68.2-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:570:5
   |
   570 |     Some(#[stable(feature = "rust1", since = "1.0.0")] T),
   |     ^^^^
   = note: this error originates in the macro `check_target_operations` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0609]: no field `next_sector` on type `dm_report_zones_args`
   --> rust/kernel/device_mapper.rs:657:34
   |
   657 |         unsafe { (*self.0.get()).next_sector as _ }
   |                                  ^^^^^^^^^^^ unknown field

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-06-25 12:25   ` Greg KH
@ 2023-06-26  3:26     ` Qingsong Chen
  2023-06-26 16:39       ` Boqun Feng
  0 siblings, 1 reply; 18+ messages in thread
From: Qingsong Chen @ 2023-06-26  3:26 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin,
	Martin Rodriguez Reboredo, Alice Ryhl, Vincenzo Palazzo,
	Asahi Lina, rust-for-linux

On 6/25/23 8:25 PM, Greg KH wrote:
> Shouldn't this be all different commits, and different files?  A bio
> shouldn't be defined in the device_mapper.rs file, as that would prevent
> anyone else from accessing it.

Yes, you're right. Since we noticed that some block layer patches[1]
are under review, we don't put much effort into that. Actually, we
don't want to include all the `bio` implementation in this patchset,
so it just some dummy codes.

If necessary, a seperate commit could be splited, including things
that do not belong to the device mapper.

[1]: 
https://lore.kernel.org/rust-for-linux/20230503090708.2524310-5-nmi@metaspace.dk/

Thanks,
Qingsong Chen

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-06-26  3:26     ` Qingsong Chen
@ 2023-06-26 16:39       ` Boqun Feng
  0 siblings, 0 replies; 18+ messages in thread
From: Boqun Feng @ 2023-06-26 16:39 UTC (permalink / raw)
  To: Qingsong Chen
  Cc: Greg KH, linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Gary Guo,
	Björn Roy Baron, Benno Lossin, Martin Rodriguez Reboredo,
	Alice Ryhl, Vincenzo Palazzo, Asahi Lina, rust-for-linux

On Mon, Jun 26, 2023 at 11:26:12AM +0800, Qingsong Chen wrote:
> On 6/25/23 8:25 PM, Greg KH wrote:
> > Shouldn't this be all different commits, and different files?  A bio
> > shouldn't be defined in the device_mapper.rs file, as that would prevent
> > anyone else from accessing it.
> 
> Yes, you're right. Since we noticed that some block layer patches[1]
> are under review, we don't put much effort into that. Actually, we
> don't want to include all the `bio` implementation in this patchset,
> so it just some dummy codes.
> 
> If necessary, a seperate commit could be splited, including things
> that do not belong to the device mapper.
> 

Maybe you can just base on one or all the patches from Andreas' series
(and mention the dependency in the cover letter)?

Regards,
Boqun

> [1]: https://lore.kernel.org/rust-for-linux/20230503090708.2524310-5-nmi@metaspace.dk/
> 
> Thanks,
> Qingsong Chen

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 8/8] samples: rust: add a device mapper linear target
  2023-06-25 12:16 ` [RFC PATCH 8/8] samples: rust: add a device mapper linear target Qingsong Chen
@ 2023-06-30  7:15   ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2023-06-30  7:15 UTC (permalink / raw)
  To: Qingsong Chen; +Cc: llvm, oe-kbuild-all

Hi Qingsong,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on rust/rust-next]
[also build test WARNING on linus/master next-20230630]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Qingsong-Chen/rust-kernel-add-basic-abstractions-for-device-mapper/20230625-201850
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/20230625121657.3631109-9-changxian.cqs%40antgroup.com
patch subject: [RFC PATCH 8/8] samples: rust: add a device mapper linear target
config: x86_64-randconfig-x001-20230629 (https://download.01.org/0day-ci/archive/20230630/202306301458.mlsxCJLl-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230630/202306301458.mlsxCJLl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306301458.mlsxCJLl-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> warning: unused import: `core::ops::Range`
   --> samples/rust/rust_dm_linear.rs:7:5
   |
   7 | use core::ops::Range;
   |     ^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default
--
>> warning: unused import: `kernel::bindings::PAGE_SECTORS_SHIFT`
   --> samples/rust/rust_dm_linear.rs:9:5
   |
   9 | use kernel::bindings::PAGE_SECTORS_SHIFT;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
  2023-06-25 12:25   ` Greg KH
@ 2023-07-14 15:48   ` Benno Lossin
  2023-07-17  3:04     ` Qingsong Chen
  1 sibling, 1 reply; 18+ messages in thread
From: Benno Lossin @ 2023-07-14 15:48 UTC (permalink / raw)
  To: Qingsong Chen
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Martin Rodriguez Reboredo,
	Alice Ryhl, Vincenzo Palazzo, Asahi Lina, rust-for-linux

> - Declare `vtable` for basic target operations.
> - Wrap `target_type` to register/unregister target.
> - Wrap `dm_target`/`dm_dev` to handle io request.
> - Add a dummy `bio` wrapper.
> 
> Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>

On my system this patch series (I did not test which patch exactly) does
not compile.

I have left some comments below, they show some patterns present in
the other patches as well.

> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/kernel/device_mapper.rs    | 382 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   1 +
>  3 files changed, 384 insertions(+)
>  create mode 100644 rust/kernel/device_mapper.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..807fc9cf41b8 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -11,6 +11,7 @@
>  #include <linux/refcount.h>
>  #include <linux/wait.h>
>  #include <linux/sched.h>
> +#include <linux/device-mapper.h>
> 
>  /* `bindgen` gets confused at certain things. */
>  const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
> new file mode 100644
> index 000000000000..28286b373b59
> --- /dev/null
> +++ b/rust/kernel/device_mapper.rs
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Device Mapper.
> +//!
> +//! C header: [`include/linux/device-mapper.h`](../../../../include/linux/device-mapper.h)
> +
> +use core::marker::PhantomData;
> +use core::ops::Index;
> +use core::ptr::{addr_of, NonNull};
> +
> +use crate::error::to_result;
> +use crate::prelude::*;
> +use crate::str::CStr;
> +use crate::types::Opaque;
> +
> +/// Declare operations that a device mapper target can do.
> +#[vtable]
> +pub trait TargetOperations: Sized {
> +    /// Persist user data.
> +    type Private: Sync;
> +
> +    /// Constructor. The target will already have the table, type, begin and
> +    /// len fields filled in. A `Private` struct can be returned to persist
> +    /// its own context.
> +    fn ctr(t: &mut Target<Self>, args: Args) -> Result<Box<Self::Private>>;
> +
> +    /// Destructor. The target could clean up anything hidden in `Private`,
> +    /// and `Private` itself can be dropped automatically.
> +    fn dtr(t: &mut Target<Self>);
> +
> +    /// Map block IOs. Return [`MapState`] to indicate how to handle the `bio`
> +    /// later (end or resubmit).
> +    fn map(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> MapState;
> +}
> +
> +/// Wrap the kernel struct `target_type`.
> +///
> +/// It contains a struct `list_head` for internal device-mapper use, so it
> +/// should be pinned. Users can use this struct to register/unregister their
> +/// own device mapper target.
> +#[pin_data(PinnedDrop)]
> +pub struct TargetType {
> +    #[pin]
> +    opaque: Opaque<bindings::target_type>,
> +}
> +
> +/// Define target feature type.
> +pub type Feature = u64;
> +
> +// SAFETY: It's OK to access `TargetType` from multiple threads. The
> +// `dm_register_target` and `dm_unregister_target` provides its own
> +// synchronization.
> +unsafe impl Sync for TargetType {}
> +
> +macro_rules! check_target_operations {
> +    ($tt:expr, $(($op:ident, $filed:ident, $func:ident),)+) => {$(
> +        if <T as TargetOperations>::$op {
> +            (*$tt).$filed = Some(TargetType::$func::<T>);
> +        }
> +    )+};
> +}

This macro should be called `set_target_operations`.

> +
> +impl TargetType {
> +    /// Provide an in-place constructor to register a new device mapper target.
> +    pub fn register<T: TargetOperations>(
> +        module: &'static ThisModule,
> +        name: &'static CStr,
> +        version: [u32; 3],
> +        feature: Feature,
> +    ) -> impl PinInit<Self, Error> {
> +        // SAFETY: `slot` is valid while the closure is called.
> +        unsafe {
> +            init::pin_init_from_closure(move |slot: *mut Self| {
> +                // `slot` contains uninit memory, avoid creating a reference.
> +                let opaque = addr_of!((*slot).opaque);
> +                let tt = Opaque::raw_get(opaque);
> +
> +                (*tt).module = module.0;
> +                (*tt).name = name.as_char_ptr();
> +                (*tt).version = version;
> +                (*tt).features = feature;
> +
> +                check_target_operations!(
> +                    tt,
> +                    (HAS_CTR, ctr, dm_ctr_fn),
> +                    (HAS_DTR, dtr, dm_dtr_fn),
> +                    (HAS_MAP, map, dm_map_fn),
> +                );
> +
> +                to_result(bindings::dm_register_target(tt))
> +            })
> +        }

This `unsafe` block should be split into multiple.

> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for TargetType {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY: `self.opaque` are initialized by the `register` constructor,
> +        // so it's valid.

This should be a type invariant of `TargetType`.

> +        unsafe {
> +            bindings::dm_unregister_target(self.opaque.get());
> +        }
> +    }
> +}
> +
> +impl TargetType {
> +    unsafe extern "C" fn dm_ctr_fn<T: TargetOperations>(
> +        target: *mut bindings::dm_target,
> +        argc: core::ffi::c_uint,
> +        argv: *mut *mut core::ffi::c_char,
> +    ) -> core::ffi::c_int {
> +        // SAFETY: the kernel splits arguments by `dm_split_args`, then pass
> +        // suitable `argc` and `argv` to `dm_ctr_fn`. If `argc` is not zero,
> +        // `argv` is non-null and valid.
> +        let args = unsafe { Args::new(argc, argv) };
> +
> +        // SAFETY: the kernel should pass a valid `dm_target`.
> +        let t = unsafe { Target::borrow_mut(target) };
> +        T::ctr(t, args).map_or_else(
> +            |e| e.to_errno(),
> +            // SAFETY: the kernel should pass a valid `dm_target`.
> +            |p| unsafe {
> +                (*target).private = Box::into_raw(p) as _;
> +                0
> +            },
> +        )
> +    }
> +    unsafe extern "C" fn dm_dtr_fn<T: TargetOperations>(ti: *mut bindings::dm_target) {
> +        // SAFETY: the kernel should pass a valid `dm_target`.
> +        let t = unsafe { Target::borrow_mut(ti) };
> +        T::dtr(t);
> +        // SAFETY: `private` is constructed in `dm_ctr_fn`, and we drop it here.
> +        unsafe {
> +            let ptr = (*ti).private as *mut T::Private;
> +            drop(Box::from_raw(ptr));
> +            (*ti).private = core::ptr::null_mut();
> +        }
> +    }
> +    unsafe extern "C" fn dm_map_fn<T: TargetOperations>(
> +        ti: *mut bindings::dm_target,
> +        bio: *mut bindings::bio,
> +    ) -> core::ffi::c_int {
> +        // SAFETY: the kernel should pass a valid `dm_target` and `bio`.
> +        unsafe {
> +            let t = Target::borrow_mut(ti);
> +            let bio = Bio::from_raw(bio);
> +            T::map(t, bio) as _
> +        }
> +    }
> +}
> +
> +/// Wrap the kernel struct `dm_target`.
> +///
> +/// This struct represents a device mapper target. And the device mapper
> +/// core will alloc/free `dm_target` instances, so we just `borrow` it.
> +/// It also holds a `Private` struct, which is used to persist user's data,
> +/// and can be accessed by the `private` method.
> +pub struct Target<T: TargetOperations + Sized> {
> +    opaque: Opaque<bindings::dm_target>,
> +    _p: PhantomData<*mut T::Private>,
> +}
> +
> +impl<T: TargetOperations> Target<T> {
> +    unsafe fn borrow<'a>(ptr: *const bindings::dm_target) -> &'a Self {

`unsafe` functions need to have a `# Safety` section.

> +        // SAFETY: the caller should pass a valid `ptr`.

This requirement needs to be documented on the function.

> +        unsafe { &*(ptr as *const Self) }
> +    }
> +
> +    unsafe fn borrow_mut<'a>(ptr: *mut bindings::dm_target) -> &'a mut Self {
> +        // SAFETY: the caller should pass a valid `ptr`.

This requirement needs to be documented on the function.

> +        unsafe { &mut *(ptr as *mut Self) }
> +    }
> +
> +    /// Access user's private data.
> +    pub fn private(&mut self) -> Option<Pin<&mut T::Private>> {
> +        let t = self.opaque.get();
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.

This should be a type invariant.

> +        // And 'p' is non-null and assigned in `dm_ctr_fn`, so it's valid.
> +        unsafe {
> +            ((*t).private as *mut T::Private)
> +                .as_mut()
> +                .map(|p| Pin::new_unchecked(p))
> +        }

Split this `unsafe` block.

> +    }
> +
> +    /// Return the target name.
> +    pub fn name(&self) -> &CStr {
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
> +        unsafe {
> +            let name = (*(*self.opaque.get()).type_).name;
> +            CStr::from_char_ptr(name)
> +        }

Split this `unsafe` block.

> +    }
> +
> +    /// Return the target version.
> +    pub fn version(&self) -> [u32; 3] {
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
> +        unsafe { (*(*self.opaque.get()).type_).version }
> +    }
> +
> +    /// Return the target begin sector.
> +    pub fn begin_sector(&self) -> usize {
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
> +        unsafe { (*self.opaque.get()).begin as _ }
> +    }
> +
> +    /// Return the target total sectors.
> +    pub fn total_sectors(&self) -> usize {
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
> +        unsafe { (*self.opaque.get()).len as _ }
> +    }
> +
> +    /// Get the underlying device by `path`. The `dm_put_device` will be called when
> +    /// [`TargetDevice`] is dropped.
> +    pub fn get_device(&mut self, path: &CStr) -> Result<TargetDevice<T>> {
> +        let mut dd = core::ptr::null_mut();
> +        // SAFETY: `self.opaque` is borrowed from foreign pointer, should be valid.
> +        unsafe {
> +            let mode = bindings::dm_table_get_mode((*self.opaque.get()).table);
> +            match bindings::dm_get_device(self.opaque.get(), path.as_char_ptr(), mode, &mut dd) {
> +                0 => {
> +                    let ti = self.opaque.get();
> +                    Ok(TargetDevice::from_raw(ti, dd))
> +                }
> +                err => Err(Error::from_errno(err)),
> +            }
> +        }

Split this `unsafe` block.

> +    }
> +}
> +
> +/// Represent an underlying device of a device mapper target.
> +///
> +/// We also holds a pointer to `dm_target`, so that we can call
> +/// `dm_put_device` in `drop`, to close the device correctly.
> +pub struct TargetDevice<T: TargetOperations + Sized> {
> +    ti: NonNull<bindings::dm_target>,
> +    dev: NonNull<bindings::dm_dev>,
> +    _p: PhantomData<*mut Target<T>>,
> +}
> +
> +impl<T: TargetOperations> TargetDevice<T> {
> +    unsafe fn from_raw(ti: *mut bindings::dm_target, dd: *mut bindings::dm_dev) -> Self {
> +        // SAFETY: the caller should pass valid `dm_target` and `dm_dev`.
> +        unsafe {
> +            Self {
> +                ti: NonNull::new_unchecked(ti),
> +                dev: NonNull::new_unchecked(dd),
> +                _p: PhantomData,
> +            }
> +        }
> +    }
> +
> +    /// Borrow the device mapper target.
> +    pub fn target(&self) -> &Target<T> {
> +        // SAFETY: the `from_raw` caller should pass valid `ti` pointer.
> +        unsafe { Target::borrow(self.ti.as_ptr()) }
> +    }
> +
> +    /// Borrow the device mapper target mutably.
> +    pub fn mut_target(&mut self) -> &mut Target<T> {
> +        // SAFETY: the `from_raw` caller should pass valid `ti` pointer.
> +        unsafe { Target::borrow_mut(self.ti.as_ptr()) }
> +    }
> +
> +    /// Return the device name.
> +    pub fn device_name(&self) -> &CStr {
> +        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
> +        unsafe {
> +            let name = (*self.dev.as_ptr()).name.as_ptr();
> +            CStr::from_char_ptr(name)
> +        }
> +    }
> +
> +    /// Return the total device sectors.
> +    pub fn device_sectors(&self) -> usize {
> +        // SAFETY: the `from_raw` caller should pass valid `dev` pointer.
> +        unsafe { (*(*self.dev.as_ptr()).bdev).bd_nr_sectors as _ }
> +    }
> +}
> +
> +impl<T: TargetOperations> Drop for TargetDevice<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: the `from_raw` caller should pass valid `ti` and `dev`.
> +        unsafe { bindings::dm_put_device(self.ti.as_ptr(), self.dev.as_ptr()) }
> +    }
> +}
> +
> +/// The return values of target map function, i.e., [`TargetOperations::map`].
> +#[repr(u32)]
> +pub enum MapState {
> +    /// The target will handle the io by resubmitting it later.
> +    Submitted = bindings::DM_MAPIO_SUBMITTED,
> +
> +    /// Simple remap complete.
> +    Remapped = bindings::DM_MAPIO_REMAPPED,
> +
> +    /// The target wants to requeue the io.
> +    Requeue = bindings::DM_MAPIO_REQUEUE,
> +
> +    /// The target wants to requeue the io after a delay.
> +    DelayRequeue = bindings::DM_MAPIO_DELAY_REQUEUE,
> +
> +    /// The target wants to complete the io.
> +    Kill = bindings::DM_MAPIO_KILL,
> +}

Is it really necessary to have these correspond to the exact values?
Could we also just have a conversion function and leave the repr to
the compiler?

> +
> +/// Wrap the `c_char` arguments, which yields `CStr`.
> +pub struct Args {
> +    argc: core::ffi::c_uint,
> +    argv: *mut *mut core::ffi::c_char,
> +}
> +
> +impl Args {
> +    /// The caller should ensure that the number of valid `argv` pointers
> +    /// should be exactly `argc`.

This should be in the `# Safety` section.

> +    unsafe fn new(argc: core::ffi::c_uint, argv: *mut *mut core::ffi::c_char) -> Self {
> +        Self { argc, argv }
> +    }
> +
> +    /// Return the number of arguments.
> +    pub fn len(&self) -> usize {
> +        self.argc as _
> +    }
> +
> +    /// Return the `nth` (from zero) argument.
> +    ///
> +    /// If the index is out of bounds, return `None`.
> +    pub fn get(&self, index: usize) -> Option<&CStr> {
> +        if self.argc == 0 || index >= self.argc as _ {
> +            None
> +        } else {
> +            // SAFETY: the `new` caller should ensure the number of valid `argv`.

Needs to be a type invariant.

--
Cheers,
Benno

> +            unsafe { Some(CStr::from_char_ptr(*self.argv.add(index))) }
> +        }
> +    }
> +}
> +
> +impl Index<usize> for Args {
> +    type Output = CStr;
> +
> +    /// When using the indexing operator(`[]`), the caller should check the
> +    /// length of [`Args`]. If the index is out of bounds, this will [`panic`].
> +    fn index(&self, index: usize) -> &Self::Output {
> +        if self.argc == 0 || index >= self.argc as _ {
> +            panic!(
> +                "Index out of bounds: the length is {} but the index is {}.",
> +                self.argc, index
> +            )
> +        } else {
> +            // SAFETY: the `new` caller should ensure the number of valid `argv`.
> +            unsafe { CStr::from_char_ptr(*self.argv.add(index)) }
> +        }
> +    }
> +}
> +
> +/// Wrap the kernel struct `bio`.
> +///
> +/// Dummy.
> +#[pin_data]
> +pub struct Bio {
> +    #[pin]
> +    opaque: Opaque<bindings::bio>,
> +}
> +
> +impl Bio {
> +    unsafe fn from_raw<'a>(ptr: *mut bindings::bio) -> Pin<&'a mut Self> {
> +        // SAFETY: the caller should pass a valid `bio` pointer.
> +        unsafe { Pin::new(&mut *(ptr as *mut Self)) }
> +    }
> +
> +    fn op_and_flags(&self) -> u32 {
> +        // SAFETY: the `from_raw` caller should pass valid `bio`, so
> +        // `self.opaque` is valid too.
> +        unsafe { (*self.opaque.get()).bi_opf }
> +    }
> +
> +    /// Return `true` if the bio request is write.
> +    pub fn is_write(&self) -> bool {
> +        self.op_and_flags() & bindings::req_op_REQ_OP_WRITE != 0
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..3dc19f4642f0 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@
>  #[cfg(not(testlib))]
>  mod allocator;
>  mod build_assert;
> +pub mod device_mapper;
>  pub mod error;
>  pub mod init;
>  pub mod ioctl;
> --
> 2.40.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations
  2023-06-25 12:16 ` [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations Qingsong Chen
@ 2023-07-14 15:49   ` Benno Lossin
  2023-07-17  3:42     ` Qingsong Chen
  0 siblings, 1 reply; 18+ messages in thread
From: Benno Lossin @ 2023-07-14 15:49 UTC (permalink / raw)
  To: Qingsong Chen
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, rust-for-linux

> - Add `request` related and end_io TargetOperations.
> - Add a dummy `request` wrapper and `blk_status_t`.
> 
> Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
> ---
>  rust/kernel/device_mapper.rs | 199 +++++++++++++++++++++++++++++++++++
>  1 file changed, 199 insertions(+)
> 
> diff --git a/rust/kernel/device_mapper.rs b/rust/kernel/device_mapper.rs
> index 28286b373b59..153bae3aad79 100644
> --- a/rust/kernel/device_mapper.rs
> +++ b/rust/kernel/device_mapper.rs
> @@ -31,6 +31,39 @@ pub trait TargetOperations: Sized {
>      /// Map block IOs. Return [`MapState`] to indicate how to handle the `bio`
>      /// later (end or resubmit).
>      fn map(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> MapState;
> +
> +    /// Map `request`. Return [`MapState`] and the optional cloned `request`.
> +    #[allow(unused)]
> +    fn clone_and_map_rq<'a>(
> +        t: &mut Target<Self>,
> +        rq: Pin<&mut Request>,
> +        map_ctx: &mut MapInfo,
> +    ) -> (MapState, Option<Pin<&'a mut Request>>) {
> +        unimplemented!()

Note that these should be implemented when you submit this as an actual
patch. (Not sure what you would do in this case, but throwing a panic is
not a good idea.) This also applies to all subsequent patches.
I think you might have done this, because you want the C side to choose
the default implementation. But it should also be possible to call these
from the Rust side.

--
Cheers,
Benno

> +    }
> +
> +    /// Release the cloned `request`.
> +    #[allow(unused)]
> +    fn release_clone_rq(rq: Pin<&mut Request>, map_ctx: &mut MapInfo) {
> +        unimplemented!()
> +    }
> +
> +    /// End the `bio`. Return [`EndState`] and [`BlkStatus`].
> +    #[allow(unused)]
> +    fn end_io(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> (EndState, BlkStatus) {
> +        unimplemented!()
> +    }
> +
> +    /// End the `request`. Return [`EndState`].
> +    #[allow(unused)]
> +    fn rq_end_io(
> +        t: &mut Target<Self>,
> +        rq: Pin<&mut Request>,
> +        map_ctx: &mut MapInfo,
> +        err: BlkStatus,
> +    ) -> EndState {
> +        unimplemented!()
> +    }
>  }
> 
>  /// Wrap the kernel struct `target_type`.
> @@ -85,6 +118,18 @@ pub fn register<T: TargetOperations>(
>                      (HAS_CTR, ctr, dm_ctr_fn),
>                      (HAS_DTR, dtr, dm_dtr_fn),
>                      (HAS_MAP, map, dm_map_fn),
> +                    (
> +                        HAS_CLONE_AND_MAP_RQ,
> +                        clone_and_map_rq,
> +                        dm_clone_and_map_request_fn
> +                    ),
> +                    (
> +                        HAS_RELEASE_CLONE_RQ,
> +                        release_clone_rq,
> +                        dm_release_clone_request_fn
> +                    ),
> +                    (HAS_END_IO, end_io, dm_endio_fn),
> +                    (HAS_RQ_END_IO, rq_end_io, dm_request_endio_fn),
>                  );
> 
>                  to_result(bindings::dm_register_target(tt))
> @@ -148,6 +193,64 @@ impl TargetType {
>              T::map(t, bio) as _
>          }
>      }
> +    unsafe extern "C" fn dm_clone_and_map_request_fn<T: TargetOperations>(
> +        ti: *mut bindings::dm_target,
> +        rq: *mut bindings::request,
> +        map_context: *mut bindings::map_info,
> +        clone: *mut *mut bindings::request,
> +    ) -> core::ffi::c_int {
> +        // SAFETY: the kernel should pass valid `dm_target`, `request`
> +        // and `map_info` pointers.
> +        unsafe {
> +            let t = Target::borrow_mut(ti);
> +            let rq = Request::from_raw(rq);
> +            let map_ctx = MapInfo::borrow_mut(map_context);
> +            let (map_state, rq_cloned) = T::clone_and_map_rq(t, rq, map_ctx);
> +            *clone = rq_cloned.map_or(core::ptr::null_mut(), |rq| rq.opaque.get());
> +            map_state as _
> +        }
> +    }
> +    unsafe extern "C" fn dm_release_clone_request_fn<T: TargetOperations>(
> +        clone: *mut bindings::request,
> +        map_context: *mut bindings::map_info,
> +    ) {
> +        // SAFETY: the kernel should pass valid `request` and `map_info` pointers.
> +        unsafe {
> +            let rq = Request::from_raw(clone);
> +            let map_ctx = MapInfo::borrow_mut(map_context);
> +            T::release_clone_rq(rq, map_ctx);
> +        }
> +    }
> +    unsafe extern "C" fn dm_endio_fn<T: TargetOperations>(
> +        ti: *mut bindings::dm_target,
> +        bio: *mut bindings::bio,
> +        error: *mut bindings::blk_status_t,
> +    ) -> core::ffi::c_int {
> +        // SAFETY: the kernel should pass valid `dm_target`, `bio` and
> +        // `error` pointers.
> +        unsafe {
> +            let t = Target::borrow_mut(ti);
> +            let bio = Bio::from_raw(bio);
> +            let (end_state, blk_status) = T::end_io(t, bio);
> +            *error = blk_status as _;
> +            end_state as _
> +        }
> +    }
> +    unsafe extern "C" fn dm_request_endio_fn<T: TargetOperations>(
> +        ti: *mut bindings::dm_target,
> +        clone: *mut bindings::request,
> +        error: bindings::blk_status_t,
> +        map_context: *mut bindings::map_info,
> +    ) -> core::ffi::c_int {
> +        // SAFETY: the kernel should pass valid `dm_target`, `request`
> +        // and `map_info` pointers.
> +        unsafe {
> +            let t = Target::borrow_mut(ti);
> +            let rq = Request::from_raw(clone);
> +            let map_ctx = MapInfo::borrow_mut(map_context);
> +            T::rq_end_io(t, rq, map_ctx, (error as u32).into()) as _
> +        }
> +    }
>  }
> 
>  /// Wrap the kernel struct `dm_target`.
> @@ -305,6 +408,33 @@ pub enum MapState {
>      Kill = bindings::DM_MAPIO_KILL,
>  }
> 
> +/// The return values of target end_io function.
> +#[repr(u32)]
> +pub enum EndState {
> +    /// Ended successfully.
> +    Done = bindings::DM_ENDIO_DONE,
> +
> +    /// The io has still not completed (eg, multipath target might
> +    /// want to requeue a failed io).
> +    Incomplete = bindings::DM_ENDIO_INCOMPLETE,
> +
> +    /// The target wants to requeue the io.
> +    Requeue = bindings::DM_ENDIO_REQUEUE,
> +
> +    /// The target wants to requeue the io after a delay.
> +    DelayRequeue = bindings::DM_ENDIO_DELAY_REQUEUE,
> +}
> +
> +/// Wrap the c struct `map_info`.
> +pub struct MapInfo(Opaque<bindings::map_info>);
> +
> +impl MapInfo {
> +    unsafe fn borrow_mut<'a>(ptr: *mut bindings::map_info) -> &'a mut Self {
> +        // SAFETY: the caller should pass a valid `ptr`.
> +        unsafe { &mut *(ptr as *mut Self) }
> +    }
> +}
> +
>  /// Wrap the `c_char` arguments, which yields `CStr`.
>  pub struct Args {
>      argc: core::ffi::c_uint,
> @@ -380,3 +510,72 @@ pub fn is_write(&self) -> bool {
>          self.op_and_flags() & bindings::req_op_REQ_OP_WRITE != 0
>      }
>  }
> +
> +/// Wrap the kernel struct `request`.
> +///
> +/// Dummy.
> +#[pin_data]
> +pub struct Request {
> +    #[pin]
> +    opaque: Opaque<bindings::request>,
> +}
> +
> +impl Request {
> +    unsafe fn from_raw<'a>(ptr: *mut bindings::request) -> Pin<&'a mut Self> {
> +        // SAFETY: the caller should pass a valid `request` pointer.
> +        unsafe { Pin::new_unchecked(&mut *(ptr as *mut Self)) }
> +    }
> +}
> +
> +/// Wrap the block error status values (see [blk_status_t]).
> +///
> +/// [`blk_status_t`]: ../../../../include/linux/blk_types.h
> +#[repr(u32)]
> +#[allow(missing_docs)]
> +pub enum BlkStatus {
> +    Ok,
> +    NotSupp,
> +    TimeOut,
> +    NoSpc,
> +    Transport,
> +    Target,
> +    Nexus,
> +    Medium,
> +    Protection,
> +    Resource,
> +    IoErr,
> +    DmRequeue,
> +    Again,
> +    DevResource,
> +    ZoneResource,
> +    ZoneOpenResource,
> +    ZoneActiveResource,
> +    Offline,
> +    Undefined,
> +}
> +
> +impl From<u32> for BlkStatus {
> +    fn from(value: u32) -> Self {
> +        match value {
> +            0 => Self::Ok,
> +            1 => Self::NotSupp,
> +            2 => Self::TimeOut,
> +            3 => Self::NoSpc,
> +            4 => Self::Transport,
> +            5 => Self::Target,
> +            6 => Self::Nexus,
> +            7 => Self::Medium,
> +            8 => Self::Protection,
> +            9 => Self::Resource,
> +            10 => Self::IoErr,
> +            11 => Self::DmRequeue,
> +            12 => Self::Again,
> +            13 => Self::DevResource,
> +            14 => Self::ZoneResource,
> +            15 => Self::ZoneOpenResource,
> +            16 => Self::ZoneActiveResource,
> +            17 => Self::Offline,
> +            _ => Self::Undefined,
> +        }
> +    }
> +}
> --
> 2.40.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper
  2023-07-14 15:48   ` Benno Lossin
@ 2023-07-17  3:04     ` Qingsong Chen
  0 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-07-17  3:04 UTC (permalink / raw)
  To: Benno Lossin
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Martin Rodriguez Reboredo,
	Alice Ryhl, Vincenzo Palazzo, Asahi Lina, rust-for-linux

On 7/14/23 11:48 PM, Benno Lossin wrote:
>> - Declare `vtable` for basic target operations.
>> - Wrap `target_type` to register/unregister target.
>> - Wrap `dm_target`/`dm_dev` to handle io request.
>> - Add a dummy `bio` wrapper.
>>
>> Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com>
> 
> On my system this patch series (I did not test which patch exactly) does
> not compile.
> 
> I have left some comments below, they show some patterns present in
> the other patches as well.

Thanks for the comments. Maybe `CONFIG_BLK_DEV_ZONED=y` is needed to
compile this patchset. This is because I cannot define two functions
with the same name in a trait (using the `vtable` macro), just like
the C definition:
```c
#ifdef CONFIG_BLK_DEV_ZONED
typedef int (*dm_report_zones_fn) (struct dm_target *ti,
                                     struct dm_report_zones_args *args,
                                     unsigned int nr_zones);
#else
typedef int (*dm_report_zones_fn) (struct dm_target *dummy);
#endif
```
To fix the `vtable`, I send a little patch[1] the other day. Based on
that, the compile error of this patchset should be fixed too.

[1]:
https://lore.kernel.org/rust-for-linux/20230626074242.3945398-1-changxian.cqs@antgroup.com/

>> +/// The return values of target map function, i.e., [`TargetOperations::map`].
>> +#[repr(u32)]
>> +pub enum MapState {
>> +    /// The target will handle the io by resubmitting it later.
>> +    Submitted = bindings::DM_MAPIO_SUBMITTED,
>> +
>> +    /// Simple remap complete.
>> +    Remapped = bindings::DM_MAPIO_REMAPPED,
>> +
>> +    /// The target wants to requeue the io.
>> +    Requeue = bindings::DM_MAPIO_REQUEUE,
>> +
>> +    /// The target wants to requeue the io after a delay.
>> +    DelayRequeue = bindings::DM_MAPIO_DELAY_REQUEUE,
>> +
>> +    /// The target wants to complete the io.
>> +    Kill = bindings::DM_MAPIO_KILL,
>> +}
> 
> Is it really necessary to have these correspond to the exact values?
> Could we also just have a conversion function and leave the repr to
> the compiler?

Sure, we can have a conversion function to return its value to C side
(without `as _`).

Regards,
Qingsong

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations
  2023-07-14 15:49   ` Benno Lossin
@ 2023-07-17  3:42     ` Qingsong Chen
  0 siblings, 0 replies; 18+ messages in thread
From: Qingsong Chen @ 2023-07-17  3:42 UTC (permalink / raw)
  To: Benno Lossin
  Cc: linux-kernel, 田洪亮,
	Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, rust-for-linux

On 7/14/23 11:49 PM, Benno Lossin wrote:
>> @@ -31,6 +31,39 @@ pub trait TargetOperations: Sized {
>>      /// Map block IOs. Return [`MapState`] to indicate how to handle the `bio`
>>      /// later (end or resubmit).
>>      fn map(t: &mut Target<Self>, bio: Pin<&mut Bio>) -> MapState;
>> +
>> +    /// Map `request`. Return [`MapState`] and the optional cloned `request`.
>> +    #[allow(unused)]
>> +    fn clone_and_map_rq<'a>(
>> +        t: &mut Target<Self>,
>> +        rq: Pin<&mut Request>,
>> +        map_ctx: &mut MapInfo,
>> +    ) -> (MapState, Option<Pin<&'a mut Request>>) {
>> +        unimplemented!()
> 
> Note that these should be implemented when you submit this as an actual
> patch. (Not sure what you would do in this case, but throwing a panic is
> not a good idea.) This also applies to all subsequent patches.
> I think you might have done this, because you want the C side to choose
> the default implementation. But it should also be possible to call these
> from the Rust side.

In fact, we don't want users to call these callbacks manually. The 
normal call-chain would be like this: `map_request`(c) ->
`dm_clone_and_map_rq_fn`(unsafe rust) -> `clone_and_map_rq`(rust).
If users don't implement `clone_and_map_rq`, we would check the
corresponding field of `TargetType`, using the `check_target_operations`
macro. So it won't call the default implementation generally.

Regards,
Qingsong

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-07-17  3:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-25 12:16 [RFC PATCH 0/8] Rust device mapper abstractions Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper Qingsong Chen
2023-06-25 12:25   ` Greg KH
2023-06-26  3:26     ` Qingsong Chen
2023-06-26 16:39       ` Boqun Feng
2023-07-14 15:48   ` Benno Lossin
2023-07-17  3:04     ` Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 2/8] rust: kernel: add `request` related TargetOperations Qingsong Chen
2023-07-14 15:49   ` Benno Lossin
2023-07-17  3:42     ` Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 3/8] rust: kernel: add some hook TargetOperations Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 4/8] rust: kernel: add some info handler TargetOperations Qingsong Chen
2023-06-25 16:58   ` kernel test robot
2023-06-25 12:16 ` [RFC PATCH 5/8] rust: kernel: add underlying device related TargetOperations Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 6/8] rust: kernel: add DAX " Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 7/8] rust: kernel: prepare to implement dm target in Rust Qingsong Chen
2023-06-25 12:16 ` [RFC PATCH 8/8] samples: rust: add a device mapper linear target Qingsong Chen
2023-06-30  7:15   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.