rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Binary Large Objects for Rust DebugFS
@ 2025-10-03 22:26 Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial() Danilo Krummrich
                   ` (6 more replies)
  0 siblings, 7 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

This series adds support for exposing binary large objects via Rust debugfs.

The first two patches extend UserSliceReader and UserSliceWriter with partial
read/write helpers.

The series further introduces read_binary_file(), write_binary_file() and
read_write_binary_file() methods for the Dir and ScopedDir types.

It also introduces the BinaryWriter and BinaryReader traits, which are used to
read/write the implementing type's binary representation with the help of the
backing file operations from/to debugfs.

Additional to some more generic blanked implementations for the BinaryWriter and
BinaryReader traits it also provides implementations for common smart pointer
types.

Both samples (file-based and scoped) are updated with corresponding examples.

A branch containing the patches can be found in [1].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=debugfs_blobs

Danilo Krummrich (7):
  rust: uaccess: add UserSliceReader::read_slice_partial()
  rust: uaccess: add UserSliceWriter::write_slice_partial()
  rust: debugfs: support for binary large objects
  rust: debugfs: support blobs from smart pointers
  samples: rust: debugfs: add example for blobs
  rust: debugfs: support binary large objects for ScopedDir
  samples: rust: debugfs_scoped: add example for blobs

 rust/kernel/debugfs.rs              | 112 ++++++++++++++++-
 rust/kernel/debugfs/file_ops.rs     | 144 ++++++++++++++++++++-
 rust/kernel/debugfs/traits.rs       | 186 +++++++++++++++++++++++++++-
 rust/kernel/uaccess.rs              |  29 +++++
 samples/rust/rust_debugfs.rs        |  13 ++
 samples/rust/rust_debugfs_scoped.rs |  14 ++-
 6 files changed, 487 insertions(+), 11 deletions(-)


base-commit: e406d57be7bd2a4e73ea512c1ae36a40a44e499e
-- 
2.51.0


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

* [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial()
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-17 11:11   ` Alice Ryhl
  2025-10-03 22:26 ` [PATCH 2/7] rust: uaccess: add UserSliceWriter::write_slice_partial() Danilo Krummrich
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

The existing read_slice() method is a wrapper around copy_from_user()
and expects the user buffer to be larger than the destination buffer.

However, userspace may split up writes in multiple partial operations
providing an offset into the destination buffer and a smaller user
buffer.

In order to support this common case, provide a helper for partial
reads.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/uaccess.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index a8fb4764185a..1b0b57e855c9 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -287,6 +287,19 @@ pub fn read_slice(&mut self, out: &mut [u8]) -> Result {
         self.read_raw(out)
     }
 
+    /// Reads raw data from the user slice into a kernel buffer partially.
+    ///
+    /// This is the same as [`Self::read_slice`] but considers the given `offset` into `out` and
+    /// truncates the read to the boundaries of `self` and `out`.
+    ///
+    /// On success, returns the number of bytes read.
+    pub fn read_slice_partial(&mut self, out: &mut [u8], offset: usize) -> Result<usize> {
+        let end = offset.checked_add(self.len()).ok_or(EINVAL)?.min(out.len());
+
+        out.get_mut(offset..end)
+            .map_or(Ok(0), |dst| self.read_slice(dst).map(|()| dst.len()))
+    }
+
     /// Reads a value of the specified type.
     ///
     /// Fails with [`EFAULT`] if the read happens on a bad address, or if the read goes out of
-- 
2.51.0


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

* [PATCH 2/7] rust: uaccess: add UserSliceWriter::write_slice_partial()
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial() Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 3/7] rust: debugfs: support for binary large objects Danilo Krummrich
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

The existing write_slice() method is a wrapper around copy_to_user() and
expects the user buffer to be larger than the source buffer.

However, userspace may split up reads in multiple partial operations
providing an offset into the source buffer and a smaller user buffer.

In order to support this common case, provide a helper for partial
writes.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/uaccess.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 1b0b57e855c9..11bb8f3265dd 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -451,6 +451,22 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
         Ok(())
     }
 
+    /// Writes raw data to this user pointer from a kernel buffer partially.
+    ///
+    /// This is the same as [`Self::write_slice`] but considers the given `offset` into `data` and
+    /// truncates the write to the boundaries of `self` and `data`.
+    ///
+    /// On success, returns the number of bytes written.
+    pub fn write_slice_partial(&mut self, data: &[u8], offset: usize) -> Result<usize> {
+        let end = offset
+            .checked_add(self.len())
+            .ok_or(EINVAL)?
+            .min(data.len());
+
+        data.get(offset..end)
+            .map_or(Ok(0), |src| self.write_slice(src).map(|()| src.len()))
+    }
+
     /// Writes the provided Rust value to this userspace pointer.
     ///
     /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
-- 
2.51.0


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

* [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial() Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 2/7] rust: uaccess: add UserSliceWriter::write_slice_partial() Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-17 12:59   ` Alice Ryhl
  2025-10-03 22:26 ` [PATCH 4/7] rust: debugfs: support blobs from smart pointers Danilo Krummrich
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Introduce support for read-only, write-only, and read-write binary files
in Rust debugfs. This adds:

- BinaryWriter and BinaryReader traits for writing to and reading from
  user slices in binary form.
- New Dir methods: read_binary_file(), write_binary_file(),
  `read_write_binary_file`.
- Corresponding FileOps implementations: BinaryReadFile,
  BinaryWriteFile, BinaryReadWriteFile.

This allows kernel modules to expose arbitrary binary data through
debugfs, with proper support for offsets and partial reads/writes.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/debugfs.rs          |  67 ++++++++++++++-
 rust/kernel/debugfs/file_ops.rs | 144 +++++++++++++++++++++++++++++++-
 rust/kernel/debugfs/traits.rs   |  45 +++++++++-
 3 files changed, 249 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 381c23b3dd83..b1a3adca7fd4 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -21,12 +21,15 @@
 use core::ops::Deref;
 
 mod traits;
-pub use traits::{Reader, Writer};
+pub use traits::{BinaryReader, BinaryWriter, Reader, Writer};
 
 mod callback_adapters;
 use callback_adapters::{FormatAdapter, NoWriter, WritableAdapter};
 mod file_ops;
-use file_ops::{FileOps, ReadFile, ReadWriteFile, WriteFile};
+use file_ops::{
+    BinaryReadFile, BinaryReadWriteFile, BinaryWriteFile, FileOps, ReadFile, ReadWriteFile,
+    WriteFile,
+};
 #[cfg(CONFIG_DEBUG_FS)]
 mod entry;
 #[cfg(CONFIG_DEBUG_FS)]
@@ -150,6 +153,33 @@ pub fn read_only_file<'a, T, E: 'a>(
         self.create_file(name, data, file_ops)
     }
 
+    /// Creates a read-only binary file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`BinaryWriter::write_to_slice`] on the value
+    /// initialized by `data`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::c_str;
+    /// # use kernel::debugfs::Dir;
+    /// # use kernel::prelude::*;
+    /// # let dir = Dir::new(c_str!("my_debugfs_dir"));
+    /// let file = KBox::pin_init(dir.read_binary_file(c_str!("foo"), [0x1, 0x2]), GFP_KERNEL)?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn read_binary_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: BinaryWriter + Send + Sync + 'static,
+    {
+        let file_ops = &<T as BinaryReadFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+
     /// Creates a read-only file in this directory, with contents from a callback.
     ///
     /// `f` must be a function item or a non-capturing closure.
@@ -206,6 +236,22 @@ pub fn read_write_file<'a, T, E: 'a>(
         self.create_file(name, data, file_ops)
     }
 
+    /// Creates a read-write binary file in this directory.
+    ///
+    /// Reading the file uses the [`BinaryWriter`] implementation.
+    /// Writing to the file uses the [`BinaryReader`] implementation.
+    pub fn read_write_binary_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: BinaryWriter + BinaryReader + Send + Sync + 'static,
+    {
+        let file_ops = &<T as BinaryReadWriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, file_ops)
+    }
+
     /// Creates a read-write file in this directory, with logic from callbacks.
     ///
     /// Reading from the file is handled by `f`. Writing to the file is handled by `w`.
@@ -248,6 +294,23 @@ pub fn write_only_file<'a, T, E: 'a>(
         self.create_file(name, data, &T::FILE_OPS)
     }
 
+    /// Creates a write-only binary file in this directory.
+    ///
+    /// The file owns its backing data. Writing to the file uses the [`BinaryReader`]
+    /// implementation.
+    ///
+    /// The file is removed when the returned [`File`] is dropped.
+    pub fn write_binary_file<'a, T, E: 'a>(
+        &'a self,
+        name: &'a CStr,
+        data: impl PinInit<T, E> + 'a,
+    ) -> impl PinInit<File<T>, E> + 'a
+    where
+        T: BinaryReader + Send + Sync + 'static,
+    {
+        self.create_file(name, data, &T::FILE_OPS)
+    }
+
     /// Creates a write-only file in this directory, with write logic from a callback.
     ///
     /// `w` must be a function item or a non-capturing closure.
diff --git a/rust/kernel/debugfs/file_ops.rs b/rust/kernel/debugfs/file_ops.rs
index 50fead17b6f3..ef31cac4cd44 100644
--- a/rust/kernel/debugfs/file_ops.rs
+++ b/rust/kernel/debugfs/file_ops.rs
@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2025 Google LLC.
 
-use super::{Reader, Writer};
+use super::{BinaryReader, BinaryWriter, Reader, Writer};
 use crate::debugfs::callback_adapters::Adapter;
 use crate::prelude::*;
 use crate::seq_file::SeqFile;
 use crate::seq_print;
 use crate::uaccess::UserSlice;
-use core::fmt::{Display, Formatter, Result};
+use core::fmt;
 use core::marker::PhantomData;
 
 #[cfg(CONFIG_DEBUG_FS)]
@@ -65,8 +65,8 @@ fn deref(&self) -> &Self::Target {
 
 struct WriterAdapter<T>(T);
 
-impl<'a, T: Writer> Display for WriterAdapter<&'a T> {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+impl<'a, T: Writer> fmt::Display for WriterAdapter<&'a T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.0.write(f)
     }
 }
@@ -245,3 +245,139 @@ impl<T: Reader + Sync> WriteFile<T> for T {
         unsafe { FileOps::new(operations, 0o200) }
     };
 }
+
+extern "C" fn blob_read<T: BinaryWriter>(
+    file: *mut bindings::file,
+    buf: *mut c_char,
+    count: usize,
+    ppos: *mut bindings::loff_t,
+) -> isize {
+    // SAFETY:
+    // - `file` is a valid pointer to a `struct file`.
+    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
+    let this = unsafe { &*((*file).private_data.cast::<T>()) };
+
+    // SAFETY: `ppos` is a valid `loff_t` pointer.
+    let pos = unsafe { &mut *ppos };
+
+    let mut writer = UserSlice::new(UserPtr::from_ptr(buf.cast()), count).writer();
+
+    let ret = || -> Result<isize> {
+        let offset = (*pos).try_into()?;
+
+        let written = this.write_to_slice(&mut writer, offset)?;
+        *pos += bindings::loff_t::try_from(written)?;
+
+        Ok(written.try_into()?)
+    }();
+
+    match ret {
+        Ok(n) => n,
+        Err(e) => e.to_errno() as isize,
+    }
+}
+
+pub(crate) trait BinaryReadFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            read: Some(blob_read::<T>),
+            llseek: Some(bindings::default_llseek),
+            open: Some(bindings::simple_open),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+
+        // SAFETY:
+        // - The private data of `struct inode` does always contain a pointer to a valid `T`.
+        // - `simple_open()` stores the `struct inode`'s private data in the private data of the
+        //   corresponding `struct file`.
+        // - `blob_read()` re-creates a reference to `T` from the `struct file`'s private data.
+        // - `default_llseek()` does not access the `struct file`'s private data.
+        unsafe { FileOps::new(operations, 0o400) }
+    };
+}
+
+extern "C" fn blob_write<T: BinaryReader>(
+    file: *mut bindings::file,
+    buf: *const c_char,
+    count: usize,
+    ppos: *mut bindings::loff_t,
+) -> isize {
+    // SAFETY:
+    // - `file` is a valid pointer to a `struct file`.
+    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
+    let this = unsafe { &*((*file).private_data.cast::<T>()) };
+
+    // SAFETY: `ppos` is a valid `loff_t` pointer.
+    let pos = unsafe { &mut *ppos };
+
+    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
+
+    let ret = || -> Result<isize> {
+        let offset = (*pos).try_into()?;
+
+        let read = this.read_from_slice(&mut reader, offset)?;
+        *pos += bindings::loff_t::try_from(read)?;
+
+        Ok(read.try_into()?)
+    }();
+
+    match ret {
+        Ok(n) => n,
+        Err(e) => e.to_errno() as isize,
+    }
+}
+
+pub(crate) trait BinaryWriteFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            write: Some(blob_write::<T>),
+            llseek: Some(bindings::default_llseek),
+            open: Some(bindings::simple_open),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+
+        // SAFETY:
+        // - The private data of `struct inode` does always contain a pointer to a valid `T`.
+        // - `simple_open()` stores the `struct inode`'s private data in the private data of the
+        //   corresponding `struct file`.
+        // - `blob_write()` re-creates a reference to `T` from the `struct file`'s private data.
+        // - `default_llseek()` does not access the `struct file`'s private data.
+        unsafe { FileOps::new(operations, 0o200) }
+    };
+}
+
+pub(crate) trait BinaryReadWriteFile<T> {
+    const FILE_OPS: FileOps<T>;
+}
+
+impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T {
+    const FILE_OPS: FileOps<T> = {
+        let operations = bindings::file_operations {
+            read: Some(blob_read::<T>),
+            write: Some(blob_write::<T>),
+            llseek: Some(bindings::default_llseek),
+            open: Some(bindings::simple_open),
+            // SAFETY: `file_operations` supports zeroes in all fields.
+            ..unsafe { core::mem::zeroed() }
+        };
+
+        // SAFETY:
+        // - The private data of `struct inode` does always contain a pointer to a valid `T`.
+        // - `simple_open()` stores the `struct inode`'s private data in the private data of the
+        //   corresponding `struct file`.
+        // - `blob_read()` re-creates a reference to `T` from the `struct file`'s private data.
+        // - `blob_write()` re-creates a reference to `T` from the `struct file`'s private data.
+        // - `default_llseek()` does not access the `struct file`'s private data.
+        unsafe { FileOps::new(operations, 0o600) }
+    };
+}
diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
index ab009eb254b3..60a6ee6c6b58 100644
--- a/rust/kernel/debugfs/traits.rs
+++ b/rust/kernel/debugfs/traits.rs
@@ -5,7 +5,8 @@
 
 use crate::prelude::*;
 use crate::sync::Mutex;
-use crate::uaccess::UserSliceReader;
+use crate::transmute::{AsBytes, FromBytes};
+use crate::uaccess::{UserSliceReader, UserSliceWriter};
 use core::fmt::{self, Debug, Formatter};
 use core::str::FromStr;
 use core::sync::atomic::{
@@ -39,6 +40,30 @@ fn write(&self, f: &mut Formatter<'_>) -> fmt::Result {
     }
 }
 
+/// Trait for types that can be written out as binary.
+pub trait BinaryWriter {
+    /// Writes the binary form of `self` into `writer`.
+    ///
+    /// `offset` is the requested offset into the binary representation of `self`.
+    ///
+    /// On success, returns the number of bytes written in to `writer`.
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize>;
+}
+
+impl<T: AsBytes> BinaryWriter for T {
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        writer.write_slice_partial(self.as_bytes(), offset)
+    }
+}
+
+impl<T: BinaryWriter> BinaryWriter for Mutex<T> {
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        let guard = self.lock();
+
+        guard.write_to_slice(writer, offset)
+    }
+}
+
 /// A trait for types that can be updated from a user slice.
 ///
 /// This works similarly to `FromStr`, but operates on a `UserSliceReader` rather than a &str.
@@ -66,6 +91,24 @@ fn read_from_slice(&self, reader: &mut UserSliceReader) -> Result {
     }
 }
 
+/// Trait for types that can be constructed from a binary representation.
+pub trait BinaryReader {
+    /// Reads the binary form of `self` from `reader`.
+    ///
+    /// `offset` is the requested offset into the binary representation of `self`.
+    ///
+    /// On success, returns the number of bytes read from `reader`.
+    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize>;
+}
+
+impl<T: AsBytes + FromBytes> BinaryReader for Mutex<T> {
+    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
+        let mut this = self.lock();
+
+        reader.read_slice_partial(this.as_bytes_mut(), offset)
+    }
+}
+
 macro_rules! impl_reader_for_atomic {
     ($(($atomic_type:ty, $int_type:ty)),*) => {
         $(
-- 
2.51.0


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

* [PATCH 4/7] rust: debugfs: support blobs from smart pointers
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
                   ` (2 preceding siblings ...)
  2025-10-03 22:26 ` [PATCH 3/7] rust: debugfs: support for binary large objects Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-03 23:12   ` Matthew Maurer
  2025-10-03 22:26 ` [PATCH 5/7] samples: rust: debugfs: add example for blobs Danilo Krummrich
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Extend Rust debugfs binary support to allow exposing data stored in
common smart pointers and heap-allocated collections.

- Implement BinaryWriter for Box<T>, Pin<Box<T>>, Arc<T>, and Vec<T>.
- Introduce BinaryReaderMut for mutable binary access with outer locks.
- Implement BinaryReaderMut for Box<T>, Vec<T>, and base types.
- Update BinaryReader to delegate to BinaryReaderMut for Mutex<T>,
  Box<T>, Pin<Box<T>> and Arc<T>.

This enables debugfs files to directly expose or update data stored
inside heap-allocated, reference-counted, or lock-protected containers
without manual dereferencing or locking.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/debugfs.rs        |   2 +-
 rust/kernel/debugfs/traits.rs | 145 +++++++++++++++++++++++++++++++++-
 2 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index b1a3adca7fd4..3c3bbcc126ef 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -21,7 +21,7 @@
 use core::ops::Deref;
 
 mod traits;
-pub use traits::{BinaryReader, BinaryWriter, Reader, Writer};
+pub use traits::{BinaryReader, BinaryReaderMut, BinaryWriter, Reader, Writer};
 
 mod callback_adapters;
 use callback_adapters::{FormatAdapter, NoWriter, WritableAdapter};
diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
index 60a6ee6c6b58..bcd0a9db3cc9 100644
--- a/rust/kernel/debugfs/traits.rs
+++ b/rust/kernel/debugfs/traits.rs
@@ -3,11 +3,14 @@
 
 //! Traits for rendering or updating values exported to DebugFS.
 
+use crate::alloc::Allocator;
 use crate::prelude::*;
+use crate::sync::Arc;
 use crate::sync::Mutex;
 use crate::transmute::{AsBytes, FromBytes};
 use crate::uaccess::{UserSliceReader, UserSliceWriter};
 use core::fmt::{self, Debug, Formatter};
+use core::ops::{Deref, DerefMut};
 use core::str::FromStr;
 use core::sync::atomic::{
     AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, AtomicU64,
@@ -50,12 +53,14 @@ pub trait BinaryWriter {
     fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize>;
 }
 
+// Base implementation for any `T: AsBytes`.
 impl<T: AsBytes> BinaryWriter for T {
     fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
         writer.write_slice_partial(self.as_bytes(), offset)
     }
 }
 
+// Delegate for `Mutex<T>`: Support a `T` with an outer mutex.
 impl<T: BinaryWriter> BinaryWriter for Mutex<T> {
     fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
         let guard = self.lock();
@@ -64,6 +69,56 @@ fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<
     }
 }
 
+// Delegate for `Box<T, A>`: Support a `Box<T, A>` with no lock or an inner lock.
+impl<T, A> BinaryWriter for Box<T, A>
+where
+    T: BinaryWriter,
+    A: Allocator,
+{
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        self.deref().write_to_slice(writer, offset)
+    }
+}
+
+// Delegate for `Pin<Box<T, A>>`: Support a `Pin<Box<T, A>>` with no lock or an inner lock.
+impl<T, A> BinaryWriter for Pin<Box<T, A>>
+where
+    T: BinaryWriter,
+    A: Allocator,
+{
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        self.deref().write_to_slice(writer, offset)
+    }
+}
+
+// Delegate for `Arc<T>`: Support a `Arc<T>` with no lock or an inner lock.
+impl<T> BinaryWriter for Arc<T>
+where
+    T: BinaryWriter,
+{
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        self.deref().write_to_slice(writer, offset)
+    }
+}
+
+// Delegate for `Vec<T, A>`.
+impl<T, A> BinaryWriter for Vec<T, A>
+where
+    T: AsBytes,
+    A: Allocator,
+{
+    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
+        let slice = self.as_slice();
+
+        // SAFETY: `T: AsBytes` allows us to treat `&[T]` as `&[u8]`.
+        let buffer = unsafe {
+            core::slice::from_raw_parts(slice.as_ptr().cast(), core::mem::size_of_val(slice))
+        };
+
+        writer.write_slice_partial(buffer, offset)
+    }
+}
+
 /// A trait for types that can be updated from a user slice.
 ///
 /// This works similarly to `FromStr`, but operates on a `UserSliceReader` rather than a &str.
@@ -92,6 +147,70 @@ fn read_from_slice(&self, reader: &mut UserSliceReader) -> Result {
 }
 
 /// Trait for types that can be constructed from a binary representation.
+///
+/// See also [`BinaryReader`] for interior mutability.
+pub trait BinaryReaderMut {
+    /// Reads the binary form of `self` from `reader`.
+    ///
+    /// Same as [`BinaryReader::read_from_slice`], but takes a mutable reference.
+    ///
+    /// `offset` is the requested offset into the binary representation of `self`.
+    ///
+    /// On success, returns the number of bytes read from `reader`.
+    fn read_from_slice_mut(&mut self, reader: &mut UserSliceReader, offset: usize)
+        -> Result<usize>;
+}
+
+// Base implementation for any `T: AsBytes + FromBytes`.
+impl<T: AsBytes + FromBytes> BinaryReaderMut for T {
+    fn read_from_slice_mut(
+        &mut self,
+        reader: &mut UserSliceReader,
+        offset: usize,
+    ) -> Result<usize> {
+        reader.read_slice_partial(self.as_bytes_mut(), offset)
+    }
+}
+
+// Delegate for `Box<T, A>`: Support a `Box<T, A>` with an outer lock.
+impl<T: ?Sized + BinaryReaderMut, A: Allocator> BinaryReaderMut for Box<T, A> {
+    fn read_from_slice_mut(
+        &mut self,
+        reader: &mut UserSliceReader,
+        offset: usize,
+    ) -> Result<usize> {
+        self.deref_mut().read_from_slice_mut(reader, offset)
+    }
+}
+
+// Delegate for `Vec<T, A>`: Support a `Vec<T, A>` with an outer lock.
+impl<T, A> BinaryReaderMut for Vec<T, A>
+where
+    T: AsBytes + FromBytes,
+    A: Allocator,
+{
+    fn read_from_slice_mut(
+        &mut self,
+        reader: &mut UserSliceReader,
+        offset: usize,
+    ) -> Result<usize> {
+        let slice = self.as_mut_slice();
+
+        // SAFETY: `T: AsBytes` allows us to treat `&[T]` as `&[u8]`.
+        let buffer = unsafe {
+            core::slice::from_raw_parts_mut(
+                slice.as_mut_ptr().cast(),
+                core::mem::size_of_val(slice),
+            )
+        };
+
+        reader.read_slice_partial(buffer, offset)
+    }
+}
+
+/// Trait for types that can be constructed from a binary representation.
+///
+/// See also [`BinaryReaderMut`] for the mutable version.
 pub trait BinaryReader {
     /// Reads the binary form of `self` from `reader`.
     ///
@@ -101,11 +220,33 @@ pub trait BinaryReader {
     fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize>;
 }
 
-impl<T: AsBytes + FromBytes> BinaryReader for Mutex<T> {
+// Delegate for `Mutex<T>`: Support a `T` with an outer `Mutex`.
+impl<T: BinaryReaderMut> BinaryReader for Mutex<T> {
     fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
         let mut this = self.lock();
 
-        reader.read_slice_partial(this.as_bytes_mut(), offset)
+        this.read_from_slice_mut(reader, offset)
+    }
+}
+
+// Delegate for `Box<T, A>`: Support a `Box<T, A>` with an inner lock.
+impl<T: ?Sized + BinaryReader, A: Allocator> BinaryReader for Box<T, A> {
+    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
+        self.deref().read_from_slice(reader, offset)
+    }
+}
+
+// Delegate for `Pin<Box<T, A>>`: Support a `Pin<Box<T, A>>` with an inner lock.
+impl<T: ?Sized + BinaryReader, A: Allocator> BinaryReader for Pin<Box<T, A>> {
+    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
+        self.deref().read_from_slice(reader, offset)
+    }
+}
+
+// Delegate for `Arc<T>`: Support an `Arc<T>` with an inner lock.
+impl<T: ?Sized + BinaryReader> BinaryReader for Arc<T> {
+    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
+        self.deref().read_from_slice(reader, offset)
     }
 }
 
-- 
2.51.0


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

* [PATCH 5/7] samples: rust: debugfs: add example for blobs
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
                   ` (3 preceding siblings ...)
  2025-10-03 22:26 ` [PATCH 4/7] rust: debugfs: support blobs from smart pointers Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-20 10:01   ` Alice Ryhl
  2025-10-03 22:26 ` [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir Danilo Krummrich
  2025-10-03 22:26 ` [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs Danilo Krummrich
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Extend the Rust debugfs sample to demonstrate usage of binary file
support. The example now shows how to expose both fixed-size arrays
and dynamically sized vectors as binary blobs in debugfs.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 samples/rust/rust_debugfs.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/samples/rust/rust_debugfs.rs b/samples/rust/rust_debugfs.rs
index 82b61a15a34b..75ceb95276fa 100644
--- a/samples/rust/rust_debugfs.rs
+++ b/samples/rust/rust_debugfs.rs
@@ -38,6 +38,7 @@
 use kernel::debugfs::{Dir, File};
 use kernel::new_mutex;
 use kernel::prelude::*;
+use kernel::sizes::*;
 use kernel::sync::Mutex;
 
 use kernel::{acpi, device::Core, of, platform, str::CString, types::ARef};
@@ -62,6 +63,10 @@ struct RustDebugFs {
     counter: File<AtomicUsize>,
     #[pin]
     inner: File<Mutex<Inner>>,
+    #[pin]
+    array_blob: File<Mutex<[u8; 4]>>,
+    #[pin]
+    vector_blob: File<Mutex<KVec<u8>>>,
 }
 
 #[derive(Debug)]
@@ -143,6 +148,14 @@ fn new(pdev: &platform::Device<Core>) -> impl PinInit<Self, Error> + '_ {
                 ),
                 counter <- Self::build_counter(&debugfs),
                 inner <- Self::build_inner(&debugfs),
