[PATCH v2 0/3] Devres optimization with bound devices

[PATCH v2 0/3] Devres optimization with bound devices

[Danilo Krummrich]

This patch series implements a direct accessor for the data stored within
a Devres container for cases where we can prove that we own a reference
to a Device<Bound> (i.e. a bound device) of the same device that was used
to create the corresponding Devres container.

Usually, when accessing the data stored within a Devres container, it is
not clear whether the data has been revoked already due to the device
being unbound and, hence, we have to try whether the access is possible
and subsequently keep holding the RCU read lock for the duration of the
access.

However, when we can prove that we hold a reference to Device<Bound>
matching the device the Devres container has been created with, we can
guarantee that the device is not unbound for the duration of the
lifetime of the Device<Bound> reference and, hence, it is not possible
for the data within the Devres container to be revoked.

Therefore, in this case, we can bypass the atomic check and the RCU read
lock, which is a great optimization and simplification for drivers.

The patches of this series are also available in [1].

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

Changes in v2:
  - Revocable::access(): remvoe explicit lifetimes; don't refer to 'a in
    the safety requirement
  - Devres::access()
    - rename Devres::access_with() to Devres::access()
    - add missing '```' at the end of the example
    - remove 's lifetime
    - add # Errors section

Danilo Krummrich (3):
  rust: revocable: implement Revocable::access()
  rust: devres: implement Devres::access_with()
  samples: rust: pci: take advantage of Devres::access_with()

 rust/kernel/devres.rs           | 38 +++++++++++++++++++++++++++++++++
 rust/kernel/revocable.rs        | 12 +++++++++++
 samples/rust/rust_driver_pci.rs | 12 +++++------
 3 files changed, 56 insertions(+), 6 deletions(-)


base-commit: 3be746ebc1e6e32f499a65afe405df9030153a63
-- 
2.49.0

[PATCH v2 1/3] rust: revocable: implement Revocable::access()

[Danilo Krummrich]

Implement an unsafe direct accessor for the data stored within the
Revocable.

This is useful for cases where we can prove that the data stored within
the Revocable is not and cannot be revoked for the duration of the
lifetime of the returned reference.

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/revocable.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 971d0dc38d83..db4aa46bb121 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -139,6 +139,18 @@ pub fn try_access_with<R, F: FnOnce(&T) -> R>(&self, f: F) -> Option<R> {
         self.try_access().map(|t| f(&*t))
     }
 
+    /// Directly access the revocable wrapped object.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure this [`Revocable`] instance hasn't been revoked and won't be revoked
+    /// as long as the returned `&T` lives.
+    pub unsafe fn access(&self) -> &T {
+        // SAFETY: By the safety requirement of this function it is guaranteed that
+        // `self.data.get()` is a valid pointer to an instance of `T`.
+        unsafe { &*self.data.get() }
+    }
+
     /// # Safety
     ///
     /// Callers must ensure that there are no more concurrent users of the revocable object.
-- 
2.49.0

[PATCH v2 2/3] rust: devres: implement Devres::access_with()

[Danilo Krummrich]

Implement a direct accessor for the data stored within the Devres for
cases where we can prove that we own a reference to a Device<Bound>
(i.e. a bound device) of the same device that was used to create the
corresponding Devres container.

Usually, when accessing the data stored within a Devres container, it is
not clear whether the data has been revoked already due to the device
being unbound and, hence, we have to try whether the access is possible
and subsequently keep holding the RCU read lock for the duration of the
access.

However, when we can prove that we hold a reference to Device<Bound>
matching the device the Devres container has been created with, we can
guarantee that the device is not unbound for the duration of the
lifetime of the Device<Bound> reference and, hence, it is not possible
for the data within the Devres container to be revoked.

Therefore, in this case, we can bypass the atomic check and the RCU read
lock, which is a great optimization and simplification for drivers.

Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/devres.rs | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 1e58f5d22044..cf6fc09b2c05 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -181,6 +181,44 @@ pub fn new_foreign_owned(dev: &Device<Bound>, data: T, flags: Flags) -> Result {
 
         Ok(())
     }
