On Mon, Feb 09, 2026 at 05:13:17AM -0600, Bartosz Golaszewski wrote: > 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.
Please hold off on reviewing the patch and the "double SRCU" usage for now. I'll remove the second RCU in the next version, which should serve as a better starting point for a clean review. Side note: It was actually one SRCU and one RCU, not a double SRCU.