+                array_blob <- debugfs.read_write_binary_file(
+                    c_str!("array_blob"),
+                    new_mutex!([0x62, 0x6c, 0x6f, 0x62]),
+                ),
+                vector_blob <- debugfs.read_write_binary_file(
+                    c_str!("vector_blob"),
+                    new_mutex!(kernel::kvec!(0x42; SZ_4K)?),
+                ),
                 _debugfs: debugfs,
                 pdev: pdev.into(),
             }
-- 
2.51.0


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

* [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
                   ` (4 preceding siblings ...)
  2025-10-03 22:26 ` [PATCH 5/7] samples: rust: debugfs: add example for blobs Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-17 13:00   ` Alice Ryhl
  2025-10-03 22:26 ` [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs Danilo Krummrich
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Add support for creating binary debugfs files via ScopedDir. This
mirrors the existing functionality for Dir, but without producing an
owning handle -- files are automatically removed when the associated
Scope is dropped.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/debugfs.rs | 45 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
index 3c3bbcc126ef..0eb1719e4953 100644
--- a/rust/kernel/debugfs.rs
+++ b/rust/kernel/debugfs.rs
@@ -531,6 +531,20 @@ pub fn read_only_file<T: Writer + Send + Sync + 'static>(&self, name: &CStr, dat
         self.create_file(name, data, &T::FILE_OPS)
     }
 
+    /// Creates a read-only binary file in this directory.
+    ///
+    /// The file's contents are produced by invoking [`BinaryWriter::write_to_slice`].
+    ///
+    /// This function does not produce an owning handle to the file. The created file is removed
+    /// when the [`Scope`] that this directory belongs to is dropped.
+    pub fn read_binary_file<T: BinaryWriter + Send + Sync + 'static>(
+        &self,
+        name: &CStr,
+        data: &'data T,
+    ) {
+        self.create_file(name, data, &T::FILE_OPS)
+    }
+
     /// Creates a read-only file in this directory, with contents from a callback.
     ///
     /// The file contents are generated by calling `f` with `data`.
