Message-ID: <20260206103216.38623-1-bartosz.golaszewski@oss.qualcomm.com>
Date: Fri,  6 Feb 2026 11:32:06 +0100
From: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
To: Tzung-Bi Shih <tzungbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Danilo Krummrich <dakr@...nel.org>, Shuah Khan <shuah@...nel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Johan Hovold <johan@...nel.org>, Bartosz Golaszewski <brgl@...nel.org>
Cc: linux-kernel@...r.kernel.org, driver-core@...ts.linux.dev,
        linux-kselftest@...r.kernel.org,
        Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
Subject: [PATCH] revocable: hide the implementation details from users

Revocable stacks two layers of SRCU on top of each other: one to protect
the actual revocable resource and another to synchronize the revoking.
While this design itself is questionable, it also forces the user of
revokable to think about the implementation details and annotate the
pointer holding the address of the revocable_provider struct with __rcu.
Hide the real type of struct revokable_provider behind a typedef to free
the users from this responsability. While adding new typedefs goes
against current guidelines, it's still better than the current
requirement.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>
---
I realized that one important person was missing from the whole review
process: Paul E. McKenney who wrote and maintains SRCU. I had Paul look
at the SRCU usage in GPIO and I think he should have also signed off on
revocable before it got queued.

Paul: I'm Cc'ing you on this patch to bring revocable to your attention.
The series that implemented it and made its way into v7.0 is here:

  https://lore.kernel.org/all/20260116080235.350305-1-tzungbi@kernel.org/

Could you please take a look and say if the design looks sane to you?
Especially the double SRCU on the revocable_provider.

 drivers/base/revocable.c                           |  8 ++++----
 drivers/base/revocable_test.c                      | 14 +++++++-------
 include/linux/revocable.h                          |  8 +++++---
 .../base/revocable/test_modules/revocable_test.c   |  4 ++--
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c
index 8532ca6a371c..02dd3ec2c9ad 100644
--- a/drivers/base/revocable.c
+++ b/drivers/base/revocable.c
@@ -82,7 +82,7 @@ struct revocable_provider {
  * Return: The pointer of struct revocable_provider.  NULL on errors.
  * It enforces the caller handles the returned pointer in RCU ways.
  */
-struct revocable_provider __rcu *revocable_provider_alloc(void *res)
+revocable_provider_t revocable_provider_alloc(void *res)
 {
 	struct revocable_provider *rp;
 
@@ -94,7 +94,7 @@ struct revocable_provider __rcu *revocable_provider_alloc(void *res)
 	RCU_INIT_POINTER(rp->res, res);
 	kref_init(&rp->kref);
 
-	return (struct revocable_provider __rcu *)rp;
+	return (revocable_provider_t)rp;
 }
 EXPORT_SYMBOL_GPL(revocable_provider_alloc);
 
@@ -120,7 +120,7 @@ static void revocable_provider_release(struct kref *kref)
  * It enforces the caller to pass a pointer of pointer of resource provider so
  * that it sets \*rp_ptr to NULL to prevent from keeping a dangling pointer.
  */
-void revocable_provider_revoke(struct revocable_provider __rcu **rp_ptr)
+void revocable_provider_revoke(revocable_provider_t *rp_ptr)
 {
 	struct revocable_provider *rp;
 
@@ -143,7 +143,7 @@ EXPORT_SYMBOL_GPL(revocable_provider_revoke);
  *
  * Return: 0 on success, -errno otherwise.
  */
-int revocable_init(struct revocable_provider __rcu *_rp, struct revocable *rev)
+int revocable_init(revocable_provider_t _rp, struct revocable *rev)
 {
 	struct revocable_provider *rp;
 
diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c
index 27f5d7d96f4b..732197c887ef 100644
--- a/drivers/base/revocable_test.c
+++ b/drivers/base/revocable_test.c
@@ -30,7 +30,7 @@
 
 static void revocable_test_basic(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	struct revocable rev;
 	void *real_res = (void *)0x12345678, *res;
 	int ret;
@@ -52,7 +52,7 @@ static void revocable_test_basic(struct kunit *test)
 
 static void revocable_test_revocation(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	struct revocable rev;
 	void *real_res = (void *)0x12345678, *res;
 	int ret;
@@ -79,7 +79,7 @@ static void revocable_test_revocation(struct kunit *test)
 
 static void revocable_test_try_access_macro(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	void *real_res = (void *)0x12345678, *res;
 
 	rp = revocable_provider_alloc(real_res);
@@ -101,7 +101,7 @@ static void revocable_test_try_access_macro(struct kunit *test)
 
 static void revocable_test_try_access_macro2(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	void *real_res = (void *)0x12345678, *res;
 	bool accessed;
 
@@ -128,7 +128,7 @@ static void revocable_test_try_access_macro2(struct kunit *test)
 
 static void revocable_test_provider_use_after_free(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	struct revocable_provider *old_rp;
 	struct revocable rev;
 	void *real_res = (void *)0x12345678;
@@ -167,7 +167,7 @@ static void revocable_test_provider_use_after_free(struct kunit *test)
 
 struct test_concurrent_access_context {
 	struct kunit *test;
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	struct revocable rev;
 	struct completion started, enter, exit;
 	struct task_struct *thread;
@@ -206,7 +206,7 @@ static int test_concurrent_access_consumer(void *data)
 
 static void revocable_test_concurrent_access(struct kunit *test)
 {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	void *real_res = (void *)0x12345678;
 	struct test_concurrent_access_context *ctx;
 	int ret, i;
diff --git a/include/linux/revocable.h b/include/linux/revocable.h
index e3d6d2c953a3..f0aea6f56a25 100644
--- a/include/linux/revocable.h
+++ b/include/linux/revocable.h
@@ -12,6 +12,8 @@
 struct device;
 struct revocable_provider;
 
+typedef struct revocable_provider __rcu *revocable_provider_t;
+
 /**
  * struct revocable - A handle for resource consumer.
  * @rp: The pointer of resource provider.
@@ -22,10 +24,10 @@ struct revocable {
 	int idx;
 };
 
-struct revocable_provider __rcu *revocable_provider_alloc(void *res);
-void revocable_provider_revoke(struct revocable_provider __rcu **rp);
+revocable_provider_t revocable_provider_alloc(void *res);
+void revocable_provider_revoke(revocable_provider_t *rp);
 
-int revocable_init(struct revocable_provider __rcu *rp, struct revocable *rev);
+int revocable_init(revocable_provider_t rp, struct revocable *rev);
 void revocable_deinit(struct revocable *rev);
 void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->srcu);
 void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp->srcu);
diff --git a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
index a560ceda7318..30cc9c145725 100644
--- a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
+++ b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
@@ -17,7 +17,7 @@
 static struct dentry *debugfs_dir;
 
 struct revocable_test_provider_priv {
-	struct revocable_provider __rcu *rp;
+	revocable_provider_t rp;
 	struct dentry *dentry;
 	char res[16];
 };
@@ -37,7 +37,7 @@ static ssize_t revocable_test_consumer_read(struct file *filp,
 	char data[16];
 	size_t len;
 	struct revocable rev;
-	struct revocable_provider __rcu *rp = filp->private_data;
+	revocable_provider_t rp = filp->private_data;
 
 	switch (*offset) {
 	case 0:
-- 
2.47.3

Powered by blists - more mailing lists