On Fri, 6 Feb 2026 23:02:44 +0100, "Paul E. McKenney" <[email protected]> said: > On Fri, Feb 06, 2026 at 11:32:06AM +0100, Bartosz Golaszewski wrote: >> 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 <[email protected]> >> --- >> 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/[email protected]/ >> >> Could you please take a look and say if the design looks sane to you? >> Especially the double SRCU on the revocable_provider. > > The first patch in the above URL adds SRCU, and the other > two add various tests. I do not see a double SRCU, just an > srcu_read_lock() in revocable_try_access() and an srcu_read_unlock() > in revocable_withdraw_access(). > > You are allowed to nest srcu_read_lock(), if that is what you are asking. > *However*, nesting revocable_try_access() on the same revocable structure > is buggy because the second call to revocable_try_access() would overwrite > the rp->srcu value written by the first call. This could result in both > SRCU grace-period hangs and too-short SRCU grace periods, more likely > the former than the latter. > > Or do you mean something else by "double SRCU"? >
This series didn't have it yet, it appeared as a fix to a race reported after it was queued, sorry for the confusion. I'm talking about this bit[1] here. It returns an __rcu-annotated pointer, forcing the user to keep and manage it. This is because when revoking the resource[2], the pointer storing the address of the revokable provider is also managed by SRCU - in addition to the revokable resource itself which seems to me like a weird concept. I understand the race condition it fixes but I assumed the whole concept of revokable is to free the user from being bothered by the implementation details behind it which leak out of the API if you need to keep __rcu around. Bart [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/linux/revocable.h#n25 [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/base/revocable.c#n123