@@ -568,6 +582,22 @@ pub fn read_write_file<T: Writer + Reader + Send + Sync + 'static>(
         self.create_file(name, data, vtable)
     }
 
+    /// Creates a read-write binary file in this directory.
+    ///
+    /// Reading the file uses the [`BinaryWriter`] implementation on `data`. Writing to the file
+    /// uses the [`BinaryReader`] implementation on `data`.
+    ///
+    /// This function does not produce an owning handle to the file. The created file is removed
+    /// when the [`Scope`] that this directory belongs to is dropped.
+    pub fn read_write_binary_file<T: BinaryWriter + BinaryReader + Send + Sync + 'static>(
+        &self,
+        name: &CStr,
+        data: &'data T,
+    ) {
+        let vtable = &<T as BinaryReadWriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, vtable)
+    }
+
     /// Creates a read-write file in this directory, with logic from callbacks.
     ///
     /// Reading from the file is handled by `f`. Writing to the file is handled by `w`.
@@ -607,6 +637,21 @@ pub fn write_only_file<T: Reader + Send + Sync + 'static>(&self, name: &CStr, da
         self.create_file(name, data, vtable)
     }
 
+    /// Creates a write-only binary file in this directory.
+    ///
+    /// Writing to the file uses the [`BinaryReader`] implementation on `data`.
+    ///
+    /// This function does not produce an owning handle to the file. The created file is removed
+    /// when the [`Scope`] that this directory belongs to is dropped.
+    pub fn write_binary_file<T: BinaryReader + Send + Sync + 'static>(
+        &self,
+        name: &CStr,
+        data: &'data T,
+    ) {
+        let vtable = &<T as BinaryWriteFile<_>>::FILE_OPS;
+        self.create_file(name, data, vtable)
+    }
+
     /// Creates a write-only file in this directory, with write logic from a callback.
     ///
     /// Writing to the file is handled by `w`.
-- 
2.51.0


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

