This patch series implements a direct accessor for the data stored within
a Devres container for cases where we can proof 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 proof 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

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           | 35 +++++++++++++++++++++++++++++++++
 rust/kernel/revocable.rs        | 12 +++++++++++
 samples/rust/rust_driver_pci.rs | 12 +++++------
 3 files changed, 53 insertions(+), 6 deletions(-)


base-commit: b5cb47f81a2857d270cabbbb3a9feec0e483caed
-- 
2.49.0

On Sat, Apr 26, 2025 at 03:30:38PM +0200, Danilo Krummrich wrote:
> This patch series implements a direct accessor for the data stored within
> a Devres container for cases where we can proof 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 proof 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.
> 

Nice! However, IIUC, if the users use Devres::new() to create a `Devres`
, they will have a `Devres` they can revoke anytime, which means you can
still revoke the `Devres` even if the device is bound.

Also if a `Devres` belongs to device A, but someone passes device B's
bound reference to `access_with()`, the compiler won't check for that,
and the `Devres` can be being revoked as the same, no? If so the
function is not safe.

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
> 
> 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           | 35 +++++++++++++++++++++++++++++++++
>  rust/kernel/revocable.rs        | 12 +++++++++++
>  samples/rust/rust_driver_pci.rs | 12 +++++------
>  3 files changed, 53 insertions(+), 6 deletions(-)
> 
> 
> base-commit: b5cb47f81a2857d270cabbbb3a9feec0e483caed
> -- 
> 2.49.0
>

On Sat, Apr 26, 2025 at 10:09:39AM -0700, Boqun Feng wrote:
> On Sat, Apr 26, 2025 at 03:30:38PM +0200, Danilo Krummrich wrote:
> > This patch series implements a direct accessor for the data stored within
> > a Devres container for cases where we can proof 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 proof 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.
> > 
> 
> Nice! However, IIUC, if the users use Devres::new() to create a `Devres`
> , they will have a `Devres` they can revoke anytime, which means you can
> still revoke the `Devres` even if the device is bound.

No, a user of Devres can't revoke the inner Revocable itself. A user can only
drop the Devres instance, in which case the user also wouldn't be able to call
access_with() anymore.

> Also if a `Devres` belongs to device A, but someone passes device B's
> bound reference to `access_with()`, the compiler won't check for that,
> and the `Devres` can be being revoked as the same, no? If so the
> function is not safe.

Devres::access_with() compares the Device<Bound> parameter with its inner
ARef<Device>, and just fails if they don't match.

On Sat, Apr 26, 2025 at 07:14:54PM +0200, Danilo Krummrich wrote:
> On Sat, Apr 26, 2025 at 10:09:39AM -0700, Boqun Feng wrote:
> > On Sat, Apr 26, 2025 at 03:30:38PM +0200, Danilo Krummrich wrote:
> > > This patch series implements a direct accessor for the data stored within
> > > a Devres container for cases where we can proof 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 proof 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.
> > > 
> > 
> > Nice! However, IIUC, if the users use Devres::new() to create a `Devres`
> > , they will have a `Devres` they can revoke anytime, which means you can
> > still revoke the `Devres` even if the device is bound.
> 
> No, a user of Devres can't revoke the inner Revocable itself. A user can only
> drop the Devres instance, in which case the user also wouldn't be able to call
> access_with() anymore.
> 

Oh, right, because it's a `Devres` not `Revocable` in general.

> > Also if a `Devres` belongs to device A, but someone passes device B's
> > bound reference to `access_with()`, the compiler won't check for that,
> > and the `Devres` can be being revoked as the same, no? If so the
> > function is not safe.
> 
> Devres::access_with() compares the Device<Bound> parameter with its inner
> ARef<Device>, and just fails if they don't match.

I see, I missed that. Thanks!

Regards,
Boqun