+
+    /// Obtain `&'a T`, bypassing the [`Revocable`].
+    ///
+    /// This method allows to directly obtain a `&'a T`, bypassing the [`Revocable`], by presenting
+    /// a `&'a Device<Bound>` of the same [`Device`] this [`Devres`] instance has been created with.
+    ///
+    /// # Errors
+    ///
+    /// An error is returned if `dev` does not match the same [`Device`] this [`Devres`] instance
+    /// has been created with.
+    ///
+    /// # Example
+    ///
+    /// ```no_run
+    /// # use kernel::{device::Core, devres::Devres, pci};
+    ///
+    /// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<0x4>>) -> Result<()> {
+    ///     let bar = devres.access(dev.as_ref())?;
+    ///
+    ///     let _ = bar.read32(0x0);
+    ///
+    ///     // might_sleep()
+    ///
+    ///     bar.write32(0x42, 0x0);
+    ///
+    ///     Ok(())
+    /// }
+    /// ```
+    pub fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Result<&'a T> {
+        if self.0.dev.as_raw() != dev.as_raw() {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: `dev` being the same device as the device this `Devres` has been created for
+        // proves that `self.0.data` hasn't been revoked and is guaranteed to not be revoked as
+        // long as `dev` lives; `dev` lives at least as long as `self`.
+        Ok(unsafe { self.deref().access() })
+    }
 }
 
 impl<T> Deref for Devres<T> {
-- 
2.49.0

[PATCH v2 3/3] samples: rust: pci: take advantage of Devres::access_with()

[Danilo Krummrich]

For the I/O operations executed from the probe() method, take advantage
of Devres::access_with(), avoiding the atomic check and RCU read lock
required otherwise entirely.

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

diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 9ce3a7323a16..15147e4401b2 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -83,12 +83,12 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
             GFP_KERNEL,
         )?;
 
-        let res = drvdata
-            .bar
-            .try_access_with(|b| Self::testdev(info, b))
-            .ok_or(ENXIO)??;
-
-        dev_info!(pdev.as_ref(), "pci-testdev data-match count: {}\n", res);
+        let bar = drvdata.bar.access(pdev.as_ref())?;
+        dev_info!(
+            pdev.as_ref(),
+            "pci-testdev data-match count: {}\n",
+            Self::testdev(info, bar)?
+        );
 
         Ok(drvdata.into())
     }
-- 
2.49.0

Re: [PATCH v2 0/3] Devres optimization with bound devices

[Boqun Feng]

On Mon, Apr 28, 2025 at 04:00:26PM +0200, Danilo Krummrich wrote:
> This patch series implements a direct accessor for the data stored within
> a Devres container for cases where we can prove that we own a reference
> to a Device<Bound> (i.e. a bound device) of the same device that was used
> to create the corresponding Devres container.
> 
> Usually, when accessing the data stored within a Devres container, it is
> not clear whether the data has been revoked already due to the device
> being unbound and, hence, we have to try whether the access is possible
> and subsequently keep holding the RCU read lock for the duration of the
> access.
> 
> However, when we can prove that we hold a reference to Device<Bound>
> matching the device the Devres container has been created with, we can
> guarantee that the device is not unbound for the duration of the
> lifetime of the Device<Bound> reference and, hence, it is not possible
> for the data within the Devres container to be revoked.
> 
> Therefore, in this case, we can bypass the atomic check and the RCU read
> lock, which is a great optimization and simplification for drivers.
> 

Acked-by: Boqun Feng <boqun.feng@gmail.com>

You would need to, however, change the titles for patch #2 and #3
because there is no `Devres::access_with()` any more.

Regards,
Boqun

> The patches of this series are also available in [1].
> 
> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/devres
> 
> Changes in v2:
>   - Revocable::access(): remvoe explicit lifetimes; don't refer to 'a in
>     the safety requirement
>   - Devres::access()
>     - rename Devres::access_with() to Devres::access()
>     - add missing '```' at the end of the example
>     - remove 's lifetime
>     - add # Errors section
> 
> Danilo Krummrich (3):
>   rust: revocable: implement Revocable::access()
>   rust: devres: implement Devres::access_with()
>   samples: rust: pci: take advantage of Devres::access_with()
> 
>  rust/kernel/devres.rs           | 38 +++++++++++++++++++++++++++++++++
>  rust/kernel/revocable.rs        | 12 +++++++++++
>  samples/rust/rust_driver_pci.rs | 12 +++++------
>  3 files changed, 56 insertions(+), 6 deletions(-)
> 
> 
> base-commit: 3be746ebc1e6e32f499a65afe405df9030153a63
> -- 
> 2.49.0
> 

Re: [PATCH v2 0/3] Devres optimization with bound devices

[Danilo Krummrich]