* [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs
  2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
                   ` (5 preceding siblings ...)
  2025-10-03 22:26 ` [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir Danilo Krummrich
@ 2025-10-03 22:26 ` Danilo Krummrich
  2025-10-20 10:02   ` Alice Ryhl
  6 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 22:26 UTC (permalink / raw)
  To: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, mmaurer
  Cc: rust-for-linux, linux-kernel, Danilo Krummrich

Extend the rust_debugfs_scoped sample to demonstrate how to export a
large binary object through a ScopedDir.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 samples/rust/rust_debugfs_scoped.rs | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/samples/rust/rust_debugfs_scoped.rs b/samples/rust/rust_debugfs_scoped.rs
index b0c4e76b123e..c80312cf168d 100644
--- a/samples/rust/rust_debugfs_scoped.rs
+++ b/samples/rust/rust_debugfs_scoped.rs
@@ -9,6 +9,7 @@
 use core::sync::atomic::AtomicUsize;
 use kernel::debugfs::{Dir, Scope};
 use kernel::prelude::*;
+use kernel::sizes::*;
 use kernel::sync::Mutex;
 use kernel::{c_str, new_mutex, str::CString};
 
@@ -66,18 +67,22 @@ fn create_file_write(
             GFP_KERNEL,
         )?;
     }
+    let blob = KBox::pin_init(new_mutex!([0x42; SZ_4K]), GFP_KERNEL)?;
 
     let scope = KBox::pin_init(
-        mod_data
-            .device_dir
-            .scope(DeviceData { name, nums }, &file_name, |dev_data, dir| {
+        mod_data.device_dir.scope(
+            DeviceData { name, nums, blob },
+            &file_name,
+            |dev_data, dir| {
                 for (idx, val) in dev_data.nums.iter().enumerate() {
                     let Ok(name) = CString::try_from_fmt(fmt!("{idx}")) else {
                         return;
                     };
                     dir.read_write_file(&name, val);
                 }
-            }),
+                dir.read_write_binary_file(c_str!("blob"), &dev_data.blob);
+            },
+        ),
         GFP_KERNEL,
     )?;
     (*mod_data.devices.lock()).push(scope, GFP_KERNEL)?;
@@ -110,6 +115,7 @@ fn init(device_dir: Dir) -> impl PinInit<Self> {
 struct DeviceData {
     name: CString,
     nums: KVec<AtomicUsize>,
+    blob: Pin<KBox<Mutex<[u8; SZ_4K]>>>,
 }
 
 fn init_control(base_dir: &Dir, dyn_dirs: Dir) -> impl PinInit<Scope<ModuleData>> + '_ {
-- 
2.51.0


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

* Re: [PATCH 4/7] rust: debugfs: support blobs from smart pointers
  2025-10-03 22:26 ` [PATCH 4/7] rust: debugfs: support blobs from smart pointers Danilo Krummrich
@ 2025-10-03 23:12   ` Matthew Maurer
  2025-10-03 23:26     ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Matthew Maurer @ 2025-10-03 23:12 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux,
	linux-kernel

On Fri, Oct 3, 2025 at 3:27 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Extend Rust debugfs binary support to allow exposing data stored in
> common smart pointers and heap-allocated collections.
>
> - Implement BinaryWriter for Box<T>, Pin<Box<T>>, Arc<T>, and Vec<T>.
> - Introduce BinaryReaderMut for mutable binary access with outer locks.
> - Implement BinaryReaderMut for Box<T>, Vec<T>, and base types.
> - Update BinaryReader to delegate to BinaryReaderMut for Mutex<T>,
>   Box<T>, Pin<Box<T>> and Arc<T>.
>
> This enables debugfs files to directly expose or update data stored
> inside heap-allocated, reference-counted, or lock-protected containers
> without manual dereferencing or locking.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/debugfs.rs        |   2 +-
>  rust/kernel/debugfs/traits.rs | 145 +++++++++++++++++++++++++++++++++-
>  2 files changed, 144 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index b1a3adca7fd4..3c3bbcc126ef 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -21,7 +21,7 @@
>  use core::ops::Deref;
>
>  mod traits;
> -pub use traits::{BinaryReader, BinaryWriter, Reader, Writer};
> +pub use traits::{BinaryReader, BinaryReaderMut, BinaryWriter, Reader, Writer};
>
>  mod callback_adapters;
>  use callback_adapters::{FormatAdapter, NoWriter, WritableAdapter};
> diff --git a/rust/kernel/debugfs/traits.rs b/rust/kernel/debugfs/traits.rs
> index 60a6ee6c6b58..bcd0a9db3cc9 100644
> --- a/rust/kernel/debugfs/traits.rs
> +++ b/rust/kernel/debugfs/traits.rs
> @@ -3,11 +3,14 @@
>
>  //! Traits for rendering or updating values exported to DebugFS.
>
> +use crate::alloc::Allocator;
>  use crate::prelude::*;
> +use crate::sync::Arc;
>  use crate::sync::Mutex;
>  use crate::transmute::{AsBytes, FromBytes};
>  use crate::uaccess::{UserSliceReader, UserSliceWriter};
>  use core::fmt::{self, Debug, Formatter};
> +use core::ops::{Deref, DerefMut};
>  use core::str::FromStr;
>  use core::sync::atomic::{
>      AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, AtomicU64,
> @@ -50,12 +53,14 @@ pub trait BinaryWriter {
>      fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize>;
>  }
>
> +// Base implementation for any `T: AsBytes`.
>  impl<T: AsBytes> BinaryWriter for T {
>      fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>          writer.write_slice_partial(self.as_bytes(), offset)
>      }
>  }
>
> +// Delegate for `Mutex<T>`: Support a `T` with an outer mutex.
>  impl<T: BinaryWriter> BinaryWriter for Mutex<T> {
>      fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>          let guard = self.lock();
> @@ -64,6 +69,56 @@ fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<
>      }
>  }
>
> +// Delegate for `Box<T, A>`: Support a `Box<T, A>` with no lock or an inner lock.
> +impl<T, A> BinaryWriter for Box<T, A>
> +where
> +    T: BinaryWriter,
> +    A: Allocator,
> +{
> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
> +        self.deref().write_to_slice(writer, offset)
> +    }
> +}
> +
> +// Delegate for `Pin<Box<T, A>>`: Support a `Pin<Box<T, A>>` with no lock or an inner lock.
> +impl<T, A> BinaryWriter for Pin<Box<T, A>>
> +where
> +    T: BinaryWriter,
> +    A: Allocator,
> +{
> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
> +        self.deref().write_to_slice(writer, offset)
> +    }
> +}
> +
> +// Delegate for `Arc<T>`: Support a `Arc<T>` with no lock or an inner lock.
> +impl<T> BinaryWriter for Arc<T>
> +where
> +    T: BinaryWriter,
> +{
> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
> +        self.deref().write_to_slice(writer, offset)
> +    }
> +}

For `Box`, `Pin<Box<`, and `Arc`, where the only operation being
performed is to deref, is there a reason that we couldn't have the
`File` object be *inside* the object, thus avoiding any need for
these? I can't see them causing trouble, but

```
Box<File<T>>
Pin<Box<File<T>>>
Arc<File<T>>
```

seem like they'd usually be fine. The one caveat I can think of is
that if you had other functions that wanted to take an `&Arc<T>` for
operations on the Arc, then having an `Arc<File<T>>` would be
suboptimal. Am I missing something?

Depending on the use case I'm missing, would a blanket implementation
for `T: Deref` in this case and `DerefMut` later on make more sense?
That should contract these into a single definition and generalize to
e.g. `ListArc` without further code.

> +
> +// Delegate for `Vec<T, A>`.
> +impl<T, A> BinaryWriter for Vec<T, A>
> +where
> +    T: AsBytes,
> +    A: Allocator,
> +{
> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
> +        let slice = self.as_slice();
> +
> +        // SAFETY: `T: AsBytes` allows us to treat `&[T]` as `&[u8]`.
> +        let buffer = unsafe {
> +            core::slice::from_raw_parts(slice.as_ptr().cast(), core::mem::size_of_val(slice))
> +        };
> +
> +        writer.write_slice_partial(buffer, offset)
> +    }
> +}
> +
>  /// A trait for types that can be updated from a user slice.
>  ///
>  /// This works similarly to `FromStr`, but operates on a `UserSliceReader` rather than a &str.
> @@ -92,6 +147,70 @@ fn read_from_slice(&self, reader: &mut UserSliceReader) -> Result {
>  }
>
>  /// Trait for types that can be constructed from a binary representation.
> +///
> +/// See also [`BinaryReader`] for interior mutability.
> +pub trait BinaryReaderMut {
> +    /// Reads the binary form of `self` from `reader`.
> +    ///
> +    /// Same as [`BinaryReader::read_from_slice`], but takes a mutable reference.
> +    ///
> +    /// `offset` is the requested offset into the binary representation of `self`.
> +    ///
> +    /// On success, returns the number of bytes read from `reader`.
> +    fn read_from_slice_mut(&mut self, reader: &mut UserSliceReader, offset: usize)
> +        -> Result<usize>;
> +}
> +
> +// Base implementation for any `T: AsBytes + FromBytes`.
> +impl<T: AsBytes + FromBytes> BinaryReaderMut for T {
> +    fn read_from_slice_mut(
> +        &mut self,
> +        reader: &mut UserSliceReader,
> +        offset: usize,
> +    ) -> Result<usize> {
> +        reader.read_slice_partial(self.as_bytes_mut(), offset)
> +    }
> +}
> +
> +// Delegate for `Box<T, A>`: Support a `Box<T, A>` with an outer lock.
> +impl<T: ?Sized + BinaryReaderMut, A: Allocator> BinaryReaderMut for Box<T, A> {
> +    fn read_from_slice_mut(
> +        &mut self,
> +        reader: &mut UserSliceReader,
> +        offset: usize,
> +    ) -> Result<usize> {
> +        self.deref_mut().read_from_slice_mut(reader, offset)
> +    }
> +}
> +
> +// Delegate for `Vec<T, A>`: Support a `Vec<T, A>` with an outer lock.
> +impl<T, A> BinaryReaderMut for Vec<T, A>
> +where
> +    T: AsBytes + FromBytes,
> +    A: Allocator,
> +{
> +    fn read_from_slice_mut(
> +        &mut self,
> +        reader: &mut UserSliceReader,
> +        offset: usize,
> +    ) -> Result<usize> {
> +        let slice = self.as_mut_slice();
> +

Nit: This is safe, but it also requires `FromBytes`, and is &mut, &mut

> +        // SAFETY: `T: AsBytes` allows us to treat `&[T]` as `&[u8]`.
> +        let buffer = unsafe {
> +            core::slice::from_raw_parts_mut(
> +                slice.as_mut_ptr().cast(),
> +                core::mem::size_of_val(slice),
> +            )
> +        };
> +
> +        reader.read_slice_partial(buffer, offset)
> +    }
> +}
> +
> +/// Trait for types that can be constructed from a binary representation.
> +///
> +/// See also [`BinaryReaderMut`] for the mutable version.
>  pub trait BinaryReader {
>      /// Reads the binary form of `self` from `reader`.
>      ///
> @@ -101,11 +220,33 @@ pub trait BinaryReader {
>      fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize>;
>  }
>
> -impl<T: AsBytes + FromBytes> BinaryReader for Mutex<T> {
> +// Delegate for `Mutex<T>`: Support a `T` with an outer `Mutex`.
> +impl<T: BinaryReaderMut> BinaryReader for Mutex<T> {
>      fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
>          let mut this = self.lock();
>
> -        reader.read_slice_partial(this.as_bytes_mut(), offset)
> +        this.read_from_slice_mut(reader, offset)
> +    }
> +}
> +
> +// Delegate for `Box<T, A>`: Support a `Box<T, A>` with an inner lock.
> +impl<T: ?Sized + BinaryReader, A: Allocator> BinaryReader for Box<T, A> {
> +    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
> +        self.deref().read_from_slice(reader, offset)
> +    }
> +}
> +
> +// Delegate for `Pin<Box<T, A>>`: Support a `Pin<Box<T, A>>` with an inner lock.
> +impl<T: ?Sized + BinaryReader, A: Allocator> BinaryReader for Pin<Box<T, A>> {
> +    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
> +        self.deref().read_from_slice(reader, offset)
> +    }
> +}
> +
> +// Delegate for `Arc<T>`: Support an `Arc<T>` with an inner lock.
> +impl<T: ?Sized + BinaryReader> BinaryReader for Arc<T> {
> +    fn read_from_slice(&self, reader: &mut UserSliceReader, offset: usize) -> Result<usize> {
> +        self.deref().read_from_slice(reader, offset)
>      }
>  }
>
> --
> 2.51.0
>

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

