Re: [PATCH v11 4/4] rust: Add `OwnableRefCounted`

["Benno Lossin" <lossin@kernel.org>]

On Mon Jul 7, 2025 at 3:21 PM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>
>> On Mon Jul 7, 2025 at 1:12 PM CEST, Andreas Hindborg wrote:
>>> "Benno Lossin" <lossin@kernel.org> writes:
>>>> Ah right, I forgot about this. What was the refcount characteristics of
>>>> this again?
>>>>
>>>> *  1 = in flight, owned by C
>>>> *  2 = in flight, owned by Rust
>>>> * >2 = in flight, owned by Rust + additional references used by Rust
>>>>        code
>>>>
>>>> Correct? Maybe @Andreas can check.
>>>
>>> We have been a bit back and forth on this. This is how we would like it
>>> going forward:
>>>
>>>
>>> /// There are three states for a request that the Rust bindings care about:
>>> ///
>>> /// - 0: The request is owned by C block layer or is uniquely referenced (by [`Owned<_>`]).
>>> /// - 1: The request is owned by Rust abstractions but is not referenced.
>>> /// - 2+: There is one or more [`ARef`] instances referencing the request.
>>
>> Huh, now I'm more confused... Could you go into the details again?
>
> Well, there is not much to it. We found out we can alias "unique" and
> "owned by C".
>
> We initialize the refcount to 0 when we initialize the request
> structure. This happens at queue creation time.

And IIRC this refcount is only on the Rust side, since you store it in
some private data, right?

> When C block layer hands over a request for processing to a Rust driver,
> we `debug_assert!` that the refcount is 0. We unsafely invent an
> `Owned<Request<_>>` and pass that to the driver.

And you don't increment the refcount?

> The driver has the option of `into_shared` to obtain an
> `ARef<Request<_>>`. We use this for remote completion and timer
> completion in rnull.

This operation will set the refcount to 2?

> In most drivers, when the driver hands off the request to the hardware,
> the driver will stop accounting for the request. The `Owned<Request<_>>`
> is consumed when issuing to the driver, or the driver could simply drop
> it. Refcount goes to 1. An ID is passed along to hardware.

And with the current API design it must let go of all `ARef<Request<_>>`
because otherwise it can't call `end_ok` (or some other method?).

> When a completion comes back from hardware, it carries the ID. We use
> the C block layer `tag_to_rq` machinery to turn this ID back into an
> `Owned<Request<_>>`. In that process, we check that `refcount == 1` and
> if so, we set refcount to 0 and invent an `Owned<Request<_>>`.

(because if not, this would be wrong)

I have some idea what we should do for the safety requirements of
`Ownable`, but still need some time to write it down.

---
Cheers,
Benno

> There was simply no need to have two separate values for "owned by C"
> and "owned by Rust, not referenced".
>
> Best regards,
> Andreas Hindborg