On Mon, Apr 28, 2025 at 10:39:17AM -0700, Boqun Feng wrote:
> On Mon, Apr 28, 2025 at 04:00:26PM +0200, Danilo Krummrich wrote:
> > This patch series implements a direct accessor for the data stored within
> > a Devres container for cases where we can prove that we own a reference
> > to a Device<Bound> (i.e. a bound device) of the same device that was used
> > to create the corresponding Devres container.
> > 
> > Usually, when accessing the data stored within a Devres container, it is
> > not clear whether the data has been revoked already due to the device
> > being unbound and, hence, we have to try whether the access is possible
> > and subsequently keep holding the RCU read lock for the duration of the
> > access.
> > 
> > However, when we can prove that we hold a reference to Device<Bound>
> > matching the device the Devres container has been created with, we can
> > guarantee that the device is not unbound for the duration of the
> > lifetime of the Device<Bound> reference and, hence, it is not possible
> > for the data within the Devres container to be revoked.
> > 
> > Therefore, in this case, we can bypass the atomic check and the RCU read
> > lock, which is a great optimization and simplification for drivers.
> > 
> 
> Acked-by: Boqun Feng <boqun.feng@gmail.com>
> 
> You would need to, however, change the titles for patch #2 and #3
> because there is no `Devres::access_with()` any more.

Thanks, good catch! Unless we need a v3 for something else, I'll fix it when I
apply the series.

Re: [PATCH v2 0/3] Devres optimization with bound devices

[Alexandre Courbot]

On Mon Apr 28, 2025 at 11:00 PM JST, Danilo Krummrich wrote:
> This patch series implements a direct accessor for the data stored within
> a Devres container for cases where we can prove that we own a reference
> to a Device<Bound> (i.e. a bound device) of the same device that was used
> to create the corresponding Devres container.
>
> Usually, when accessing the data stored within a Devres container, it is
> not clear whether the data has been revoked already due to the device
> being unbound and, hence, we have to try whether the access is possible
> and subsequently keep holding the RCU read lock for the duration of the
> access.
>
> However, when we can prove that we hold a reference to Device<Bound>
> matching the device the Devres container has been created with, we can
> guarantee that the device is not unbound for the duration of the
> lifetime of the Device<Bound> reference and, hence, it is not possible
> for the data within the Devres container to be revoked.
>
> Therefore, in this case, we can bypass the atomic check and the RCU read
> lock, which is a great optimization and simplification for drivers.

Thanks, this removes one of my pain points with the way revocable
resources were accessed and will allow to write drivers in a much more
natural way.

FWIW, the series

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH v2 0/3] Devres optimization with bound devices

[Joel Fernandes]

> On Apr 29, 2025, at 10:46 PM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> 
> On Mon Apr 28, 2025 at 11:00 PM JST, Danilo Krummrich wrote:
>> This patch series implements a direct accessor for the data stored within
>> a Devres container for cases where we can prove that we own a reference
>> to a Device<Bound> (i.e. a bound device) of the same device that was used
>> to create the corresponding Devres container.
>> 
>> Usually, when accessing the data stored within a Devres container, it is
>> not clear whether the data has been revoked already due to the device
>> being unbound and, hence, we have to try whether the access is possible
>> and subsequently keep holding the RCU read lock for the duration of the
>> access.
>> 
>> However, when we can prove that we hold a reference to Device<Bound>
>> matching the device the Devres container has been created with, we can
>> guarantee that the device is not unbound for the duration of the
>> lifetime of the Device<Bound> reference and, hence, it is not possible
>> for the data within the Devres container to be revoked.
>> 
>> Therefore, in this case, we can bypass the atomic check and the RCU read
>> lock, which is a great optimization and simplification for drivers.
> 
> Thanks, this removes one of my pain points with the way revocable
> resources were accessed and will allow to write drivers in a much more
> natural way.
> 
> FWIW, the series
> 
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Nice, I like it. Quite a readability improvement too
on the caller side!

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

Thanks.

Re: [PATCH v2 1/3] rust: revocable: implement Revocable::access()

[Miguel Ojeda]

On Mon, 28 Apr 2025 16:00:27 +0200 Danilo Krummrich <dakr@kernel.org> wrote:
>
> Implement an unsafe direct accessor for the data stored within the
> Revocable.
>
> This is useful for cases where we can prove that the data stored within
> the Revocable is not and cannot be revoked for the duration of the
> lifetime of the returned reference.
>
> Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH v2 0/3] Devres optimization with bound devices

[Danilo Krummrich]

On Mon, Apr 28, 2025 at 04:00:26PM +0200, Danilo Krummrich wrote:
> This patch series implements a direct accessor for the data stored within
> a Devres container for cases where we can prove that we own a reference
> to a Device<Bound> (i.e. a bound device) of the same device that was used
> to create the corresponding Devres container.

Applied to nova-next, thanks!

Re: [PATCH v2 2/3] rust: devres: implement Devres::access_with()

[Miguel Ojeda]

On Mon, Apr 28, 2025 at 4:01 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> +    /// # use kernel::{device::Core, devres::Devres, pci};
> +    ///
> +    /// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<0x4>>) -> Result<()> {

We need to skip this one when `!PCI` -- quick patch at:
https://lore.kernel.org/rust-for-linux/20250511182533.1016163-1-ojeda@kernel.org/

Cheers,
Miguel