* Re: [PATCH 4/7] rust: debugfs: support blobs from smart pointers
  2025-10-03 23:12   ` Matthew Maurer
@ 2025-10-03 23:26     ` Danilo Krummrich
  2025-10-03 23:36       ` Matthew Maurer
  0 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-03 23:26 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux,
	linux-kernel

On Sat Oct 4, 2025 at 1:12 AM CEST, Matthew Maurer wrote:
> On Fri, Oct 3, 2025 at 3:27 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> +// Base implementation for any `T: AsBytes`.
>>  impl<T: AsBytes> BinaryWriter for T {
>>      fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>>          writer.write_slice_partial(self.as_bytes(), offset)
>>      }
>>  }
>>
>> +// Delegate for `Mutex<T>`: Support a `T` with an outer mutex.
>>  impl<T: BinaryWriter> BinaryWriter for Mutex<T> {
>>      fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>>          let guard = self.lock();
>> @@ -64,6 +69,56 @@ fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<
>>      }
>>  }
>>
>> +// Delegate for `Box<T, A>`: Support a `Box<T, A>` with no lock or an inner lock.
>> +impl<T, A> BinaryWriter for Box<T, A>
>> +where
>> +    T: BinaryWriter,
>> +    A: Allocator,
>> +{
>> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>> +        self.deref().write_to_slice(writer, offset)
>> +    }
>> +}
>> +
>> +// Delegate for `Pin<Box<T, A>>`: Support a `Pin<Box<T, A>>` with no lock or an inner lock.
>> +impl<T, A> BinaryWriter for Pin<Box<T, A>>
>> +where
>> +    T: BinaryWriter,
>> +    A: Allocator,
>> +{
>> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>> +        self.deref().write_to_slice(writer, offset)
>> +    }
>> +}
>> +
>> +// Delegate for `Arc<T>`: Support a `Arc<T>` with no lock or an inner lock.
>> +impl<T> BinaryWriter for Arc<T>
>> +where
>> +    T: BinaryWriter,
>> +{
>> +    fn write_to_slice(&self, writer: &mut UserSliceWriter, offset: usize) -> Result<usize> {
>> +        self.deref().write_to_slice(writer, offset)
>> +    }
>> +}
>
> For `Box`, `Pin<Box<`, and `Arc`, where the only operation being
> performed is to deref, is there a reason that we couldn't have the
> `File` object be *inside* the object, thus avoiding any need for
> these? I can't see them causing trouble, but
>
> ```
> Box<File<T>>
> Pin<Box<File<T>>>
> Arc<File<T>>
> ```
>
> seem like they'd usually be fine. The one caveat I can think of is
> that if you had other functions that wanted to take an `&Arc<T>` for
> operations on the Arc, then having an `Arc<File<T>>` would be
> suboptimal. Am I missing something?

I think this way around is not compatible with scoped API.

Besides that, there is a semantical difference between File<Arc<T>> and
Arc<File<T>>, i.e. the file itself would become reference counted.

> Depending on the use case I'm missing, would a blanket implementation
> for `T: Deref` in this case and `DerefMut` later on make more sense?
> That should contract these into a single definition and generalize to
> e.g. `ListArc` without further code.

It was also my first thought to generalize over T: Deref, but unfortunately this
does lead to conflicting implementations of BinaryWriter.

>> +// Delegate for `Vec<T, A>`: Support a `Vec<T, A>` with an outer lock.
>> +impl<T, A> BinaryReaderMut for Vec<T, A>
>> +where
>> +    T: AsBytes + FromBytes,
>> +    A: Allocator,
>> +{
>> +    fn read_from_slice_mut(
>> +        &mut self,
>> +        reader: &mut UserSliceReader,
>> +        offset: usize,
>> +    ) -> Result<usize> {
>> +        let slice = self.as_mut_slice();
>> +
>
> Nit: This is safe, but it also requires `FromBytes`, and is &mut, &mut

It already requires T: AsBytes + FromBytes above. Or do you mean something else?

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

* Re: [PATCH 4/7] rust: debugfs: support blobs from smart pointers
  2025-10-03 23:26     ` Danilo Krummrich
