On Fri Sep 12, 2025 at 10:17 AM CEST, Tzung-Bi Shih wrote:
+/**
- struct revocable_provider - A handle for resource provider.
- @srcu: The SRCU to protect the resource.
- @res: The pointer of resource. It can point to anything.
- @kref: The refcount for this handle.
- */
+struct revocable_provider {
- struct srcu_struct srcu;
- void __rcu *res;
- struct kref kref;
+};
I think a revocable provider should provide an optional revoke() callback where users of the revocable provider can release the revoked resource.
But this can also be done as a follow-up.
+/**
- struct revocable - A handle for resource consumer.
- @rp: The pointer of resource provider.
- @idx: The index for the RCU critical section.
- */
+struct revocable {
- struct revocable_provider *rp;
- int idx;
+};
I think I asked about this in the previous version (but I don't remember if there was an answer):
In Rust we get away with a single Revocable<T> structure, but we're using RCU instead of SRCU. It seems to me that the split between struct revocable and struct revocable_provider is only for the SRCU index? Or is there any other reason?
+/**
- revocable_provider_free() - Free struct revocable_provider.
- @rp: The pointer of resource provider.
- This sets the resource `(struct revocable_provider *)->res` to NULL to
- indicate the resource has gone.
- This drops the refcount to the resource provider. If it is the final
- reference, revocable_provider_release() will be called to free the struct.
- */
+void revocable_provider_free(struct revocable_provider *rp) +{
- rcu_assign_pointer(rp->res, NULL);
- synchronize_srcu(&rp->srcu);
- kref_put(&rp->kref, revocable_provider_release);
+} +EXPORT_SYMBOL_GPL(revocable_provider_free);
I think naming this "free" is a bit misleading, since what it basically does is
(1) Revoke access to the resource.
(2) Drop a reference count of struct revocable_provider.
In a typical application there may still be struct revocable instances that have a reference to the provider, so we can't claim that it's freed here.
So, given that, I'd rather call this revocable_provider_revoke().
+static void devm_revocable_provider_free(void *data) +{
- struct revocable_provider *rp = data;
- revocable_provider_free(rp);
+}
Same here, I'd call this devm_revocable_provider_revoke().
+DEFINE_FREE(revocable, struct revocable *, if (_T) revocable_release(_T))
+#define _REVOCABLE(_rev, _label, _res) \
- for (struct revocable *__UNIQUE_ID(name) __free(revocable) = _rev; \
(_res = revocable_try_access(_rev)) || true; ({ goto _label; })) \if (0) { \+_label: \
break; \} else+#define REVOCABLE(_rev, _res) _REVOCABLE(_rev, __UNIQUE_ID(label), _res)
This is basically the same as Revocable::try_access_with() [1] in Rust, i.e. try to access and run a closure.
Admittedly, REVOCABLE_TRY_ACCESS_WITH() is pretty verbose and I also do not have a great idea to shorten it.
Maybe you have a good idea, otherwise I'm also fine with the current name.
Otherwise, maybe it's worth to link to the Rust Revocable API for reference?
With *_free() renamed to *_revoke(), feel free to add:
Acked-by: Danilo Krummrich dakr@kernel.org
[1] https://rust.docs.kernel.org/kernel/revocable/struct.Revocable.html#method.t...