Re: [PATCH 1/3] lib: Add ref_proxy module

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Thu, Aug 14, 2025 at 12:55:55PM +0200, Danilo Krummrich wrote:
> On Thu Aug 14, 2025 at 11:10 AM CEST, Tzung-Bi Shih wrote:
> As mentioned in a sub-thread [1], this is pretty much what we already do in Rust
> with Revocable [2].
> 
> The Rust struct Devres [3] is built on Revocable, such that a device resource is
> only accessible for drivers as long as the device is actually bound to the
> driver. Once the device is unbound the resource is "revoked" and drivers are
> prevented from subsequent accesses.
> 
> I think it's be good to have a common naming scheme for this, hence I propose to
> name this struct revocable instead.

Sure, will address all review comments and fix in the next version.

> Besides that, I'm not exactly sure I understand why we need two structs for this.
> struct ref_proxy seems unnecessary. I think we should remove it and rename
> struct ref_proxy_provider to struct revocable. Or do I miss anything?

srcu_read_lock() returns an index [4].  The struct ref_proxy is necessary
for tracking the index for leaving the critical section.

[4] https://elixir.bootlin.com/linux/v6.16/source/kernel/rcu/srcutree.c#L750

> > + *
> > + * This holds an initial refcount to the struct.
> > + *
> > + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> > + */
> > +struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref)
> 
> The ref_proxy_provider owns the resource now, so when the ref_proxy_provider
> gets revoked (through the devres callback) the resource must be released, right?
> Where is this done? Who is responsible to do so? Shouldn't this function take a
> release callback that is called once the ref_proxy_provider is revoked?

Thank you, that's a valuable suggestion.  While both approaches are valid,
the current implementation is based on a clear separation of ownership.

The design is that struct ref_proxy_provider doesn't own the resource.
Instead, the resource provider (e.g., cros_ec_spi) is responsible for the
full lifecycle:
* It owns and ultimately releases the resource (e.g., [5]).
* It calls ref_proxy_provider_alloc() to expose the resource.
* It calls ref_proxy_provider_free() to revoke access to the resource.

By doing so, the resource provider doesn't need to create a bunch of
void (*release)(void *) callbacks (if multiple resources are exposing).

[5] https://elixir.bootlin.com/linux/v6.16/source/drivers/platform/chrome/cros_ec_spi.c#L752