@ 2025-10-03 23:36       ` Matthew Maurer
  0 siblings, 0 replies; 32+ messages in thread
From: Matthew Maurer @ 2025-10-03 23:36 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux,
	linux-kernel

>
> I think this way around is not compatible with scoped API.
>
I thought you'd just be able to deref inside the callback, but perhaps
I'm wrong. I'll try to find a chance to pull down the patch and check.

> >> +// Delegate for `Vec<T, A>`: Support a `Vec<T, A>` with an outer lock.
> >> +impl<T, A> BinaryReaderMut for Vec<T, A>
> >> +where
> >> +    T: AsBytes + FromBytes,
> >> +    A: Allocator,
> >> +{
> >> +    fn read_from_slice_mut(
> >> +        &mut self,
> >> +        reader: &mut UserSliceReader,
> >> +        offset: usize,
> >> +    ) -> Result<usize> {
> >> +        let slice = self.as_mut_slice();
> >> +
> >
> > Nit: This is safe, but it also requires `FromBytes`, and is &mut, &mut
>
> It already requires T: AsBytes + FromBytes above. Or do you mean something else?

I mean the safety comment. Your code is right, the comment needs to be updated.

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

* Re: [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial()
  2025-10-03 22:26 ` [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial() Danilo Krummrich
@ 2025-10-17 11:11   ` Alice Ryhl
  2025-10-17 11:50     ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-17 11:11 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sat, Oct 04, 2025 at 12:26:38AM +0200, Danilo Krummrich wrote:
> The existing read_slice() method is a wrapper around copy_from_user()
> and expects the user buffer to be larger than the destination buffer.
> 
> However, userspace may split up writes in multiple partial operations
> providing an offset into the destination buffer and a smaller user
> buffer.
> 
> In order to support this common case, provide a helper for partial
> reads.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/uaccess.rs | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index a8fb4764185a..1b0b57e855c9 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -287,6 +287,19 @@ pub fn read_slice(&mut self, out: &mut [u8]) -> Result {
>          self.read_raw(out)
>      }
>  
> +    /// Reads raw data from the user slice into a kernel buffer partially.
> +    ///
> +    /// This is the same as [`Self::read_slice`] but considers the given `offset` into `out` and
> +    /// truncates the read to the boundaries of `self` and `out`.
> +    ///
> +    /// On success, returns the number of bytes read.
> +    pub fn read_slice_partial(&mut self, out: &mut [u8], offset: usize) -> Result<usize> {
> +        let end = offset.checked_add(self.len()).ok_or(EINVAL)?.min(out.len());

Should this be?
let end = offset.checked_add(self.len()).unwrap_or(out.len()).min(out.len());

> +        out.get_mut(offset..end)
> +            .map_or(Ok(0), |dst| self.read_slice(dst).map(|()| dst.len()))

So if out.len() < offset, then we return Ok(0)?

Alice

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

* Re: [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial()
  2025-10-17 11:11   ` Alice Ryhl
@ 2025-10-17 11:50     ` Danilo Krummrich
  0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-17 11:50 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri Oct 17, 2025 at 1:11 PM CEST, Alice Ryhl wrote:
> On Sat, Oct 04, 2025 at 12:26:38AM +0200, Danilo Krummrich wrote:
>> The existing read_slice() method is a wrapper around copy_from_user()
>> and expects the user buffer to be larger than the destination buffer.
>> 
>> However, userspace may split up writes in multiple partial operations
>> providing an offset into the destination buffer and a smaller user
>> buffer.
>> 
>> In order to support this common case, provide a helper for partial
>> reads.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>  rust/kernel/uaccess.rs | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> index a8fb4764185a..1b0b57e855c9 100644
>> --- a/rust/kernel/uaccess.rs
>> +++ b/rust/kernel/uaccess.rs
>> @@ -287,6 +287,19 @@ pub fn read_slice(&mut self, out: &mut [u8]) -> Result {
>>          self.read_raw(out)
>>      }
>>  
>> +    /// Reads raw data from the user slice into a kernel buffer partially.
>> +    ///
>> +    /// This is the same as [`Self::read_slice`] but considers the given `offset` into `out` and
>> +    /// truncates the read to the boundaries of `self` and `out`.
>> +    ///
>> +    /// On success, returns the number of bytes read.
>> +    pub fn read_slice_partial(&mut self, out: &mut [u8], offset: usize) -> Result<usize> {
>> +        let end = offset.checked_add(self.len()).ok_or(EINVAL)?.min(out.len());
>
> Should this be?
> let end = offset.checked_add(self.len()).unwrap_or(out.len()).min(out.len());

Yes, that seems reasonable.

>> +        out.get_mut(offset..end)
>> +            .map_or(Ok(0), |dst| self.read_slice(dst).map(|()| dst.len()))
>
> So if out.len() < offset, then we return Ok(0)?

Yes, because it tells userspace that there are no more bytes left to read.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-03 22:26 ` [PATCH 3/7] rust: debugfs: support for binary large objects Danilo Krummrich
@ 2025-10-17 12:59   ` Alice Ryhl
  2025-10-17 14:37     ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-17 12:59 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
> Introduce support for read-only, write-only, and read-write binary files
> in Rust debugfs. This adds:
> 
> - BinaryWriter and BinaryReader traits for writing to and reading from
>   user slices in binary form.
> - New Dir methods: read_binary_file(), write_binary_file(),
>   `read_write_binary_file`.
> - Corresponding FileOps implementations: BinaryReadFile,
>   BinaryWriteFile, BinaryReadWriteFile.
> 
> This allows kernel modules to expose arbitrary binary data through
> debugfs, with proper support for offsets and partial reads/writes.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

> +extern "C" fn blob_write<T: BinaryReader>(
> +    file: *mut bindings::file,
> +    buf: *const c_char,
> +    count: usize,
> +    ppos: *mut bindings::loff_t,
> +) -> isize {
> +    // SAFETY:
> +    // - `file` is a valid pointer to a `struct file`.
> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
> +
> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
> +    let pos = unsafe { &mut *ppos };
> +
> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
> +
> +    let ret = || -> Result<isize> {
> +        let offset = (*pos).try_into()?;

So offsets larger than the buffer result in Ok(0) unless the offset
doesn't fit in an usize, in which case it's an error instead? I think we
should treat offsets that are too large in the same manner no matter
how large they are.

> +        let read = this.read_from_slice(&mut reader, offset)?;
> +        *pos += bindings::loff_t::try_from(read)?;

This addition could overflow and panic the kernel.

> +        Ok(read.try_into()?)
> +    }();
> +
> +    match ret {
> +        Ok(n) => n,
> +        Err(e) => e.to_errno() as isize,
> +    }
> +}
> +
> +pub(crate) trait BinaryWriteFile<T> {
> +    const FILE_OPS: FileOps<T>;
> +}

Hmm ... this is inconsistent with how we do vtables in other parts of
`kernel`. Normally a struct is used instead of a trait (see e.g.
miscdevice or block). But the inconsistency is already present.

Alice

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

* Re: [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir
  2025-10-03 22:26 ` [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir Danilo Krummrich
@ 2025-10-17 13:00   ` Alice Ryhl
  2025-10-17 14:55     ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-17 13:00 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sat, Oct 04, 2025 at 12:26:43AM +0200, Danilo Krummrich wrote:
> Add support for creating binary debugfs files via ScopedDir. This
> mirrors the existing functionality for Dir, but without producing an
> owning handle -- files are automatically removed when the associated
> Scope is dropped.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/debugfs.rs | 45 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
> index 3c3bbcc126ef..0eb1719e4953 100644
> --- a/rust/kernel/debugfs.rs
> +++ b/rust/kernel/debugfs.rs
> @@ -531,6 +531,20 @@ pub fn read_only_file<T: Writer + Send + Sync + 'static>(&self, name: &CStr, dat
>          self.create_file(name, data, &T::FILE_OPS)
>      }
>  
> +    /// Creates a read-only binary file in this directory.
> +    ///
> +    /// The file's contents are produced by invoking [`BinaryWriter::write_to_slice`].
> +    ///
> +    /// This function does not produce an owning handle to the file. The created file is removed
> +    /// when the [`Scope`] that this directory belongs to is dropped.
> +    pub fn read_binary_file<T: BinaryWriter + Send + Sync + 'static>(
> +        &self,
> +        name: &CStr,
> +        data: &'data T,
> +    ) {
> +        self.create_file(name, data, &T::FILE_OPS)

Why isn't <T as MyTrait> need here when it's needed for the other
methods?

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-17 12:59   ` Alice Ryhl
@ 2025-10-17 14:37     ` Danilo Krummrich
  2025-10-17 14:53       ` Danilo Krummrich
  2025-10-19  9:50       ` Alice Ryhl
  0 siblings, 2 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-17 14:37 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri Oct 17, 2025 at 2:59 PM CEST, Alice Ryhl wrote:
> On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
>> Introduce support for read-only, write-only, and read-write binary files
>> in Rust debugfs. This adds:
>> 
>> - BinaryWriter and BinaryReader traits for writing to and reading from
>>   user slices in binary form.
>> - New Dir methods: read_binary_file(), write_binary_file(),
>>   `read_write_binary_file`.
>> - Corresponding FileOps implementations: BinaryReadFile,
>>   BinaryWriteFile, BinaryReadWriteFile.
>> 
>> This allows kernel modules to expose arbitrary binary data through
>> debugfs, with proper support for offsets and partial reads/writes.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>
>> +extern "C" fn blob_write<T: BinaryReader>(
>> +    file: *mut bindings::file,
>> +    buf: *const c_char,
>> +    count: usize,
>> +    ppos: *mut bindings::loff_t,
>> +) -> isize {
>> +    // SAFETY:
>> +    // - `file` is a valid pointer to a `struct file`.
>> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
>> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
>> +
>> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
>> +    let pos = unsafe { &mut *ppos };
>> +
>> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
>> +
>> +    let ret = || -> Result<isize> {
>> +        let offset = (*pos).try_into()?;
>
> So offsets larger than the buffer result in Ok(0) unless the offset
> doesn't fit in an usize, in which case it's an error instead? I think we
> should treat offsets that are too large in the same manner no matter
> how large they are.

The offset being larger than thhe buffer is fine, userspace has to try to read
until the kernel indicates that there are no more bytes left to read by
returning zero.

But if the offset is larger than a usize there isn't a chance this can ever be
successful in the first place, hence I'd consider this an error.

>> +        let read = this.read_from_slice(&mut reader, offset)?;
>> +        *pos += bindings::loff_t::try_from(read)?;
>
> This addition could overflow and panic the kernel.

I don't see a real scenario where this could overflow when read_from_slice() was
successful, but I (also) think this should be checked_add() instead.

>> +        Ok(read.try_into()?)
>> +    }();
>> +
>> +    match ret {
>> +        Ok(n) => n,
>> +        Err(e) => e.to_errno() as isize,
>> +    }
>> +}
>> +
>> +pub(crate) trait BinaryWriteFile<T> {
>> +    const FILE_OPS: FileOps<T>;
>> +}
>
> Hmm ... this is inconsistent with how we do vtables in other parts of
> `kernel`. Normally a struct is used instead of a trait (see e.g.
> miscdevice or block). But the inconsistency is already present.

The reason I went with a trait is because that's consistent within the file.

Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
fine with that. :)

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-17 14:37     ` Danilo Krummrich
@ 2025-10-17 14:53       ` Danilo Krummrich
  2025-10-19  9:44         ` Alice Ryhl
  2025-10-19  9:50       ` Alice Ryhl
  1 sibling, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-17 14:53 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
> The reason I went with a trait is because that's consistent within the file.
>
> Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
> fine with that. :)

Actually, there's another reason I forgot about since I sent the series. :)

We need it because we derive it from blanket implementations:

	impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
	impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
	impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T

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

* Re: [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir
  2025-10-17 13:00   ` Alice Ryhl
@ 2025-10-17 14:55     ` Danilo Krummrich
  0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-17 14:55 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri Oct 17, 2025 at 3:00 PM CEST, Alice Ryhl wrote:
> On Sat, Oct 04, 2025 at 12:26:43AM +0200, Danilo Krummrich wrote:
>> Add support for creating binary debugfs files via ScopedDir. This
>> mirrors the existing functionality for Dir, but without producing an
>> owning handle -- files are automatically removed when the associated
>> Scope is dropped.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>  rust/kernel/debugfs.rs | 45 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>> 
>> diff --git a/rust/kernel/debugfs.rs b/rust/kernel/debugfs.rs
>> index 3c3bbcc126ef..0eb1719e4953 100644
>> --- a/rust/kernel/debugfs.rs
>> +++ b/rust/kernel/debugfs.rs
>> @@ -531,6 +531,20 @@ pub fn read_only_file<T: Writer + Send + Sync + 'static>(&self, name: &CStr, dat
>>          self.create_file(name, data, &T::FILE_OPS)
>>      }
>>  
>> +    /// Creates a read-only binary file in this directory.
>> +    ///
>> +    /// The file's contents are produced by invoking [`BinaryWriter::write_to_slice`].
>> +    ///
>> +    /// This function does not produce an owning handle to the file. The created file is removed
>> +    /// when the [`Scope`] that this directory belongs to is dropped.
>> +    pub fn read_binary_file<T: BinaryWriter + Send + Sync + 'static>(
>> +        &self,
>> +        name: &CStr,
>> +        data: &'data T,
>> +    ) {
>> +        self.create_file(name, data, &T::FILE_OPS)
>
> Why isn't <T as MyTrait> need here when it's needed for the other
> methods?

It's not needed for write_binary_file() I think, but read_write_binary_file()
needs it because:

	fn read_write_binary_file<T: BinaryWriter + BinaryReader + Send + Sync + 'static>()

So, just &T::FILE_OPS is ambiguous, because it implements BinaryReadFile,
BinaryWriteFile and BinaryReadWriteFile.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-17 14:53       ` Danilo Krummrich
@ 2025-10-19  9:44         ` Alice Ryhl
  2025-10-19 12:01           ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-19  9:44 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
> > The reason I went with a trait is because that's consistent within the file.
> >
> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
> > fine with that. :)
> 
> Actually, there's another reason I forgot about since I sent the series. :)
> 
> We need it because we derive it from blanket implementations:
> 
> 	impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
> 	impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
> 	impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T

You can still use a struct:

struct BinaryWriterVtable<T: BinaryWriter + Sync>;

impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
    const VTABLE: bindings::foo = ...;
}

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-17 14:37     ` Danilo Krummrich
  2025-10-17 14:53       ` Danilo Krummrich
@ 2025-10-19  9:50       ` Alice Ryhl
  2025-10-19 11:24         ` Danilo Krummrich
  1 sibling, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-19  9:50 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Fri, Oct 17, 2025 at 04:37:48PM +0200, Danilo Krummrich wrote:
> On Fri Oct 17, 2025 at 2:59 PM CEST, Alice Ryhl wrote:
> > On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
> >> Introduce support for read-only, write-only, and read-write binary files
> >> in Rust debugfs. This adds:
> >> 
> >> - BinaryWriter and BinaryReader traits for writing to and reading from
> >>   user slices in binary form.
> >> - New Dir methods: read_binary_file(), write_binary_file(),
> >>   `read_write_binary_file`.
> >> - Corresponding FileOps implementations: BinaryReadFile,
> >>   BinaryWriteFile, BinaryReadWriteFile.
> >> 
> >> This allows kernel modules to expose arbitrary binary data through
> >> debugfs, with proper support for offsets and partial reads/writes.
> >> 
> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >
> >> +extern "C" fn blob_write<T: BinaryReader>(
> >> +    file: *mut bindings::file,
> >> +    buf: *const c_char,
> >> +    count: usize,
> >> +    ppos: *mut bindings::loff_t,
> >> +) -> isize {
> >> +    // SAFETY:
> >> +    // - `file` is a valid pointer to a `struct file`.
> >> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
> >> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
> >> +
> >> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
> >> +    let pos = unsafe { &mut *ppos };
> >> +
> >> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
> >> +
> >> +    let ret = || -> Result<isize> {
> >> +        let offset = (*pos).try_into()?;
> >
> > So offsets larger than the buffer result in Ok(0) unless the offset
> > doesn't fit in an usize, in which case it's an error instead? I think we
> > should treat offsets that are too large in the same manner no matter
> > how large they are.
> 
> The offset being larger than thhe buffer is fine, userspace has to try to read
> until the kernel indicates that there are no more bytes left to read by
> returning zero.
> 
> But if the offset is larger than a usize there isn't a chance this can ever be
> successful in the first place, hence I'd consider this an error.

I don't really agree with this. Obviously we have to return Ok(0) if the
position is equal to the buffer size. But for positions strictly larger
than the buffer size I think it's reasonable to choose between Ok(0) or
an error. But please, let's be consistent about whether we return Ok(0)
or errors for positions larger than the buffer size.

Besides, it could succeed. Imagine a debugfs file whose contents aren't
stored in memory as a big byte array, but can be fetched / computed on
the fly based on any offset. Such a file could be larger than 4GB on a
32-bit machine no problem.

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-19  9:50       ` Alice Ryhl
@ 2025-10-19 11:24         ` Danilo Krummrich
  2025-10-20  8:13           ` Alice Ryhl
  0 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-19 11:24 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sun Oct 19, 2025 at 11:50 AM CEST, Alice Ryhl wrote:
> On Fri, Oct 17, 2025 at 04:37:48PM +0200, Danilo Krummrich wrote:
>> On Fri Oct 17, 2025 at 2:59 PM CEST, Alice Ryhl wrote:
>> > On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
>> >> Introduce support for read-only, write-only, and read-write binary files
>> >> in Rust debugfs. This adds:
>> >> 
>> >> - BinaryWriter and BinaryReader traits for writing to and reading from
>> >>   user slices in binary form.
>> >> - New Dir methods: read_binary_file(), write_binary_file(),
>> >>   `read_write_binary_file`.
>> >> - Corresponding FileOps implementations: BinaryReadFile,
>> >>   BinaryWriteFile, BinaryReadWriteFile.
>> >> 
>> >> This allows kernel modules to expose arbitrary binary data through
>> >> debugfs, with proper support for offsets and partial reads/writes.
>> >> 
>> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> >
>> >> +extern "C" fn blob_write<T: BinaryReader>(
>> >> +    file: *mut bindings::file,
>> >> +    buf: *const c_char,
>> >> +    count: usize,
>> >> +    ppos: *mut bindings::loff_t,
>> >> +) -> isize {
>> >> +    // SAFETY:
>> >> +    // - `file` is a valid pointer to a `struct file`.
>> >> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
>> >> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
>> >> +
>> >> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
>> >> +    let pos = unsafe { &mut *ppos };
>> >> +
>> >> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
>> >> +
>> >> +    let ret = || -> Result<isize> {
>> >> +        let offset = (*pos).try_into()?;
>> >
>> > So offsets larger than the buffer result in Ok(0) unless the offset
>> > doesn't fit in an usize, in which case it's an error instead? I think we
>> > should treat offsets that are too large in the same manner no matter
>> > how large they are.
>> 
>> The offset being larger than thhe buffer is fine, userspace has to try to read
>> until the kernel indicates that there are no more bytes left to read by
>> returning zero.
>> 
>> But if the offset is larger than a usize there isn't a chance this can ever be
>> successful in the first place, hence I'd consider this an error.
>
> I don't really agree with this. Obviously we have to return Ok(0) if the
> position is equal to the buffer size. But for positions strictly larger
> than the buffer size I think it's reasonable to choose between Ok(0) or
> an error. But please, let's be consistent about whether we return Ok(0)
> or errors for positions larger than the buffer size.

There's not really a choice, it has to be Ok(0), otherwise we break userspace.

However, you do have a point with how the offset conversion to usize should be
handled. We shouldn't try to convert it to usize in the first place, but rather
pass it through as it is and make it a common offset-larger-buffer case.

We probably also want a type alias for bindings::loff_t.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-19  9:44         ` Alice Ryhl
@ 2025-10-19 12:01           ` Danilo Krummrich
  2025-10-20  8:12             ` Alice Ryhl
  0 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-19 12:01 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sun Oct 19, 2025 at 11:44 AM CEST, Alice Ryhl wrote:
> On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
>> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
>> > The reason I went with a trait is because that's consistent within the file.
>> >
>> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
>> > fine with that. :)
>> 
>> Actually, there's another reason I forgot about since I sent the series. :)
>> 
>> We need it because we derive it from blanket implementations:
>> 
>> 	impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
>> 	impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
>> 	impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T
>
> You can still use a struct:
>
> struct BinaryWriterVtable<T: BinaryWriter + Sync>;
>
> impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
>     const VTABLE: bindings::foo = ...;
> }

Yeah, but do we get something for adding yet another type in this case?

Another point to consider is if we want a more generic fops abstraction type.

In any case, I'd like to add this as good first issue for the whole file to be
changed accordingly.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-19 12:01           ` Danilo Krummrich
@ 2025-10-20  8:12             ` Alice Ryhl
  2025-10-20  9:40               ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20  8:12 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sun, Oct 19, 2025 at 02:01:03PM +0200, Danilo Krummrich wrote:
> On Sun Oct 19, 2025 at 11:44 AM CEST, Alice Ryhl wrote:
> > On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
> >> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
> >> > The reason I went with a trait is because that's consistent within the file.
> >> >
> >> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
> >> > fine with that. :)
> >> 
> >> Actually, there's another reason I forgot about since I sent the series. :)
> >> 
> >> We need it because we derive it from blanket implementations:
> >> 
> >> 	impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
> >> 	impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
> >> 	impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T
> >
> > You can still use a struct:
> >
> > struct BinaryWriterVtable<T: BinaryWriter + Sync>;
> >
> > impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
> >     const VTABLE: bindings::foo = ...;
> > }
> 
> Yeah, but do we get something for adding yet another type in this case?
> 
> Another point to consider is if we want a more generic fops abstraction type.
> 
> In any case, I'd like to add this as good first issue for the whole file to be
> changed accordingly.

Yes, keep it as-is for consistency with the rest of the file, even if
the file is inconsistent with the rest of `kernel`. Please go ahead and
file a good-first-issue for this.

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-19 11:24         ` Danilo Krummrich
@ 2025-10-20  8:13           ` Alice Ryhl
  2025-10-20  9:35             ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20  8:13 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sun, Oct 19, 2025 at 01:24:05PM +0200, Danilo Krummrich wrote:
> On Sun Oct 19, 2025 at 11:50 AM CEST, Alice Ryhl wrote:
> > On Fri, Oct 17, 2025 at 04:37:48PM +0200, Danilo Krummrich wrote:
> >> On Fri Oct 17, 2025 at 2:59 PM CEST, Alice Ryhl wrote:
> >> > On Sat, Oct 04, 2025 at 12:26:40AM +0200, Danilo Krummrich wrote:
> >> >> Introduce support for read-only, write-only, and read-write binary files
> >> >> in Rust debugfs. This adds:
> >> >> 
> >> >> - BinaryWriter and BinaryReader traits for writing to and reading from
> >> >>   user slices in binary form.
> >> >> - New Dir methods: read_binary_file(), write_binary_file(),
> >> >>   `read_write_binary_file`.
> >> >> - Corresponding FileOps implementations: BinaryReadFile,
> >> >>   BinaryWriteFile, BinaryReadWriteFile.
> >> >> 
> >> >> This allows kernel modules to expose arbitrary binary data through
> >> >> debugfs, with proper support for offsets and partial reads/writes.
> >> >> 
> >> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >> >
> >> >> +extern "C" fn blob_write<T: BinaryReader>(
> >> >> +    file: *mut bindings::file,
> >> >> +    buf: *const c_char,
> >> >> +    count: usize,
> >> >> +    ppos: *mut bindings::loff_t,
> >> >> +) -> isize {
> >> >> +    // SAFETY:
> >> >> +    // - `file` is a valid pointer to a `struct file`.
> >> >> +    // - The type invariant of `FileOps` guarantees that `private_data` points to a valid `T`.
> >> >> +    let this = unsafe { &*((*file).private_data.cast::<T>()) };
> >> >> +
> >> >> +    // SAFETY: `ppos` is a valid `loff_t` pointer.
> >> >> +    let pos = unsafe { &mut *ppos };
> >> >> +
> >> >> +    let mut reader = UserSlice::new(UserPtr::from_ptr(buf.cast_mut().cast()), count).reader();
> >> >> +
> >> >> +    let ret = || -> Result<isize> {
> >> >> +        let offset = (*pos).try_into()?;
> >> >
> >> > So offsets larger than the buffer result in Ok(0) unless the offset
> >> > doesn't fit in an usize, in which case it's an error instead? I think we
> >> > should treat offsets that are too large in the same manner no matter
> >> > how large they are.
> >> 
> >> The offset being larger than thhe buffer is fine, userspace has to try to read
> >> until the kernel indicates that there are no more bytes left to read by
> >> returning zero.
> >> 
> >> But if the offset is larger than a usize there isn't a chance this can ever be
> >> successful in the first place, hence I'd consider this an error.
> >
> > I don't really agree with this. Obviously we have to return Ok(0) if the
> > position is equal to the buffer size. But for positions strictly larger
> > than the buffer size I think it's reasonable to choose between Ok(0) or
> > an error. But please, let's be consistent about whether we return Ok(0)
> > or errors for positions larger than the buffer size.
> 
> There's not really a choice, it has to be Ok(0), otherwise we break userspace.
> 
> However, you do have a point with how the offset conversion to usize should be
> handled. We shouldn't try to convert it to usize in the first place, but rather
> pass it through as it is and make it a common offset-larger-buffer case.

SGTM.

> We probably also want a type alias for bindings::loff_t.

I ended up using i64 for simple_read_from_buffer in iov.rs instead of
loff_t. But if they can differ, then yeah let's introduce a loff_t type
alias.

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  8:13           ` Alice Ryhl
@ 2025-10-20  9:35             ` Danilo Krummrich
  2025-10-20  9:39               ` Alice Ryhl
  0 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-20  9:35 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon Oct 20, 2025 at 10:13 AM CEST, Alice Ryhl wrote:
> I ended up using i64 for simple_read_from_buffer in iov.rs instead of
> loff_t. But if they can differ, then yeah let's introduce a loff_t type
> alias.

No, I don't think they can differ (I used i64 in earlier version that didn't
make it to the list as well), but I think it could still make sense to indicate
the relationship with loff_t. When I see an i64, an offset into a buffer is not
the first thing that comes to my mind.

What about uaccess::Offset?

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  9:35             ` Danilo Krummrich
@ 2025-10-20  9:39               ` Alice Ryhl
  2025-10-20  9:41                 ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20  9:39 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon, Oct 20, 2025 at 11:35:15AM +0200, Danilo Krummrich wrote:
> On Mon Oct 20, 2025 at 10:13 AM CEST, Alice Ryhl wrote:
> > I ended up using i64 for simple_read_from_buffer in iov.rs instead of
> > loff_t. But if they can differ, then yeah let's introduce a loff_t type
> > alias.
> 
> No, I don't think they can differ (I used i64 in earlier version that didn't
> make it to the list as well), but I think it could still make sense to indicate
> the relationship with loff_t. When I see an i64, an offset into a buffer is not
> the first thing that comes to my mind.
> 
> What about uaccess::Offset?

Hmm. That seems wrong. loff_t is a *file position*, so it should go in
kernel::fs, right? We're only using it in uaccess/iov because they
happen to have utility methods to help with implementing fops entries.
None of the "base" uaccess/iov functions use loff_t for anything since
they deal with sizes in the address space, for which usize is the
correct type.

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  8:12             ` Alice Ryhl
@ 2025-10-20  9:40               ` Danilo Krummrich
  2025-10-20  9:42                 ` Alice Ryhl
  0 siblings, 1 reply; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-20  9:40 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon Oct 20, 2025 at 10:12 AM CEST, Alice Ryhl wrote:
> On Sun, Oct 19, 2025 at 02:01:03PM +0200, Danilo Krummrich wrote:
>> On Sun Oct 19, 2025 at 11:44 AM CEST, Alice Ryhl wrote:
>> > On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
>> >> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
>> >> > The reason I went with a trait is because that's consistent within the file.
>> >> >
>> >> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
>> >> > fine with that. :)
>> >> 
>> >> Actually, there's another reason I forgot about since I sent the series. :)
>> >> 
>> >> We need it because we derive it from blanket implementations:
>> >> 
>> >> 	impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
>> >> 	impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
>> >> 	impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T
>> >
>> > You can still use a struct:
>> >
>> > struct BinaryWriterVtable<T: BinaryWriter + Sync>;
>> >
>> > impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
>> >     const VTABLE: bindings::foo = ...;
>> > }
>> 
>> Yeah, but do we get something for adding yet another type in this case?
>> 
>> Another point to consider is if we want a more generic fops abstraction type.
>> 
>> In any case, I'd like to add this as good first issue for the whole file to be
>> changed accordingly.
>
> Yes, keep it as-is for consistency with the rest of the file, even if
> the file is inconsistent with the rest of `kernel`. Please go ahead and
> file a good-first-issue for this.

Before doing so, can you please answer the question above? While I'm all for
consistency, in this specific case it seems we'd need another indirection for
that. And I'm not convinced that's an improvement.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  9:39               ` Alice Ryhl
@ 2025-10-20  9:41                 ` Danilo Krummrich
  0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-20  9:41 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon Oct 20, 2025 at 11:39 AM CEST, Alice Ryhl wrote:
> On Mon, Oct 20, 2025 at 11:35:15AM +0200, Danilo Krummrich wrote:
>> On Mon Oct 20, 2025 at 10:13 AM CEST, Alice Ryhl wrote:
>> > I ended up using i64 for simple_read_from_buffer in iov.rs instead of
>> > loff_t. But if they can differ, then yeah let's introduce a loff_t type
>> > alias.
>> 
>> No, I don't think they can differ (I used i64 in earlier version that didn't
>> make it to the list as well), but I think it could still make sense to indicate
>> the relationship with loff_t. When I see an i64, an offset into a buffer is not
>> the first thing that comes to my mind.
>> 
>> What about uaccess::Offset?
>
> Hmm. That seems wrong. loff_t is a *file position*, so it should go in
> kernel::fs, right? We're only using it in uaccess/iov because they
> happen to have utility methods to help with implementing fops entries.
> None of the "base" uaccess/iov functions use loff_t for anything since
> they deal with sizes in the address space, for which usize is the
> correct type.

Indeed, fair enough.

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  9:40               ` Danilo Krummrich
@ 2025-10-20  9:42                 ` Alice Ryhl
  2025-10-20  9:49                   ` Danilo Krummrich
  0 siblings, 1 reply; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20  9:42 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon, Oct 20, 2025 at 11:40 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon Oct 20, 2025 at 10:12 AM CEST, Alice Ryhl wrote:
> > On Sun, Oct 19, 2025 at 02:01:03PM +0200, Danilo Krummrich wrote:
> >> On Sun Oct 19, 2025 at 11:44 AM CEST, Alice Ryhl wrote:
> >> > On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
> >> >> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
> >> >> > The reason I went with a trait is because that's consistent within the file.
> >> >> >
> >> >> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
> >> >> > fine with that. :)
> >> >>
> >> >> Actually, there's another reason I forgot about since I sent the series. :)
> >> >>
> >> >> We need it because we derive it from blanket implementations:
> >> >>
> >> >>   impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
> >> >>   impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
> >> >>   impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T
> >> >
> >> > You can still use a struct:
> >> >
> >> > struct BinaryWriterVtable<T: BinaryWriter + Sync>;
> >> >
> >> > impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
> >> >     const VTABLE: bindings::foo = ...;
> >> > }
> >>
> >> Yeah, but do we get something for adding yet another type in this case?
> >>
> >> Another point to consider is if we want a more generic fops abstraction type.
> >>
> >> In any case, I'd like to add this as good first issue for the whole file to be
> >> changed accordingly.
> >
> > Yes, keep it as-is for consistency with the rest of the file, even if
> > the file is inconsistent with the rest of `kernel`. Please go ahead and
> > file a good-first-issue for this.
>
> Before doing so, can you please answer the question above? While I'm all for
> consistency, in this specific case it seems we'd need another indirection for
> that. And I'm not convinced that's an improvement.

