Re: [PATCH v2 03/14] rust: sync: add `Arc::as_ptr`

From: Benno Lossin
Date: Tue Oct 01 2024 - 04:41:03 EST


On 01.10.24 06:56, Dirk Behme wrote:
> On 19.09.2024 16:03, Benno Lossin wrote:
>> On 18.09.24 00:27, Andreas Hindborg wrote:
>>> Add a method to get a pointer to the data contained in an `Arc`.
>>>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
>>> ---
>>> rust/kernel/sync/arc.rs | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>>> index 3673496c2363..a57ea3e2b44c 100644
>>> --- a/rust/kernel/sync/arc.rs
>>> +++ b/rust/kernel/sync/arc.rs
>>> @@ -258,6 +258,14 @@ pub fn into_raw(self) -> *const T {
>>> unsafe { core::ptr::addr_of!((*ptr).data) }
>>> }
>>>
>>> + /// Return a raw pointer to the data in this arc.
>>> + pub fn as_ptr(&self) -> *const T {
>>
>> I don't know if we have a convention for this, but shouldn't this be an
>> associated function? Because if `T` also has an `as_ptr` function, it
>> will be shadowed by this one.
>
> Yes. In Fabien's out of tree regmap we have an as_ptr() for Regmap [1]
> which operates on &Arc<Regmap> [2]. Once this patch is applied to arc.rs
> the compilation fails as then Arc.as_ptr() is used, not the
> Regmap.as_ptr() any more [3]. Switching this to something like [4] makes
> the compiler happy.

Yeah then we should switch to that.

> Thanks,
>
> Dirk
>
> P.S.: Just to learn something: For the unmodified, failing case: Is
> there a rule when which as_ptr() will be used? Is there an order rule
> for the shadowing? Any documentation link?

Yes, there is a page about the method call expression in the reference:
https://doc.rust-lang.org/reference/expressions/method-call-expr.html

---
Cheers,
Benno

> [1]
> https://github.com/Fabo/linux/blob/fparent/rust-ncv6336/rust/kernel/regmap.rs#L71
>
> [2]
> https://github.com/Fabo/linux/blob/fparent/rust-ncv6336/rust/kernel/regulator/driver.rs#L418
>
> [3]
>
> error[E0308]: mismatched types
> --> rust/kernel/regulator/driver.rs:420:33
> |
> 420 | config.cfg.regmap = regmap.as_ptr();
> | ^^^^^^^^^^^^^^^ types differ in
> mutability
> |
> = note: expected raw pointer `*mut bindings::regmap`
> found raw pointer `*const Regmap`
>
> error: aborting due to 1 previous error
>
> [4]
>
> diff --git a/rust/kernel/hrtimer/arc.rs b/rust/kernel/hrtimer/arc.rs
> index ff04b0b75bb39..7c39ab440e1c6 100644
> --- a/rust/kernel/hrtimer/arc.rs
> +++ b/rust/kernel/hrtimer/arc.rs
> @@ -25,7 +25,7 @@ unsafe impl<U> TimerHandle for ArcTimerHandle<U>
> U: HasTimer<U>,
> {
> fn cancel(&mut self) -> bool {
> - let self_ptr = self.inner.as_ptr();
> + let self_ptr = Arc::as_ptr(&self.inner);
>
> // SAFETY: As we obtained `self_ptr` from a valid reference
> above, it
> // must point to a valid `U`.
> @@ -57,7 +57,7 @@ impl<U> TimerPointer for Arc<U>
> fn schedule(self, expires: Ktime) -> ArcTimerHandle<U> {
> // SAFETY: Since we generate the pointer passed to `schedule`
> from a
> // valid reference, it is a valid pointer.
> - unsafe { U::schedule(self.as_ptr(), expires) };
> + unsafe { U::schedule(Arc::as_ptr(&self), expires) };
>
> ArcTimerHandle { inner: self }
> }
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 1466d9cd41652..0a314c2f4c5ea 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -259,8 +259,8 @@ pub fn into_raw(self) -> *const T {
> }
>
> /// Return a raw pointer to the data in this arc.
> - pub fn as_ptr(&self) -> *const T {
> - let ptr = self.ptr.as_ptr();
> + pub fn as_ptr(arc: &Self) -> *const T {
> + let ptr = arc.ptr.as_ptr();
> // SAFETY: As we derive the pointer from a reference above,
> the pointer
> // must be valid.
> unsafe { core::ptr::addr_of!((*ptr).data) }
>
>