The choice is between adding a new type or a new trait. There's no
intrinsic advantage to choosing either one, but the rest of `kernel`
chose "new type" over "new trait", so it makes sense to be consistent.

Alice

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

* Re: [PATCH 3/7] rust: debugfs: support for binary large objects
  2025-10-20  9:42                 ` Alice Ryhl
@ 2025-10-20  9:49                   ` Danilo Krummrich
  0 siblings, 0 replies; 32+ messages in thread
From: Danilo Krummrich @ 2025-10-20  9:49 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Mon Oct 20, 2025 at 11:42 AM CEST, Alice Ryhl wrote:
> On Mon, Oct 20, 2025 at 11:40 AM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Mon Oct 20, 2025 at 10:12 AM CEST, Alice Ryhl wrote:
>> > On Sun, Oct 19, 2025 at 02:01:03PM +0200, Danilo Krummrich wrote:
>> >> On Sun Oct 19, 2025 at 11:44 AM CEST, Alice Ryhl wrote:
>> >> > On Fri, Oct 17, 2025 at 04:53:09PM +0200, Danilo Krummrich wrote:
>> >> >> On Fri Oct 17, 2025 at 4:37 PM CEST, Danilo Krummrich wrote:
>> >> >> > The reason I went with a trait is because that's consistent within the file.
>> >> >> >
>> >> >> > Otherwise, I don't mind one or the other. If we always want to use a struct, I'm
>> >> >> > fine with that. :)
>> >> >>
>> >> >> Actually, there's another reason I forgot about since I sent the series. :)
>> >> >>
>> >> >> We need it because we derive it from blanket implementations:
>> >> >>
>> >> >>   impl<T: BinaryWriter + Sync> BinaryReadFile<T> for T
>> >> >>   impl<T: BinaryReader + Sync> BinaryWriteFile<T> for T
>> >> >>   impl<T: BinaryWriter + BinaryReader + Sync> BinaryReadWriteFile<T> for T
>> >> >
>> >> > You can still use a struct:
>> >> >
>> >> > struct BinaryWriterVtable<T: BinaryWriter + Sync>;
>> >> >
>> >> > impl<T: BinaryWriter + Sync> BinaryWriterVtable<T> {
>> >> >     const VTABLE: bindings::foo = ...;
>> >> > }
>> >>
>> >> Yeah, but do we get something for adding yet another type in this case?
>> >>
>> >> Another point to consider is if we want a more generic fops abstraction type.
>> >>
>> >> In any case, I'd like to add this as good first issue for the whole file to be
>> >> changed accordingly.
>> >
>> > Yes, keep it as-is for consistency with the rest of the file, even if
>> > the file is inconsistent with the rest of `kernel`. Please go ahead and
>> > file a good-first-issue for this.
>>
>> Before doing so, can you please answer the question above? While I'm all for
>> consistency, in this specific case it seems we'd need another indirection for
>> that. And I'm not convinced that's an improvement.
>
> The choice is between adding a new type or a new trait. There's no
> intrinsic advantage to choosing either one, but the rest of `kernel`
> chose "new type" over "new trait", so it makes sense to be consistent.

My hesitation came from the assumption that we'd need another type (additional
to the existing trait). But we can indeed replace it, so that's fine.

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

* Re: [PATCH 5/7] samples: rust: debugfs: add example for blobs
  2025-10-03 22:26 ` [PATCH 5/7] samples: rust: debugfs: add example for blobs Danilo Krummrich
@ 2025-10-20 10:01   ` Alice Ryhl
  0 siblings, 0 replies; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20 10:01 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sat, Oct 04, 2025 at 12:26:42AM +0200, Danilo Krummrich wrote:
> Extend the Rust debugfs sample to demonstrate usage of binary file
> support. The example now shows how to expose both fixed-size arrays
> and dynamically sized vectors as binary blobs in debugfs.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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

* Re: [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs
  2025-10-03 22:26 ` [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs Danilo Krummrich
@ 2025-10-20 10:02   ` Alice Ryhl
  0 siblings, 0 replies; 32+ messages in thread
From: Alice Ryhl @ 2025-10-20 10:02 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, mmaurer, rust-for-linux,
	linux-kernel

On Sat, Oct 04, 2025 at 12:26:44AM +0200, Danilo Krummrich wrote:
> Extend the rust_debugfs_scoped sample to demonstrate how to export a
> large binary object through a ScopedDir.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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

end of thread, other threads:[~2025-10-20 10:02 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 22:26 [PATCH 0/7] Binary Large Objects for Rust DebugFS Danilo Krummrich
2025-10-03 22:26 ` [PATCH 1/7] rust: uaccess: add UserSliceReader::read_slice_partial() Danilo Krummrich
2025-10-17 11:11   ` Alice Ryhl
2025-10-17 11:50     ` Danilo Krummrich
2025-10-03 22:26 ` [PATCH 2/7] rust: uaccess: add UserSliceWriter::write_slice_partial() Danilo Krummrich
2025-10-03 22:26 ` [PATCH 3/7] rust: debugfs: support for binary large objects Danilo Krummrich
2025-10-17 12:59   ` Alice Ryhl
2025-10-17 14:37     ` Danilo Krummrich
2025-10-17 14:53       ` Danilo Krummrich
2025-10-19  9:44         ` Alice Ryhl
2025-10-19 12:01           ` Danilo Krummrich
2025-10-20  8:12             ` Alice Ryhl
2025-10-20  9:40               ` Danilo Krummrich
2025-10-20  9:42                 ` Alice Ryhl
2025-10-20  9:49                   ` Danilo Krummrich
2025-10-19  9:50       ` Alice Ryhl
2025-10-19 11:24         ` Danilo Krummrich
2025-10-20  8:13           ` Alice Ryhl
2025-10-20  9:35             ` Danilo Krummrich
2025-10-20  9:39               ` Alice Ryhl
2025-10-20  9:41                 ` Danilo Krummrich
2025-10-03 22:26 ` [PATCH 4/7] rust: debugfs: support blobs from smart pointers Danilo Krummrich
2025-10-03 23:12   ` Matthew Maurer
2025-10-03 23:26     ` Danilo Krummrich
2025-10-03 23:36       ` Matthew Maurer
2025-10-03 22:26 ` [PATCH 5/7] samples: rust: debugfs: add example for blobs Danilo Krummrich
2025-10-20 10:01   ` Alice Ryhl
2025-10-03 22:26 ` [PATCH 6/7] rust: debugfs: support binary large objects for ScopedDir Danilo Krummrich
2025-10-17 13:00   ` Alice Ryhl
2025-10-17 14:55     ` Danilo Krummrich
2025-10-03 22:26 ` [PATCH 7/7] samples: rust: debugfs_scoped: add example for blobs Danilo Krummrich
2025-10-20 10:02   ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).